[
https://issues.apache.org/jira/browse/HDFS-9671?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15149570#comment-15149570
]
Arpit Agarwal edited comment on HDFS-9671 at 2/17/16 12:29 AM:
---------------------------------------------------------------
Hi [~anu], my apologies for the late review.
# {{DiskBalancer#shutdown}} should wait for the executor to terminate following
the convention in {{DataNode#shutdown}}. e.g. {{DirectoryScanner#shutdown}}.
# Related to the volume references mentioned by [~eddyxu], it looks safer to
use storageuuids in DiskBalancer and never cache volume references. I assume
you'd call into the dataset to perform the actual move at some point. We could
defer the storageuuid-->volume resolution to the dataset until then.
# {{blockMover.setExitFlag()}} looks unnecessary. {{scheduler.shutdownNow()}}
will cancel it as long as the BlockMover implementation does not swallow
InterruptedException. I could have failed to think of some case here.
# A future improvement - can we reclaim the thread once the executor has
finished executing a plan? This reminds me to fix an extra thread I recently
added to the NN. :)
Couple of nitpicks:
# Spurious edits to the license header in DiskBalancerException.java?
# {{verifyTimeStamp}} - 24 hours limit should be a constant and probably
documented somewhere. Also we can use {{TimeUnit.DAYS.toMillis}}. Ok with
handling in a subsequent Jira.
was (Author: arpitagarwal):
Hi [~anu], my apologies for the late review.
# {{DiskBalancer#shutdown}} should wait for the executor to terminate following
the convention in {{DataNode#shutdown}}. e.g. {{DirectoryScanner#shutdown}}.
# Related to the volume references mentioned by [~eddyxu], it looks safer to
use storageuuids in DiskBalancer and never cache volume references. I assume
you'd call into the dataset to perform the actual move at some point we could
consider defering the storageuuid-->volume resolution to the dataset until then?
# {{blockMover.setExitFlag()}} looks unnecessary. {{scheduler.shutdownNow()}}
will cancel it as long as the BlockMover implementation does not swallow
InterruptedException. I could have failed to think of some case here.
# A future improvement - can we reclaim the thread once the executor has
finished executing a plan? This reminds me to fix an extra thread I recently
added to the NN. :)
Couple of nitpicks:
# Spurious edits to the license header in DiskBalancerException.java?
# {{verifyTimeStamp}} - 24 hours limit should be a constant and probably
documented somewhere. Also we can use {{TimeUnit.DAYS.toMillis}}. Ok with
handling in a subsequent Jira.
> DiskBalancer : SubmitPlan implementation
> -----------------------------------------
>
> Key: HDFS-9671
> URL: https://issues.apache.org/jira/browse/HDFS-9671
> Project: Hadoop HDFS
> Issue Type: Sub-task
> Components: balancer & mover
> Affects Versions: HDFS-1312
> Reporter: Anu Engineer
> Assignee: Anu Engineer
> Attachments: HDFS-9671-HDFS-1312.001.patch,
> HDFS-9671-HDFS-1312.002.patch, HDFS-9671-HDFS-1312.003.patch,
> HDFS-9671-HDFS-1312.004.patch, HDFS-9671-HDFS-1312.005.patch
>
>
> Datanode side code for submit plan for diskbalancer.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
