[GitHub] [hadoop-ozone] adoroszlai commented on a change in pull request #183: HDDS-2505. Fix logic related to SCM address calculation in HddsUtils

[GitBox <gi...@apache.org>]

adoroszlai commented on a change in pull request #183: HDDS-2505. Fix logic related to SCM address calculation in HddsUtils
URL: https://github.com/apache/hadoop-ozone/pull/183#discussion_r348291029
 
 

 ##########
 File path: hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/HddsUtils.java
 ##########
 @@ -277,53 +246,67 @@ public static SCMSecurityProtocolClientSideTranslatorPB getScmSecurityClient(
    * @throws IllegalArgumentException if any values are not in the 'host'
    *             or host:port format.
    */
-  public static Optional<Integer> getPortNumberFromConfigKeys(
+  public static OptionalInt getPortNumberFromConfigKeys(
       Configuration conf, String... keys) {
     for (final String key : keys) {
       final String value = conf.getTrimmed(key);
-      final Optional<Integer> hostPort = getHostPort(value);
+      final OptionalInt hostPort = getHostPort(value);
       if (hostPort.isPresent()) {
         return hostPort;
       }
     }
-    return Optional.empty();
+    return OptionalInt.empty();
   }
 
   /**
    * Retrieve the socket addresses of all storage container managers.
    *
-   * @param conf
    * @return A collection of SCM addresses
    * @throws IllegalArgumentException If the configuration is invalid
    */
   public static Collection<InetSocketAddress> getSCMAddresses(
-      Configuration conf) throws IllegalArgumentException {
-    Collection<InetSocketAddress> addresses =
-        new HashSet<InetSocketAddress>();
+      Configuration conf) {
     Collection<String> names =
         conf.getTrimmedStringCollection(ScmConfigKeys.OZONE_SCM_NAMES);
-    if (names == null || names.isEmpty()) {
+    if (names.isEmpty()) {
       throw new IllegalArgumentException(ScmConfigKeys.OZONE_SCM_NAMES
           + " need to be a set of valid DNS names or IP addresses."
-          + " Null or empty address list found.");
+          + " Empty address list found.");
     }
 
-    final Optional<Integer> defaultPort = Optional
-        .of(ScmConfigKeys.OZONE_SCM_DEFAULT_PORT);
+    Collection<InetSocketAddress> addresses = new HashSet<>(names.size());
     for (String address : names) {
       Optional<String> hostname = getHostName(address);
       if (!hostname.isPresent()) {
         throw new IllegalArgumentException("Invalid hostname for SCM: "
-            + hostname);
+            + address);
       }
-      Optional<Integer> port = getHostPort(address);
-      InetSocketAddress addr = NetUtils.createSocketAddr(hostname.get(),
-          port.orElse(defaultPort.get()));
+      int port = getHostPort(address)
+          .orElse(ScmConfigKeys.OZONE_SCM_DEFAULT_PORT);
+      InetSocketAddress addr = NetUtils.createSocketAddr(hostname.get(), port);
       addresses.add(addr);
     }
     return addresses;
   }
-  
+
+  /**
+   * Same as {@link #getSCMAddresses(Configuration)}, but verifies that
+   * only one SCM address is configured, as currently multiple ones are not
+   * supported.  When multi-SCM support is added, simply replace calls with the
+   * delegate method.
+   *
+   * @return a collection of SCM addresses with exactly 1 item
+   * @throws IllegalArgumentException if {@code conf} has more than one SCM
+   *         address or it has none
+   */
+  public static Collection<InetSocketAddress> getSingleSCMAddress(
 
 Review comment:
   Thanks @xiaoyuyao reviewing this.
   
   > should we just return a single InetSocketAddress here instead of a collection?
   
   I think it depends on whether we may want to keep this method when multi-SCM support is added.  I considered it to be a temporary one.  Otherwise a single address might be better, as it simplifies callers.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org