[
https://issues.apache.org/jira/browse/HDFS-9543?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15243878#comment-15243878
]
Lei (Eddy) Xu commented on HDFS-9543:
-------------------------------------
Hi, [~anu] Thanks for the patches
A few comments:
Could you put {{getNextBlock()}} logic into a separate Iterator, and make it
{{Closable}}, which will include {{getBlockToCopy(), openPoolIters(),
getNextBlock(), closePoolIters()}}. There are a few draw backs of separating
them into different functions. 1) The states (i.e. poolIndex,) are stored
outside these functions, the caller needs maintain these states. 2)
{{poolIndex}} is never initialized and is not able be reset.
{code}
} catch (IOException e) {
item.incErrorCount();
}
{code}
Please always log the IOEs. And I think it is better to throw {{IOE}} here as
well as many other places.
{code}
private void openPoolIters();
{code}
Can it be a {{private static List<BlockIterator> openPoolIters()}}?
{code}
// Check for the max error count constraint.
if (item.getErrorCount() > getMaxError(item)) {
LOG.error("Exceeded the max error count. source {}, dest: {} " +
"error count: {}", source.getBasePath(),
dest.getBasePath(),
item.getErrorCount());
this.setExitFlag();
continue;
}
{code}
In a few such places, should we actually {{break}} the while loop? Wouldn't
{{continue}} here just generate a lot of LOGS and spend CPU cycles?
Why do you need to change {{float}} to {{double}}. In this case, wouldn't
{{float}} good enough ? I think a {{5%}} of errors are OK for these tasks.
Thanks very much!
> DiskBalancer : Add Data mover
> ------------------------------
>
> Key: HDFS-9543
> URL: https://issues.apache.org/jira/browse/HDFS-9543
> Project: Hadoop HDFS
> Issue Type: Sub-task
> Components: datanode
> Reporter: Anu Engineer
> Assignee: Anu Engineer
> Attachments: HDFS-9543-HDFS-1312.001.patch
>
>
> This patch adds the actual mover logic to the datanode.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
