hadoop-hdfs-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Lei (Eddy) Xu (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (HDFS-9671) DiskBalancer : SubmitPlan implementation
Date Tue, 02 Feb 2016 22:38:39 GMT

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

Mime
View raw message