Lei (Eddy) Xu commented on HDFS-10637:

HI, [~virajith]

Thanks for the patch. 

* Please add ASF license to {{FsVolumeImplBuilder}}
* {{TestNameNodePrunesMissingStorages.java}}:

      StorageLocation volumeDirectoryToRemove = null;

Change to {{volumeLocationToRemove}}?

237               new File(volumeDirectoryToRemove.getFile().toString()));

{{getFile()}} already returns a {{File}}. you dont need to create a new one.

* In {{FsVolumeImpl}}.  can you remove {{this.currentDir}}? 

* {{FsDatasetSpi#getFileInFinalizedDir()}}, it still returns a File. Is there a 
way to eliminate it?

* {{FsVolumeSpi#containsPath}} is only used by test?  I feel that it is still 
assuming a file based {{FsVolume}}.

It is a large patch. Please allow more comments to come later. Thanks.

> Modifications to remove the assumption that FsVolumes are backed by 
> java.io.File.
> ---------------------------------------------------------------------------------
>                 Key: HDFS-10637
>                 URL: https://issues.apache.org/jira/browse/HDFS-10637
>             Project: Hadoop HDFS
>          Issue Type: Sub-task
>          Components: datanode, fs
>            Reporter: Virajith Jalaparti
>            Assignee: Virajith Jalaparti
>         Attachments: HDFS-10637.001.patch, HDFS-10637.002.patch, 
> HDFS-10637.003.patch, HDFS-10637.004.patch, HDFS-10637.005.patch, 
> HDFS-10637.006.patch, HDFS-10637.007.patch, HDFS-10637.008.patch
> Modifications to {{FsVolumeSpi}} and {{FsVolumeImpl}} to remove references to 
> {{java.io.File}}.

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