hadoop-hdfs-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Zhe Zhang (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (HDFS-7621) Erasure Coding: update the Balancer/Mover data migration logic
Date Fri, 29 May 2015 05:38:19 GMT

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

Zhe Zhang commented on HDFS-7621:
---------------------------------

Thanks Walter for the discussion.

bq. Changes like getBlockList() and markMovedIfGoodBlock need be done anyway
This is a good point. We do need to adjust the {{bytesReceived}} in {{getBlockList}}. Not
so sure about {{markMovedIfGoodBlock}}.

I reviewed the patch again; seems to me it does the following. Please find my comments inline
with each item.
# Renames {{PendingMove#block}} to {{reportedBlock}}. This is a clean change (+1)
# Creates and handles {{BlocksWithLocations#StripedBlockWithLocations}}. This change looks
good to me. +1 pending the below minor issues:
#* Please fix indentation below (which caused long line):
{code}
+    public StripedBlockWithLocations(BlockWithLocations blk, byte[] indices,
+                                     short dataBlockNum, short parityBlockNum) {
{code}
#* Please fix the indentation below:
{code}
+      BlockWithLocations blkWithLocs = new BlockWithLocations(block,
+          datanodeUuids, storageIDs,
+          storageTypes);
{code}
#* Not necessary to fix in this JIRA, but let's use _internal blocks_ consistently, instead
of _inner blocks_.
#* The constructor could also use a {{checkArgument}} to make sure the number of indices is
the same as the number of locations.
#* {{parityBlockNum}} is not used and should be removed.
# Creates and uses {{DBlockStriped}}. Per above discussion, it not only translates {{storage}}
to internal block index, but also carries {{dataBlockNum}} to calculate {{bytesReceived}}.
So I agree it's a reasonable change. +1 pending the below:
#* Seems like the new {{getBlockList()}} method should use a cleaner if-else flow:
{code}
if (blkLocs instanceof StripedBlockWithLocations) {
	StripedBlockWithLocations sblkLocs = (StripedBlockWithLocations) blkLocs;
	xxx
} else {
	xxx
}
{code}

Great work!

> Erasure Coding: update the Balancer/Mover data migration logic
> --------------------------------------------------------------
>
>                 Key: HDFS-7621
>                 URL: https://issues.apache.org/jira/browse/HDFS-7621
>             Project: Hadoop HDFS
>          Issue Type: Sub-task
>            Reporter: Jing Zhao
>            Assignee: Walter Su
>              Labels: HDFS-7285
>         Attachments: HDFS-7621.001.patch, HDFS-7621.002.patch, HDFS-7621.003.patch, HDFS-7621.004.patch,
HDFS-7621.005.patch, HDFS-7621.006.patch
>
>
> Currently the Balancer/Mover only considers the distribution of replicas of the same
block during data migration: the migration cannot decrease the number of racks. With EC the
Balancer and Mover should also take into account the distribution of blocks belonging to the
same block group.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Mime
View raw message