[ https://issues.apache.org/jira/browse/HDFS-9671?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15129207#comment-15129207 ]
Lei (Eddy) Xu commented on HDFS-9671: ------------------------------------- Hey, [~anu] Thanks for providing this patch. Would you mind to address the following patches? * {{WorkItem.java}} misses apache license. * 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. * 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. * 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. * {{DiskbalancerException}} should be {{DiskBalancerException}}. * Are {{DiskBalancerException.DISK_BALANCE_NOT_ENABLED, INVALID_PLAN, ...}} the only possible value for {{int result}}? Could us use a {{enum}} here?j * {code} try { lock.lock(); {code} We might want to move {{lock()}} out of {{try \{..\}}} * {{BlockMover}} should hold a {{FsVolumeReference}} when doing the IOs. So in this sense, it might extend from {{Closable}}. * {code} try { 312 synchronized (this.dataset) { 313 references = this.dataset.getFsVolumeReferences(); .... references.close(); {code} You can probably directly use JDK7 {{try-ressource}} to manage {{references}}. * A few logs like {{LOG.info("Disk Balancer - Unable to find destination volume. " ...}} should use {{LOG.error()}}? * {{getPathToVolumeMap()}} is misleading. The key is {{storageID}} but not a path. * {{LOG.info("Executing Disk balancer plan ");}} would you mind to also output plan ID in the log? * Either in {{VolumePair(source, target)}} or {{createWorkPlan}}, you might want to check that {{soruce}} dose not equal to {{dest}}? Again, thanks for the work, [~anu]! > 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 > > > Datanode side code for submit plan for diskbalancer. -- This message was sent by Atlassian JIRA (v6.3.4#6332)