[GitHub] [hadoop] fengnanli commented on a change in pull request #3346: HDFS-16188. RBF: Router to support resolving monitored namenodes with DNS

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

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



##########
File path: hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/tools/NNHAServiceTarget.java
##########
@@ -56,9 +56,25 @@
   private final String nnId;
   private final String nsId;
   private final boolean autoFailoverEnabled;
-  
+
   public NNHAServiceTarget(Configuration conf,
       String nsId, String nnId) {
+    this(conf, nsId, nnId, null, null, null);
+  }
+
+  /**
+   * Create a NNHAServiceTarget for a namenode.
+   *
+   * @param conf          HDFS configuration.
+   * @param nsId          nsId of this nn.
+   * @param nnId          nnId of this nn.
+   * @param serviceAddr   Provided service address.
+   * @param addr          Provided service address.
+   * @param lifelineAddr  Provided service address.
+   */
+  public NNHAServiceTarget(Configuration conf,

Review comment:
       Instead of overloading the constructor like this, can we have the common part extracted as a function and call it inside? Having a lot of null check for parameters feels not ideal:
   NNHAServiceTarget(Configuration conf, String nsId, String nnId) {
      // do addrs assignment from config
      .....
      assignAutofailoverAndFencer();
   }
   
   NNHAServiceTarget(Configuration conf, String nsId, String nnId, String serviceAddr, addr, lifelineAddr) {
      // do assignment from param
      ....
      assignAutofailoverAndFencer();
   }
   
   Also please add a simple test for it.




-- 
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.

To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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