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

Nanda kumar commented on HDFS-13162:
------------------------------------

Thanks [~bharatviswa] for working on this, overall the patch looks good to me.
Apart from checkstyle issues, couple of minor comments

*StorageLocation.java*
 * {{makeBlockPoolDir}}: We can use {{DiskChecker.checkDir}} to create replica 
trash directory and also have it inside the same {{try..cache}} block.

{code:java}
 try {
   DiskChecker.checkDir(localFS, new Path(data.toURI()), permission);
   if (replicaTrashEnable) {
     File replicaTrash = new File(data, DataStorage.STORAGE_DIR_REPLICA_TRASH);
     DiskChecker.checkDir(localFS, new Path(replicaTrash.toURI()), permission);
   }
 } catch (IOException e) {
      DataStorage.LOG.warn("Invalid directory in: " + data.getCanonicalPath() +
          ": " + e.getMessage());
 }
{code}
With this approach, we are not explicitly printing {{replicaTrashDir}} in case 
of an exception, but that should be printed as part of {{e.getMessage()}}.
Also based on existing implementation, we are not throwing IOException even if 
directory creation fails. We log a warn message and continue.

*hdfs-default.xml*
 * Description "{{which will help in accidental deletion to restore back the 
data.}}" is somewhat ambiguous, can we rephrase it to something like "{{which 
will help in the recovery of accidentally deleted data.}}"

> Create Replica Trash directory on DN startup
> --------------------------------------------
>
>                 Key: HDFS-13162
>                 URL: https://issues.apache.org/jira/browse/HDFS-13162
>             Project: Hadoop HDFS
>          Issue Type: Sub-task
>            Reporter: Bharat Viswanadham
>            Assignee: Bharat Viswanadham
>            Priority: Major
>         Attachments: HDFS-13162.00-HDFS-12996.00.patch
>
>




--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

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