[ https://issues.apache.org/jira/browse/HDFS-9543?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15258637#comment-15258637 ]
Arpit Agarwal commented on HDFS-9543: ------------------------------------- Hi Anu, thanks for the updated patch. My comments: # In copyBlocks, the openPoolIters call should be inside the while loop, just before you open the try block. # I didn't understand the new computeDelay calculation. Your original calculation (v1 patch) looked correct, all that remained was to subtract {{timeUsed}} and check for negative result. # Thanks for the pointer to {{DirectoryScanner#scan}}. The synchronization is needed only when making changes to the in-memory block map. {{FsDatasetImpl#moveBlock}} and its callees already synchronize on the dataset when updating the map so we should remove the {{synchronized}} block in {{DiskBalancerMover#copyBlocks}}. # Nitpick: Line 818: We can also log the maximum error count value here for quick reference. Also a typo _cound_ -> _count_. # bq. I will make this comment clearer in the next update. Balancer supports a flag called -blockpools – HDFS-8890 seems to have added this. Just leaving a note that we don't do this yet. I think the -blockpools flag just restricts the balancing to a subset of blockpools. We cannot copy blocks across blockpools. Would you consider removing the TODO and filing a Jira if you think a similar flag will be useful for disk balancer. Sorry about not catching some of these on the last pass. > 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, > HDFS-9543-HDFS-1312.002.patch > > > This patch adds the actual mover logic to the datanode. -- This message was sent by Atlassian JIRA (v6.3.4#6332)