hadoop-hdfs-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Anu Engineer (JIRA)" <j...@apache.org>
Subject [jira] [Updated] (HDFS-9671) DiskBalancer : SubmitPlan implementation
Date Fri, 05 Feb 2016 00:49:39 GMT

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

Mime
View raw message