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

Lei (Eddy) Xu commented on HDFS-10637:
--------------------------------------

Hi, [~virajith]  Thanks a lot for update the patch. It looks really good.

bq. currentDir is now just a private variable that holds this value. 

My only concerns is that, because {{currentDir == storageLocation + 
STORAGE_DIR_CURRENT}}, putting both of them into {{FsVolumeImpl}}, introduces 
the chance of inconsistent states (i.e., {{currentDir != storageLocation + 
/current}}).  Alternatively, what about setting {{FsVolumeImpl#storageStorage}} 
as {{final}}, and do not introduce {{private FsVolumeImpl(...String 
currentDir...StorageLocation storageLocation)}}, which will likely become 
{{public}}  later by other people.  So that you can consider {{currentDir}} is 
a cached value of {{storageLocation}} or vice versa. 

What do you think?



> 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, 
> HDFS-10637.009.patch
>
>
> Modifications to {{FsVolumeSpi}} and {{FsVolumeImpl}} to remove references to 
> {{java.io.File}}.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

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