[
https://issues.apache.org/jira/browse/HDFS-9469?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15069197#comment-15069197
]
Anu Engineer commented on HDFS-9469:
------------------------------------
[~arpitagarwal] Thanks for the review comments. I will update the patch soon
based on your comments.
bq. DiskBalancerCluster.java:265: Will this be called by the executor?
No , this is called by the planner tool
bq. DiskBalancerCluster.java:346: These threads are used only for computing
plans correct? It looks like computing a plan for a node should be quite fast
once we have collected the cluster information? Perhaps we can avoid making it
multi-threaded for now.
Absolutely, I did not bench mark this, So it might be an overkill. Just so that
we are both on same page, Each Plan computation is assumed to be fully
serialized. That is each datanode's plan is single threaded only, this allows
planning for each node to proceed in different threads. I will return a config
based , default to 1 from this function, or do you just want me to hard code
single threading into planner code ?
bq. GreedyPlanner.java:103: We remove the volumes from the set here and add
them back in applyStep below. Is that necessary, looks like we can skip both
steps.
I am open to removing the removeVolume calls, but with this approach,
multi-threading is just a lock away. This is basically a treeset, so these
operations are not very costly. if the concern is about scalability the unit
tests verify that we are able to create a plan for a node with 256 disks.
Please do let me know what you think about this.
bq. GreedyPlanner.java:105: I think I need to convince myself this condition
can be hit in practice since we just recomputed idealUsed at the end of
removeSkipVolumes. GreedyPlanner.java:116: Same question as above.
Now that I think about it, I am also not able to convince myself that we will
ever hit this condition. Good catch I will remove this check from here.
bq. GreedyPlanner.java:231: Can we use MoveStep#toString instead?
Sure, will fix that.
bq. DiskBalancerTestUtil.java:36: Curious why the switch from base-2 to base-10
units as much as I dislike base-10 units
A little bit of history (Since you asked ) - I originally used base-10 since
disk manufactures like to use that and since I was writing a tool that did disk
management I thought it made sense. I also used to have a cute little print
human-readable string function.
But Nicholas pointed out to me that StringUtils have a function that does print
human readable numbers and I don't need to re-invent it. However when I tried
to use that function all my tests started failing since that function was
base-2. Hence my switch to a saner world :)
> DiskBalancer : Add Planner
> ---------------------------
>
> Key: HDFS-9469
> URL: https://issues.apache.org/jira/browse/HDFS-9469
> Project: Hadoop HDFS
> Issue Type: Sub-task
> Components: balancer & mover
> Affects Versions: 2.8.0
> Reporter: Anu Engineer
> Assignee: Anu Engineer
> Attachments: HDFS-9469-HDFS-1312.001.patch,
> HDFS-9469-HDFS-1312.002.patch, HDFS-9469-HDFS-1312.003.patch
>
>
> Disk Balancer reads the cluster data and then creates a plan for the data
> moves based on the snap-shot of the data read from the nodes. This plan is
> later submitted to data nodes for execution.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
