You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hbase.apache.org by bu...@apache.org on 2020/04/09 05:31:52 UTC

[hbase] branch branch-1 updated: HBASE-24138 log more details about balancer decisions for StochasticLoadBalancer (#1455)

This is an automated email from the ASF dual-hosted git repository.

busbey pushed a commit to branch branch-1
in repository https://gitbox.apache.org/repos/asf/hbase.git


The following commit(s) were added to refs/heads/branch-1 by this push:
     new 4b94f2e  HBASE-24138 log more details about balancer decisions for StochasticLoadBalancer (#1455)
4b94f2e is described below

commit 4b94f2ed417e1776dd736d6903e9ba66dc1b08a0
Author: Sean Busbey <bu...@apache.org>
AuthorDate: Wed Apr 8 14:03:33 2020 -0500

    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 <vj...@apache.org>
    (cherry picked from commit 2d78a286b682b34e8a718c521e265d58536f1e5e)
---
 .../hbase/master/balancer/ServerAndLoad.java       |  5 +++
 .../master/balancer/StochasticLoadBalancer.java    | 43 ++++++++++++++++------
 .../hbase/master/balancer/BalancerTestBase.java    | 13 ++++---
 3 files changed, 44 insertions(+), 17 deletions(-)

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 cceaf87..25086d6 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<ServerAndLoad>, 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 60c8b67..f3c756c 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
@@ -1208,15 +1216,26 @@ public class StochasticLoadBalancer extends BaseLoadBalancer {
     }
 
     @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 adeebd2..bc0ae82 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<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 @@ public class BalancerTestBase {
       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) {