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

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

 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());
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.

 * 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

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