ndimiduk commented on a change in pull request #3415:
URL: https://github.com/apache/hbase/pull/3415#discussion_r658136085
##########
File path:
hbase-balancer/src/main/java/org/apache/hadoop/hbase/master/balancer/DoubleArrayCost.java
##########
@@ -106,4 +88,32 @@ private static double getSum(double[] stats) {
}
return total;
}
+
+ /**
+ * Return the min skew of distribution
+ */
+ public static double getMinSkew(double total, double numServers) {
Review comment:
Why are the input arguments `double`? Can there be a fractional amount
of either of these quantities?
##########
File path:
hbase-balancer/src/main/java/org/apache/hadoop/hbase/master/balancer/BalancerClusterState.java
##########
@@ -301,17 +307,28 @@ public String getRack(ServerName server) {
for (int i = 0; i < regionIndexToServerIndex.length; i++) {
if (regionIndexToServerIndex[i] >= 0) {
numRegionsPerServerPerTable[regionIndexToServerIndex[i]][regionIndexToTableIndex[i]]++;
+ numRegionsPerTable[regionIndexToTableIndex[i]]++;
}
}
- numMaxRegionsPerTable = new int[numTables];
+ // Avoid repeated computation for planning
+ meanRegionsPerTable = new double[numTables];
+ regionSkewByTable = new double[numTables];
+ maxRegionSkewByTable = new double[numTables];
+ minRegionSkewByTable = new double[numTables];
+
+ for (int i = 0; i < numTables; i++) {
+ meanRegionsPerTable[i] = Double.valueOf(numRegionsPerTable[i]) /
numServers;
+ minRegionSkewByTable[i] +=
DoubleArrayCost.getMinSkew(numRegionsPerTable[i], numServers);
+ maxRegionSkewByTable[i] +=
DoubleArrayCost.getMaxSkew(numRegionsPerTable[i], numServers);
+ }
+
for (int[] aNumRegionsPerServerPerTable : numRegionsPerServerPerTable) {
- for (tableIndex = 0; tableIndex < aNumRegionsPerServerPerTable.length;
tableIndex++) {
- if (aNumRegionsPerServerPerTable[tableIndex] >
numMaxRegionsPerTable[tableIndex]) {
- numMaxRegionsPerTable[tableIndex] =
aNumRegionsPerServerPerTable[tableIndex];
- }
+ for (int tableIdx = 0; tableIdx < aNumRegionsPerServerPerTable.length;
tableIdx++) {
+ regionSkewByTable[tableIdx] +=
Math.abs(aNumRegionsPerServerPerTable[tableIdx]
+ - meanRegionsPerTable[tableIdx]);
}
- }
+ }
Review comment:
nit: white space.
##########
File path:
hbase-balancer/src/main/java/org/apache/hadoop/hbase/master/balancer/DoubleArrayCost.java
##########
@@ -106,4 +88,32 @@ private static double getSum(double[] stats) {
}
return total;
}
+
+ /**
+ * Return the min skew of distribution
+ */
+ public static double getMinSkew(double total, double numServers) {
Review comment:
Is `total` the "total number of regions in the cluster"?
##########
File path:
hbase-balancer/src/main/java/org/apache/hadoop/hbase/master/balancer/DoubleArrayCost.java
##########
@@ -106,4 +88,32 @@ private static double getSum(double[] stats) {
}
return total;
}
+
+ /**
+ * Return the min skew of distribution
+ */
+ public static double getMinSkew(double total, double numServers) {
+ double mean = total / numServers;
+ // It's possible that there aren't enough regions to go around
+ double min;
+ if (numServers > total) {
Review comment:
should this be `>=` ?
##########
File path:
hbase-balancer/src/main/java/org/apache/hadoop/hbase/master/balancer/StochasticLoadBalancer.java
##########
@@ -240,7 +240,8 @@ protected void loadConf(Configuration conf) {
curFunctionCosts = new double[costFunctions.size()];
tempFunctionCosts = new double[costFunctions.size()];
- LOG.info("Loaded config; maxSteps=" + maxSteps + ", stepsPerRegion=" +
stepsPerRegion +
+ LOG.info("Loaded config; maxSteps=" + maxSteps + " ,runMaxSteps=" +
runMaxSteps,
Review comment:
nit: white space.
##########
File path:
hbase-balancer/src/main/java/org/apache/hadoop/hbase/master/balancer/CostFunction.java
##########
@@ -89,13 +89,14 @@ protected void regionMoved(int region, int oldServer, int
newServer) {
* @return The scaled value.
*/
protected static double scale(double min, double max, double value) {
- if (max <= min || value <= min) {
+ if (max <= min || value <= min
+ || Math.abs(max - min) <= 0.01 || Math.abs(value - min) <= 0.01) {
Review comment:
values like `minCostNeedBalance` below are at 0.00 precision, so I think
we should have an epsilon of at least 0.000 precision.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
