[
https://issues.apache.org/jira/browse/HDFS-11221?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15929226#comment-15929226
]
Virajith Jalaparti commented on HDFS-11221:
-------------------------------------------
Thanks [~jiajia] for taking on this work.
Going through the latest patch, it is unclear to me if we need this fix. The
current patch essentially replaces calls to {{getRoot()}}, {{getCurrentDir()}}
etc. with calls to {{getRoot().get()}}, {{getCurrentDir().get()}} etc. So, as
[~ehiggs] mentioned in his earlier comment, instead of a
{{NullPointerException}}, we get a {{NoSuchElementException}} when the {{root}}
of a {{StorageDirectory}} is {{null}}. This does not add much functionality.
Ideally, the fix should be such that we take actions based on whether {{root}}
(and/or the other directories) is (are) {{null}} or not. It seems hard to come
up with a generic action to take when, for example, {{root}} indeed turns out
to be {{null}}. The latest patch for HDFS-10675 deals with this case in the
Datanode assuming that {{root}} is {{null}} only for a provided
{{StorageDirectory}}. When calls to {{getCurrentDir()}}, {{getRoot()}} etc
return {{null}}, it takes the appropriate action (e.g.,
{{StorageDirectory#analyzeStorage}}, {{BlockPoolSliceStorage#doTransition}},
etc.) that is specific to the function where they are called from. Doing this
generally seems hard as it will not be clear why {{root}} is {{null}}.
[~jiajia], [~ehiggs] [~andrew.wang] what do you think?
> Have StorageDirectory return Optional<File> instead of File/null
> ----------------------------------------------------------------
>
> Key: HDFS-11221
> URL: https://issues.apache.org/jira/browse/HDFS-11221
> Project: Hadoop HDFS
> Issue Type: Task
> Reporter: Ewan Higgs
> Assignee: Jiajia Li
> Priority: Minor
> Attachments: HDFS-11221-002.patch, HDFS-11221-v1.patch
>
>
> In HDFS-10675, {{StorageDirectory.root}} can be {{null}} because {{PROVIDED}}
> storage locations will not have any directories associated with them. Hence,
> we need to add checks to StorageDirectory to make sure we handle this. This
> would also lead to changes in code that call {{StorageDirectory.getRoot}},
> {{StorageDirectory.getCurrentDir}}, {{StorageDirectory.getVersionFile}} etc.
> as the return value can be {{nul}}l (if {{StorageDirectory.root}} is null).
> The proposal to handle this is to change the return type of the above
> functions to {{Optional<File>}}. According to my preliminary check, this will
> result in changes in ~70 places, which is why it's not appropriate to put it
> in the patch for HDFS-10675. But it is certainly a valuable fix.
--
This message was sent by Atlassian JIRA
(v6.3.15#6346)
---------------------------------------------------------------------
To unsubscribe, e-mail: hdfs-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: hdfs-issues-h...@hadoop.apache.org
