fengnanli commented on a change in pull request #2639:
URL: https://github.com/apache/hadoop/pull/2639#discussion_r574117112



##########
File path: 
hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DFSUtil.java
##########
@@ -647,6 +634,58 @@ public static String addKeySuffixes(String key, String... 
suffixes) {
       getNNLifelineRpcAddressesForCluster(Configuration conf)
       throws IOException {
 
+    Collection<String> parentNameServices = getParentNameServices(conf);
+
+    return getAddressesForNsIds(conf, parentNameServices, null,
+        DFS_NAMENODE_LIFELINE_RPC_ADDRESS_KEY);
+  }
+
+  //
+  /**
+   * Returns the configured address for all NameNodes in the cluster.
+   * This is similar with DFSUtilClient.getAddressesForNsIds()
+   * but can access DFSConfigKeys.
+   *
+   * @param conf configuration
+   * @param defaultAddress default address to return in case key is not found.
+   * @param keys Set of keys to look for in the order of preference
+   *
+   * @return a map(nameserviceId to map(namenodeId to InetSocketAddress))
+   */
+  static Map<String, Map<String, InetSocketAddress>> getAddressesForNsIds(

Review comment:
       Can we try this to reduce the code duplicity? Override function 
`DFSUtilClient.getAddressesForNsIds()` by adding a boolean var indicating 
whether to resolve (the var is fetched from the config). 
   Inside the `DFSUtilClient.getAddressesForNameserviceId`, add another 
override with the boolean, make the current one with the value false. If the 
var is true, do the DNS resolving and return addresses.

##########
File path: 
hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DFSConfigKeys.java
##########
@@ -1557,6 +1557,17 @@
   public static final double
       DFS_DATANODE_RESERVE_FOR_ARCHIVE_DEFAULT_PERCENTAGE_DEFAULT = 0.0;
 
+
+  public static final String
+      DFS_NAMESERVICES_RESOLUTION_ENABLED =

Review comment:
       If we maintain only one config across nn, qjm, zkfc and dn, this is an 
issue since the other three don't support DNS yet. I am thinking about how to 
do it for now and it requires some refactor in places such as 
`DFSUtil.getSuffixIDs` (used by zkfc). I will follow up on this soon. ATM we 
can keep a separate config for DN only as a short term solution.




-- 
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:
[email protected]



---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to