[ 
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

Reply via email to