HBASE-24138 log more details about balancer decisions for StochasticLoadBalancer (#1455)

- at DEBUG log messages about RegionCountSkewCostFunction region/server totals
- at DEBUG log messages about the decision to balance or not with total costs
- at TRACE log messages about region count on each server RegionCountSkewCostFunction sees
- at TRACE log message with the individual cost functions used in the decision to balance or not

Signed-off-by: Viraj Jasani <vjasani@apache.org>
(cherry picked from commit 2d78a28)

Author: busbey

hbase-server/src/main/java/org/apache/hadoop/hbase/master/balancer/ServerAndLoad.java

@@ -65,4 +65,9 @@ public boolean equals(Object o) {
     }
     return false;
   }
+
+  @Override
+  public String toString() {
+    return "server=" + sn + " , load=" + load;
+  }
 }

hbase-server/src/main/java/org/apache/hadoop/hbase/master/balancer/StochasticLoadBalancer.java

@@ -334,26 +334,34 @@ protected boolean needsBalance(Cluster cluster) {
     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 @@ static class RegionCountSkewCostFunction extends CostFunction {
       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);
     }
   }

hbase-server/src/test/java/org/apache/hadoop/hbase/master/balancer/BalancerTestBase.java

@@ -202,9 +202,11 @@ public void assertClusterAsBalanced(List<ServerAndLoad> servers) {
     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 @@ protected void testWithCluster(Map<ServerName, List<HRegionInfo>> serverMap,
     loadBalancer.setRackManager(rackManager);
     // Run the balancer.
     List<RegionPlan> 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 @@ protected void testWithCluster(Map<ServerName, List<HRegionInfo>> serverMap,
       if (assertFullyBalanced) {
         assertClusterAsBalanced(balancedCluster);
         List<RegionPlan> secondPlans =  loadBalancer.balanceCluster(serverMap);
-        assertNull(secondPlans);
+        assertNull("Given a requirement to be fully balanced, second attempt at plans should " +
+            "produce none.", secondPlans);
       }
 
       if (assertFullyBalancedForReplicas) {