hadoop-hdfs-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Anu Engineer (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (HDFS-9850) DiskBalancer : Explore removing references to FsVolumeSpi
Date Mon, 26 Sep 2016 18:06:22 GMT

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

Anu Engineer commented on HDFS-9850:

[~manojg] Thanks for updating the patch as well providing a fix for this issues. Couple of
minor comments.

In {{DiskBalancer.java}}

Changes in {{queryWorkStatus}} can be pulled into a function and invoked twice for source
and destination.

{{createWorkPlan}} -- We left the "Disk Balancer" -- in the datanode side so it is easy to
grep for error messages in datanode logs. If you don't mind can you please put it back.

{{createWorkPlan}} -- In this error messages it is easier for people to puzzle out the volume
from the path than from UUID. I think we should leave the path in the error message. Please
scan the file for removal of "Disk Balancer" from logging, also add that string in LOG messages
that are added new.

nit : {{Line : 655}} function comments say we are returning volume, but in reality we are
returning volume UUID.

{{copyBlocks#1013, 1021}} : this.setExitFlag is spurious, since we return from the function
in the next line. That flag never gets evaluated. Also would you be able to set the path of
volume in the error string instead of the UUID of the volumes.

{{FsDataSetImpl.java#getFsVolumeReference}} Do we really need to a function to FsDataSet Interface
? Cannot we do this via getVolumeReferences and using a helper function in DiskBalancer.java
itself, I don't think we should add this to FsDatasetSpi.

{{TestDiskBalancer.java#577}} -- You might want to assert that test failed along with logging
an info for the reconfig error.

{{TestDiskBalancer.java#597}} -- We verify that we are getting DiskbalancerException, but
not the payload inside the exception. Would it make sense to verify that error is indeed some
kind of volume error and string verify that verifies whether it is the source or dest ? 

> DiskBalancer : Explore removing references to FsVolumeSpi 
> ----------------------------------------------------------
>                 Key: HDFS-9850
>                 URL: https://issues.apache.org/jira/browse/HDFS-9850
>             Project: Hadoop HDFS
>          Issue Type: Sub-task
>          Components: balancer & mover
>    Affects Versions: 3.0.0-alpha2
>            Reporter: Anu Engineer
>            Assignee: Manoj Govindassamy
>         Attachments: HDFS-9850.001.patch, HDFS-9850.002.patch
> In HDFS-9671, [~arpitagarwal] commented that we should explore the possibility of removing
references to FsVolumeSpi at any point and only deal with storage ID. We are not sure if this
is possible, this JIRA is to explore if that can be done without issues.

This message was sent by Atlassian JIRA

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

View raw message