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):
+    public StripedBlockWithLocations(BlockWithLocations blk, byte[] indices,
+                                     short dataBlockNum, short parityBlockNum) {
#* Please fix the indentation below:
+      BlockWithLocations blkWithLocs = new BlockWithLocations(block,
+          datanodeUuids, storageIDs,
+          storageTypes);
#* 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:
if (blkLocs instanceof StripedBlockWithLocations) {
	StripedBlockWithLocations sblkLocs = (StripedBlockWithLocations) blkLocs;
} else {

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

View raw message