HBASE-25625 StochasticBalancer CostFunctions needs a better way to ev… #3011
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…aluate resource distribution
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
🎊 +1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
🎊 +1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
test updates
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
d-c-manning
reviewed
Mar 5, 2021
hbase-server/src/main/java/org/apache/hadoop/hbase/master/balancer/BaseLoadBalancer.java
Outdated
Show resolved
Hide resolved
hbase-server/src/main/java/org/apache/hadoop/hbase/master/balancer/BaseLoadBalancer.java
Show resolved
Hide resolved
hbase-server/src/main/java/org/apache/hadoop/hbase/master/balancer/BaseLoadBalancer.java
Show resolved
Hide resolved
hbase-server/src/main/java/org/apache/hadoop/hbase/master/balancer/BaseLoadBalancer.java
Show resolved
Hide resolved
| } | ||
| } | ||
| // recalculate stdev | ||
| reComputeRegionStDevPerTable(); |
There was a problem hiding this comment.
It seems like here we should only be recomputing the stdev for the affected tableIndex. I don't want to over-optimize if it makes the code less readable... but in clusters with large numbers of tables, we would be doing unnecessary work.
I'm thinking in the context of HBASE-22300 where large clusters no longer run through enough iterations of the balancer. In that case, the issue is number of regions, which will always be larger than number of tables... and likely much larger. So it may not be a major issue here, but I'm bringing it up just in case.
There was a problem hiding this comment.
Thank you for bringing up this. reComputeRegionStDevPerTable() can be optimized to only compute the impacted table index to reduce computation cost.
hbase-server/src/main/java/org/apache/hadoop/hbase/master/balancer/BaseLoadBalancer.java
Outdated
Show resolved
Hide resolved
hbase-server/src/main/java/org/apache/hadoop/hbase/master/balancer/BaseLoadBalancer.java
Outdated
Show resolved
Hide resolved
hbase-server/src/test/java/org/apache/hadoop/hbase/master/balancer/TestBaseLoadBalancer.java
Outdated
Show resolved
Hide resolved
...est/java/org/apache/hadoop/hbase/master/balancer/TestStochasticLoadBalancerLargeCluster.java
Show resolved
Hide resolved
| testWithCluster(numNodes, numRegions, numRegionsPerServer, replication, numTables, true, true); | ||
|
|
||
| // we need to capture the outlier and generate a move | ||
| testWithCluster(numNodes, numRegions, numRegionsPerServer, replication, numTables, false, false); |
There was a problem hiding this comment.
The change of code suggests that the balancer will no longer fully balance the cluster in one run... why is that? Are we making the balancer less effective, even in this test with large cluster with one outlier node? I thought this is the type of case that the fix is supposed to improve?
There was a problem hiding this comment.
This is one of the side impact. the computation gets more complicated. I will work on it.
There was a problem hiding this comment.
It turns out to be related to https://issues.apache.org/jira/browse/HBASE-25767. Will merge the fix.
|
🎊 +1 overall
This message was automatically generated. |
sunhelly
reviewed
Mar 6, 2021
hbase-server/src/main/java/org/apache/hadoop/hbase/master/balancer/BaseLoadBalancer.java
Show resolved
Hide resolved
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
The new heuristic needs to be evaluated by simulation at different scales. so this pr is currently pending the validation. 3067 is a scaled back fix for table skew only as a sub-task https://issues.apache.org/jira/browse/HBASE-25739?filter=-2 |
|
Should this or the other PR have their own JIRA then @clarax Thanks. |
|
Close the pr for now since the other pr #3067 landed and relieved most of the problem. Will revisit when the problem arises again. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
…aluate resource distribution