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

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

Hi, [~virajith] 

Thanks for addressing my comments. 

{code}
    // for local replicas, the replica location is assumed to be a file.
    File diskFile = null;
    try {
      diskFile = new File(replicaLocation.getUri());
    } catch (NullPointerException | IllegalArgumentException e) {
      diskFile = null;
    }
{code}

* Catching {{NullPointerException}} here seems suspicious to me. Could you tell 
which one is potentially {{null}}? {{replicaLocation}} or {{getUrl() -> 
baseUrl}}?

* For {{StorageLocation#toString()}}, it might be good to just print {{URI}} 
itself instead of its path to be more verbose. 

* {{public File getBpFile(String bpid, String currentStorageDir) }}, renamed to 
{{getBpDir(...)}}?

* Should we mark {{StorageLocation#getFile}} as deprecated?

One more question, as we are going to remove {{File}} from {{StorageLocation}}. 
Should we just remove {{StorageLocation#file}} and 
{{StorageLocation#getFile()}} once for all? I took a simple scan of the code 
base, {{getFile()}} is used 35 times, while 10 usages are in the scope of 
{{StorageLocation}} and {{StorageDirectory}}, while rest of them are all in the 
tests. So I think it'd be rather safe to remove them in hadoop 3. What do you 
think?

* Should {{StorageLocation#matchesStorageDirectory()}}s be moved to 
{{StorageDirectory}} class?


> Modifications to remove the assumption that StorageLocation is associated 
> with java.io.File in Datanode.
> --------------------------------------------------------------------------------------------------------
>
>                 Key: HDFS-10638
>                 URL: https://issues.apache.org/jira/browse/HDFS-10638
>             Project: Hadoop HDFS
>          Issue Type: Sub-task
>          Components: datanode, fs
>            Reporter: Virajith Jalaparti
>            Assignee: Virajith Jalaparti
>         Attachments: HDFS-10638.001.patch, HDFS-10638.002.patch, 
> HDFS-10638.003.patch, HDFS-10638.004.patch
>
>
> Changes to ensure that {{StorageLocation}} need not be associated with a 
> {{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