[
https://issues.apache.org/jira/browse/HDFS-9671?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Anu Engineer updated HDFS-9671:
-------------------------------
Attachment: HDFS-9671-HDFS-1312.003.patch
Hi [~eddyxu],
Thanks for the review. I have addressed all comments in the new patch.
bq. WorkItem.java misses apache license.
Fixed.
bq. Could you rename WorkItem and WorkStatus to DiskBalancerWorkItem and
DiskBalancerWorkStatus. They are in the "o.a.h.h.s.datanode" namespace, while
the names are too general.
Fixed.
bq. IMO, private long copiedSoFar; might be named to private long bytesCopied.
Putting units into the variable makes it little bit easier for me to understand.
Fixed.
bq. Would you mind to fix comments to follow jdoc format? e.g., we do not need
- in @param errMsg - msg. Also there are a few places like @return long, that
we can probably remove.
I agree, but checkstyle and IntelliJ seems to complain about these things
unnecessarily, that is why they are there.
bq. DiskbalancerException should be DiskBalancerException.
Fixed.
bq. Are DiskBalancerException.DISK_BALANCE_NOT_ENABLED, INVALID_PLAN, ... the
only possible value for int result? Could us use a enum here?
Fixed.
bq. We might want to move lock() out of try {..}
Fixed.
bq. BlockMover should hold a FsVolumeReference when doing the IOs. So in this
sense, it might extend from Closable.You can probably directly use JDK7
try-ressource to manage references.
Thanks for the tip. Some of the later patches will make this issue clearer and
we can revisit this at that point of time, But do you see any issues with the
current code ?
bq. A few logs like LOG.info("Disk Balancer - Unable to find destination
volume. " ... should use LOG.error()?
Fixed.
bq. getPathToVolumeMap() is misleading. The key is storageID but not a path.
Fixed.
bq. LOG.info("Executing Disk balancer plan "); would you mind to also output
plan ID in the log?
Fixed.
bq. Either in VolumePair(source, target) or createWorkPlan, you might want to
check that source dose not equal to dest?
Fixed.
> 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
>
>
> Datanode side code for submit plan for diskbalancer.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
