hadoop-hdfs-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Xiaoyu Yao (JIRA)" <j...@apache.org>
Subject [jira] [Comment Edited] (HDDS-116) Implement VolumeSet to manage disk volumes
Date Tue, 29 May 2018 20:03:00 GMT

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

Xiaoyu Yao edited comment on HDDS-116 at 5/29/18 8:02 PM:
----------------------------------------------------------

Thanks [~hanishakoneru] for the update. The patch v3 looks good to me overall. I have a few
more comments:

 

VolumeInfo.java

Line 21: NIT: unused imports

 

Line 53-54: we could use a single EnumMap to track volumes in individual  states, this way,
we can accommodate new states with less code changes.

private EnumMap<VolumeInfo.VolumeState, List<VolumeInfo>> volumeStateMap;

 

Line 90: should we have a UNINITIALIZED(or INVALID) state so that invalid volume will not
be used. This can be done by setting a default volume state in the builder class.

 

VolumeSet.java

Do we need expose the volume set lock acquire/release to the callers such as the volume choosing
policy? Otherwise, the caller won't have control over the volume set changes.

 

RoundRobinVolumeChoosingPolicy.java

Line 38: nextVolumeIndex can be replaced with a LongAdder. This way, we might not need a policy
lock.

 

Line 40-51: we need to grab the volumeset lock here to avoid changes during the choosing.
Assuming we get a unmodifible list from VolumeSet, the actual volume in VolumeSet may still
be removed even with the RRVolumeChoosingPolicy lock in line 51, resulting in I/O error when
writing to a removed volume.

 

TestRoundRobinVolumeChoosingPolicy.java

Lien 93: This is a good test. Can we add a multi-threaded test so that one thread add/remove
volumes while the other run policy choosing based on the same volume set works correctly?


was (Author: xyao):
Thanks [~hanishakoneru] for the update. The patch v3 looks good to me overall. I have a few
more comments:

 

VolumeInfo.java

Line 21: NIT: unused imports

 

Line 53-54: we could use a single EnumMap to track volumes in individual  states, this way,
we can accommodate new states with less code changes.

private EnumMap<VolumeInfo.VolumeState, List<VolumeInfo>> volumeStateMap;

 

Line 90: should we have a UNINITIALIZED(or INVALID) state so that invalid volume will not
be used. This can be done by setting a default volume state in the builder class.

 

VolumeSet.java

Do we need expose the volume set lock acquire/release to the callers such as the volume choosing
policy? Otherwise, the caller won't have control over the volume set changes.

 

RoundRobinVolumeChoosingPolicy.java

Line 38: *nextVolumeIndex can be replaced with a LongAdder. This way, we might not need a
policy lock.*

 

Line 40-51: we need to grab the volumeset lock here to avoid changes during the choosing.
Assuming we get a unmodifible list from VolumeSet, the actual volume in VolumeSet may still
be removed even with the RRVolumeChoosingPolicy lock in line 51, resulting in I/O error when
writing to a removed volume.

 

TestRoundRobinVolumeChoosingPolicy.java

Lien 93: This is a good test. Can we add a multi-threaded test so that one thread add/remove
volumes while the other run policy choosing based on the same volume set works correctly?

> Implement VolumeSet to manage disk volumes
> ------------------------------------------
>
>                 Key: HDDS-116
>                 URL: https://issues.apache.org/jira/browse/HDDS-116
>             Project: Hadoop Distributed Data Store
>          Issue Type: Sub-task
>            Reporter: Hanisha Koneru
>            Assignee: Hanisha Koneru
>            Priority: Major
>              Labels: ContainerIO
>             Fix For: 0.2.1
>
>         Attachments: HDDS-116-HDDS-48.001.patch, HDDS-116-HDDS-48.002.patch, HDDS-116-HDDS-48.003.patch
>
>
> VolumeSet would be responsible for managing volumes in the Datanode. Some of its functions
are:
>  # Initialize volumes on startup
>  # Provide APIs to add/ remove volumes
>  # Choose and return volume to calling service based on the volume choosing policy (currently
implemented Round Robin choosing policy)



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

---------------------------------------------------------------------
To unsubscribe, e-mail: hdfs-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: hdfs-issues-help@hadoop.apache.org


Mime
View raw message