[
https://issues.apache.org/jira/browse/HDFS-7496?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Lei (Eddy) Xu updated HDFS-7496:
--------------------------------
Attachment: HDFS-7496.001.patch
Thanks for the reviews, [~cmccabe]. They are very helpful. I have made changes
accordingly, detailed as following:
bq. * Rather than having a boolean hasReference, let's have an actual pointer
to the FsVolumeSpi object.
bq. * We should release the reference count in close(), not in the finally block
bq. * We don't need this code any more:
Done
bq. This change seems unrelated to this JIRA...
Yes, this is not related. I removed it from this patch. I should follow another
JIRA to use {{File#canonicalPath}} to compare the volumes. But it should not
throw {{IOE}} for {{File#getCanonicalPath()}}, as you mentioned above.
bq. How about using Preconditions.checkNonNull here... might look nicer
It is for simplifying the test code, i.e., it does not need to construct a
{{Datanode}} object for {{FsVolumeImpl}} tests.
bq. What I was envisioning was having getNextVolume increment the reference
count when it retrieved the volume,
{{getNextVolume}} is not the only place to pass an _active_ volume. I have made
the change that if BlockReceiver's constructor successes, it must hold a
reference count from
{{FsDatasetImpl#createRbw/createTemporary/append/recoverRbw/recoverAppend}}.
Also I added a {{FsVolumeReference}} helper class to let the caller use
{{try-with-resources}} on the reference count.
> Fix FsVolume removal race conditions on the DataNode
> -----------------------------------------------------
>
> Key: HDFS-7496
> URL: https://issues.apache.org/jira/browse/HDFS-7496
> Project: Hadoop HDFS
> Issue Type: Bug
> Reporter: Colin Patrick McCabe
> Assignee: Lei (Eddy) Xu
> Attachments: HDFS-7496.000.patch, HDFS-7496.001.patch
>
>
> We discussed a few FsVolume removal race conditions on the DataNode in
> HDFS-7489. We should figure out a way to make removing an FsVolume safe.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
