[ 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)