diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/balancer/ServerAndLoad.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/balancer/ServerAndLoad.java index cceaf876299..25086d69eca 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/balancer/ServerAndLoad.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/balancer/ServerAndLoad.java @@ -65,4 +65,9 @@ class ServerAndLoad implements Comparable, Serializable { } return false; } + + @Override + public String toString() { + return "server=" + sn + " , load=" + load; + } } diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/balancer/StochasticLoadBalancer.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/balancer/StochasticLoadBalancer.java index 60c8b672aad..f3c756c9066 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/balancer/StochasticLoadBalancer.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/balancer/StochasticLoadBalancer.java @@ -334,26 +334,34 @@ public class StochasticLoadBalancer extends BaseLoadBalancer { for (CostFunction c : costFunctions) { float multiplier = c.getMultiplier(); if (multiplier <= 0) { + if (LOG.isTraceEnabled()) { + LOG.trace(c.getClass().getSimpleName() + " not needed because multiplier is <= 0"); + } continue; } if (!c.isNeeded()) { - LOG.debug(c.getClass().getName() + " indicated that its cost should not be considered"); + if (LOG.isTraceEnabled()) { + LOG.trace(c.getClass().getSimpleName() + " not needed"); + } continue; } sumMultiplier += multiplier; total += c.cost() * multiplier; } - if (total <= 0 || sumMultiplier <= 0 - || (sumMultiplier > 0 && (total / sumMultiplier) < minCostNeedBalance)) { - final String loadBalanceTarget = - isByTable ? String.format("table (%s)", tableName) : "cluster"; - LOG.info(String.format("Skipping load balancing because the %s is balanced. Total cost: %s, " - + "Sum multiplier: %s, Minimum cost needed for balance: %s", loadBalanceTarget, total, - sumMultiplier, minCostNeedBalance)); - return false; + boolean balanced = total <= 0 || sumMultiplier <= 0 || + (sumMultiplier > 0 && (total / sumMultiplier) < minCostNeedBalance); + if (LOG.isDebugEnabled()) { + LOG.debug( + (balanced ? "Skipping load balancing because balanced" : "We need to load balance") + + " " + (isByTable ? String.format("table (%s)", tableName) : "cluster") + + "; total cost=" + total + ", sum multiplier=" + sumMultiplier + "; cost/multiplier to " + + "need a balance is " + minCostNeedBalance); + if (LOG.isTraceEnabled()) { + LOG.trace("Balance decision detailed function costs=" + functionCost()); + } } - return true; + return !balanced; } @Override @@ -1207,16 +1215,27 @@ public class StochasticLoadBalancer extends BaseLoadBalancer { this.setMultiplier(conf.getFloat(REGION_COUNT_SKEW_COST_KEY, DEFAULT_REGION_COUNT_SKEW_COST)); } + @Override + void init(Cluster cluster) { + super.init(cluster); + LOG.debug(getClass().getSimpleName() + " sees a total of " + cluster.numServers + + " servers and " + cluster.numRegions + " regions."); + if (LOG.isTraceEnabled()) { + for (int i =0; i < cluster.numServers; i++) { + LOG.trace(getClass().getSimpleName() + " sees server '" + cluster.servers[i] + + "' has " + cluster.regionsPerServer[i].length + " regions"); + } + } + } + @Override protected double cost() { if (stats == null || stats.length != cluster.numServers) { stats = new double[cluster.numServers]; } - for (int i =0; i < cluster.numServers; i++) { stats[i] = cluster.regionsPerServer[i].length; } - return costFromArray(stats); } } diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/balancer/BalancerTestBase.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/balancer/BalancerTestBase.java index adeebd22bde..bc0ae82a22c 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/balancer/BalancerTestBase.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/balancer/BalancerTestBase.java @@ -202,9 +202,11 @@ public class BalancerTestBase { int max = numRegions % numServers == 0 ? min : min + 1; for (ServerAndLoad server : servers) { - assertTrue(server.getLoad() >= 0); - assertTrue(server.getLoad() <= max); - assertTrue(server.getLoad() >= min); + assertTrue("All servers should have a positive load. " + server, server.getLoad() >= 0); + assertTrue("All servers should have load no more than " + max + ". " + server, + server.getLoad() <= max); + assertTrue("All servers should have load no less than " + min + ". " + server, + server.getLoad() >= min); } } @@ -434,7 +436,7 @@ public class BalancerTestBase { loadBalancer.setRackManager(rackManager); // Run the balancer. List plans = loadBalancer.balanceCluster(serverMap); - assertNotNull(plans); + assertNotNull("Initial cluster balance should produce plans.", plans); // Check to see that this actually got to a stable place. if (assertFullyBalanced || assertFullyBalancedForReplicas) { @@ -447,7 +449,8 @@ public class BalancerTestBase { if (assertFullyBalanced) { assertClusterAsBalanced(balancedCluster); List secondPlans = loadBalancer.balanceCluster(serverMap); - assertNull(secondPlans); + assertNull("Given a requirement to be fully balanced, second attempt at plans should " + + "produce none.", secondPlans); } if (assertFullyBalancedForReplicas) {