[ 
https://issues.apache.org/jira/browse/HDFS-6728?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14088388#comment-14088388
 ] 

Aaron T. Myers commented on HDFS-6728:
--------------------------------------

Hi Eddy, the latest patch looks pretty good to me. Just a few small nits. +1 
once these are addressed:

# I realize you didn't write this error message, rather you just moved it, but 
let's go ahead and switch the wording to the more common "DataNode" and 
"NameNode":
{code}
+    LOG.info("Data-node version: " + HdfsConstants.DATANODE_LAYOUT_VERSION
+        + " and name-node layout version: " + nsInfo.getLayoutVersion());
{code}
# Could you perhaps expand upon this comment what exactly "fully processed" 
means here?
{code}
+   * If isInitialize is false, only fully processed directories will be added
+   * into DataStorage.
{code}
# s/firstly ensure/first ensures/g:
{code}
+    // Similar to recoverTransitionRead, it firstly ensure the datanode level
{code}
# The test class is a little over-sharey. For example, I think all of the 
helper methods createStorageLocations(int), 
createStorageLocations(int,boolean), createNamespaceInfos, and both checkDirs 
can be made static if you make the constants at the top of the file static. 
Related, please make all the constants in the test file static, and instead of 
camel-casing them make them ALL_CAPS with underscores to separate the words.

> Dynamically add new volumes to DataStorage, formatted if necessary.
> -------------------------------------------------------------------
>
>                 Key: HDFS-6728
>                 URL: https://issues.apache.org/jira/browse/HDFS-6728
>             Project: Hadoop HDFS
>          Issue Type: Sub-task
>          Components: datanode
>    Affects Versions: 2.4.0
>            Reporter: Lei (Eddy) Xu
>            Assignee: Lei (Eddy) Xu
>              Labels: datanode
>         Attachments: HDFS-6728.000.patch, HDFS-6728.000.patch, 
> HDFS-6728.001.patch, HDFS-6728.002.patch
>
>
> When dynamically adding a volume to {{DataStorage}}, it should prepare the 
> {{data dir}}, e.g., formatting if it is empty.



--
This message was sent by Atlassian JIRA
(v6.2#6252)

Reply via email to