[ https://issues.apache.org/jira/browse/HDFS-15346?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17120981#comment-17120981 ]
Yiqun Lin commented on HDFS-15346: ---------------------------------- Hi [~LiJinglun] , some initial review comments from me: *DistCpFedBalance.java* # line 77 I suggest to extract 'submit' as a static variable in this class. # line 85 the same comment to extract. # line 127 Can you complete the javadoc of this method? # line 132: Why the default bandwidth is only 1 for fedbaalance, will not be too small? # line 137, 140, 150 We can use method CommandLine#hasOption to extract Boolean type input value. # line 178 Can you complete the javadoc of construct method? # line 199, 206, 210, 215 Also suggest to use static variable rather than hard-coded value in these places. # line 228 rClient not closed after it's used. *DistCpProcedure.java* # line 191 We can use HdfsConstants.SEPARATOR_DOT_SNAPSHOT_DIR_SEPARATOR to replace '/.snapshot/' # line 306 It will be better if we can add some necessary describe for the steps of diff distcp job submission. # line 374 Can we replace '.snapshot' with HdfsConstants.DOT_SNAPSHOT_DIR in all other places in this class? *TestDistCpProcedure.java* Can you use replace HdfsConstants.DOT_SNAPSHOT_DIR to replace '.snapshot' in this class? *TestTrashProcedure.java* {quote}Path src = new Path(nnUri + "/"+getMethodName()+"-src"); Path dst = new Path(nnUri + "/"+getMethodName()+"-dst"); {quote} We don't need to use nnUri here because we have already got the Filesystem instance. If we don't want to specified for one namespace, URI prefix can be ignored, default fs will be used. We can simplifed to {quote}Path src = new Path("/"+getMethodName()+"-src"); Path dst = new Path("/"+getMethodName()+"-dst"); {quote} > RBF: DistCpFedBalance implementation > ------------------------------------ > > Key: HDFS-15346 > URL: https://issues.apache.org/jira/browse/HDFS-15346 > Project: Hadoop HDFS > Issue Type: Sub-task > Reporter: Jinglun > Assignee: Jinglun > Priority: Major > Attachments: HDFS-15346.001.patch, HDFS-15346.002.patch, > HDFS-15346.003.patch, HDFS-15346.004.patch > > > Patch in HDFS-15294 is too big to review so we split it into 2 patches. This > is the second one. Detail can be found at HDFS-15294. -- This message was sent by Atlassian Jira (v8.3.4#803005) --------------------------------------------------------------------- To unsubscribe, e-mail: hdfs-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: hdfs-issues-h...@hadoop.apache.org