[
https://issues.apache.org/jira/browse/HDFS-7758?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14382425#comment-14382425
]
Colin Patrick McCabe commented on HDFS-7758:
--------------------------------------------
Thanks, Eddy. Sorry that reviews have been a little slow.
It's nice to see that we can use the new java 7 try-with-resources with this
class. Instead of {{ReferredFsVolumeList}}, how about calling the class
{{FsVolumeRefs}}?
{code}
295 } else {
296 // If the volume is not put into a volume scanner, it does not
need to
297 // hold the reference.
298 IOUtils.cleanup(null, ref);
{code}
This looks like a bugfix, separate from this code cleanup. Let's file another
JIRA for this and get it committed quickly.
{code}
-public interface FsVolumeReference extends Closeable {
+public abstract class FsVolumeReference implements Closeable {
{code}
What's the motivation for switching from an interface to an abstract class? An
interface seems more appropriate here since there are no implementations of any
of the functions.
Why do we need both {{ReferredFsVolumeListImpl}} and {{ReferredFsVolumeList}}?
It seems like every FsDatasetSpi implementation is going to want to implement
this the same way, as a list of {{FsVolumeRef}} objects. Since {{FsVolumeRef}}
objects are used by every FSDataset implementation, this code is already
generic and we don't need more abstractions.
> Retire FsDatasetSpi#getVolumes() and use FsDatasetSpi#getVolumeRefs() instead
> -----------------------------------------------------------------------------
>
> Key: HDFS-7758
> URL: https://issues.apache.org/jira/browse/HDFS-7758
> Project: Hadoop HDFS
> Issue Type: Improvement
> Components: datanode
> Affects Versions: 2.6.0
> Reporter: Lei (Eddy) Xu
> Assignee: Lei (Eddy) Xu
> Attachments: HDFS-7758.000.patch, HDFS-7758.001.patch,
> HDFS-7758.002.patch, HDFS-7758.003.patch
>
>
> HDFS-7496 introduced reference-counting the volume instances being used to
> prevent race condition when hot swapping a volume.
> However, {{FsDatasetSpi#getVolumes()}} can still leak the volume instance
> without increasing its reference count. In this JIRA, we retire the
> {{FsDatasetSpi#getVolumes()}} and propose {{FsDatasetSpi#getVolumeRefs()}}
> and etc. method to access {{FsVolume}}. Thus it makes sure that the consumer
> of {{FsVolume}} always has correct reference count.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
