Cache costs so we don't waste so much time recomputing things that havent changed, written against hbase 1.2. This patch was written to be minimal diff as opposed to the most ideal/clean solution. It improved performance on our clusters by about 10x.

0001-Cache-costs-so-we-don-t-waste-so-much-time-recomputi.patch

From bc5de4ffd41b7dbb88a51ad255af6b617e9282f8 Mon Sep 17 00:00:00 2001
From: Bryan Beaudreault <bbeaudreault@hubspot.com>
Date: Thu, 6 May 2021 10:39:05 -0400
Subject: [PATCH] Cache costs so we don't waste so much time recomputing things
 that havent changed

---
 .../master/balancer/BaseLoadBalancer.java     | 22 +++++++
 .../balancer/StochasticLoadBalancer.java      | 65 ++++++++++++-------
 .../balancer/TestStochasticLoadBalancer.java  |  2 +-
 3 files changed, 65 insertions(+), 24 deletions(-)

diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/balancer/BaseLoadBalancer.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/balancer/BaseLoadBalancer.java
index 287db0917..e7bb5a171 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/balancer/BaseLoadBalancer.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/balancer/BaseLoadBalancer.java
@@ -132,6 +132,7 @@ public abstract class BaseLoadBalancer implements LoadBalancer {
     int[] regionIndexToPrimaryIndex;   //regionIndex -> regionIndex of the primary
     boolean hasRegionReplicas = false;   //whether there is regions with replicas
     double[] serverIndexToInstanceSizeScalar; //serverIndex -> serverInstanceSizeScalar
+    double[][] regionLoadCurrentCosts; // serverIndex -> costFunctionIndex -> cost
 
     Integer[] serverIndicesSortedByRegionCount;
     Integer[] serverIndicesSortedByLocality;
@@ -261,6 +262,7 @@ public abstract class BaseLoadBalancer implements LoadBalancer {
       primariesOfRegionsPerHost = new int[numHosts][];
       primariesOfRegionsPerRack = new int[numRacks][];
       serverIndexToInstanceSizeScalar = new double[numServers];
+      regionLoadCurrentCosts = new double[numServers][];
 
       int tableIndex = 0, regionIndex = 0, regionPerServerIndex = 0;
 
@@ -832,6 +834,7 @@ public abstract class BaseLoadBalancer implements LoadBalancer {
           AssignRegionAction ar = (AssignRegionAction) action;
           regionsPerServer[ar.server] = addRegion(regionsPerServer[ar.server], ar.region);
           regionMoved(ar.region, -1, ar.server);
+          unCacheRegionLoadCosts(-1, ar.server);
           break;
         case MOVE_REGION:
           assert action instanceof MoveRegionAction : action.getClass();
@@ -839,6 +842,7 @@ public abstract class BaseLoadBalancer implements LoadBalancer {
           regionsPerServer[mra.fromServer] = removeRegion(regionsPerServer[mra.fromServer], mra.region);
           regionsPerServer[mra.toServer] = addRegion(regionsPerServer[mra.toServer], mra.region);
           regionMoved(mra.region, mra.fromServer, mra.toServer);
+          unCacheRegionLoadCosts(mra.fromServer, mra.toServer);
           break;
         case SWAP_REGIONS:
           assert action instanceof SwapRegionsAction : action.getClass();
@@ -847,6 +851,7 @@ public abstract class BaseLoadBalancer implements LoadBalancer {
           regionsPerServer[a.toServer] = replaceRegion(regionsPerServer[a.toServer], a.toRegion, a.fromRegion);
           regionMoved(a.fromRegion, a.fromServer, a.toServer);
           regionMoved(a.toRegion, a.toServer, a.fromServer);
+          unCacheRegionLoadCosts(a.fromServer, a.toServer);
           break;
         default:
           throw new RuntimeException("Uknown action:" + action.type);
@@ -921,6 +926,23 @@ public abstract class BaseLoadBalancer implements LoadBalancer {
       doAction(new AssignRegionAction(region, server));
     }
 
+    private void unCacheRegionLoadCosts(int oldServer, int newServer){
+      unCacheRegionLoadCost(oldServer);
+      unCacheRegionLoadCost(newServer);
+    }
+
+    private void unCacheRegionLoadCost(int serverIdx) {
+      if (serverIdx < 0 || serverIdx >= regionLoadCurrentCosts.length) {
+        return;
+      }
+
+      double[] serverRegionLoadCosts = regionLoadCurrentCosts[serverIdx];
+      if (serverRegionLoadCosts == null) {
+        return;
+      }
+      Arrays.fill(serverRegionLoadCosts, Double.NaN);
+    }
+
     void regionMoved(int region, int oldServer, int newServer) {
       if (oldServer == newServer) {
         return;  // do nothing, else we over-decrement numMovedRegions
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 7e6790b18..9a4064769 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
@@ -214,10 +214,10 @@ public class StochasticLoadBalancer extends BaseLoadBalancer {
     candidateGenerators = filterGeneratorsFromConfig(conf, candidateGenerators);
 
     regionLoadFunctions = new CostFromRegionLoadFunction[]{
-        new ReadRequestCostFunction(conf),
-        new WriteRequestCostFunction(conf),
-        new MemstoreSizeCostFunction(conf),
-        new StoreFileCostFunction(conf)
+        new ReadRequestCostFunction(conf, 0),
+        new WriteRequestCostFunction(conf, 1),
+        new MemstoreSizeCostFunction(conf, 2),
+        new StoreFileCostFunction(conf, 3)
     };
 
     regionReplicaHostCostFunction = new RegionReplicaHostCostFunction(conf);
@@ -363,7 +363,7 @@ public class StochasticLoadBalancer extends BaseLoadBalancer {
     long maxMovedRegions = Math.round(Math.ceil(this.maxMovePercent * cluster.numRegions));
     boolean doneBalancing = (maxMovedRegions == 0);
 
-    logClusterCosts("before");
+    logClusterCosts(cluster, "before");
 
     // Perform a stochastic walk to see if we can find a load balancer plan
     // that improves the state of the cluster. The balancer is forced to 'think' for maxRunningTime,
@@ -401,7 +401,7 @@ public class StochasticLoadBalancer extends BaseLoadBalancer {
       clusterPlan = walk.getLowestCostClusterPlan().getClusterPlan();
       logLowestCostClusterPlanCosts(walk, "after");
     } else {
-      logClusterCosts("after");
+      logClusterCosts(cluster, "after");
     }
 
     logCostFunctionAndGeneratorTimes();
@@ -651,6 +651,7 @@ public class StochasticLoadBalancer extends BaseLoadBalancer {
   private void logClusterCosts(String when) {
     if (LOG.isDebugEnabled()) {
       LOG.debug("Cluster costs " + when + " balancer:");
+      initCosts(cluster);
       printCostBreakdown();
     }
   }
@@ -820,6 +821,10 @@ public class StochasticLoadBalancer extends BaseLoadBalancer {
     for (CostFunction c : costFunctions) {
       c.init(cluster);
     }
+    for (int i = 0; i < cluster.numServers; i++) {
+      cluster.regionLoadCurrentCosts[i] = new double[regionLoadFunctions.length];
+      Arrays.fill(cluster.regionLoadCurrentCosts[i], Double.NaN);
+    }
   }
 
   protected void updateCostsWithAction(Cluster cluster, Action action) {
@@ -2423,12 +2428,14 @@ public class StochasticLoadBalancer extends BaseLoadBalancer {
    */
   abstract static class CostFromRegionLoadFunction extends CostFunction {
 
+    private final int regionLoadCostIdx;
     private ClusterStatus clusterStatus = null;
     private Map<String, Deque<RegionLoad>> loads = null;
     private double[] stats = null;
 
-    CostFromRegionLoadFunction(Configuration conf) {
+    CostFromRegionLoadFunction(Configuration conf, int regionLoadCostIdx) {
       super(conf);
+      this.regionLoadCostIdx = regionLoadCostIdx;
     }
 
     void setClusterStatus(ClusterStatus status) {
@@ -2457,14 +2464,12 @@ public class StochasticLoadBalancer extends BaseLoadBalancer {
           //Cost this server has from RegionLoad
           long cost = 0;
 
-          // for every region on this server get the rl
-          for (int regionIndex : cluster.regionsPerServer[i]) {
-            Collection<RegionLoad> regionLoadList = cluster.regionLoads[regionIndex];
-
-            // Now if we found a region load get the type of cost that was requested.
-            if (regionLoadList != null) {
-              cost += getRegionLoadCost(regionLoadList);
-            }
+          double cachedCost = cluster.regionLoadCurrentCosts[i][regionLoadCostIdx];
+          if (cachedCost == cachedCost) { // NaN check, which we use to null out these cached values in cluster.doAction
+            cost = (long) cachedCost;
+          } else {
+            cost = computeRegionLoadCostForServer(i);
+            cluster.regionLoadCurrentCosts[i][regionLoadCostIdx] = cost;
           }
 
           // Add the total cost to the stats.
@@ -2476,6 +2481,20 @@ public class StochasticLoadBalancer extends BaseLoadBalancer {
       return costFromArray(stats);
     }
 
+    private long computeRegionLoadCostForServer(int serverIdx) {
+      long cost = 0;
+      // for every region on this server get the rl
+      for (int regionIndex : cluster.regionsPerServer[serverIdx]) {
+        Collection<RegionLoad> regionLoadList = cluster.regionLoads[regionIndex];
+
+        // Now if we found a region load get the type of cost that was requested.
+        if (regionLoadList != null) {
+          cost += getRegionLoadCost(regionLoadList);
+        }
+      }
+      return cost;
+    }
+
     protected double getRegionLoadCost(Collection<RegionLoad> regionLoadList) {
       double cost = 0;
 
@@ -2526,8 +2545,8 @@ public class StochasticLoadBalancer extends BaseLoadBalancer {
         "hbase.master.balancer.stochastic.readRequestCost";
     private static final float DEFAULT_READ_REQUEST_COST = 5;
 
-    ReadRequestCostFunction(Configuration conf) {
-      super(conf);
+    ReadRequestCostFunction(Configuration conf, int regionLoadCostIdx) {
+      super(conf, regionLoadCostIdx);
       this.setMultiplier(conf.getFloat(READ_REQUEST_COST_KEY, DEFAULT_READ_REQUEST_COST));
     }
 
@@ -2548,8 +2567,8 @@ public class StochasticLoadBalancer extends BaseLoadBalancer {
         "hbase.master.balancer.stochastic.writeRequestCost";
     private static final float DEFAULT_WRITE_REQUEST_COST = 5;
 
-    WriteRequestCostFunction(Configuration conf) {
-      super(conf);
+    WriteRequestCostFunction(Configuration conf, int regionLoadCostIdx) {
+      super(conf, regionLoadCostIdx);
       this.setMultiplier(conf.getFloat(WRITE_REQUEST_COST_KEY, DEFAULT_WRITE_REQUEST_COST));
     }
 
@@ -2726,8 +2745,8 @@ public class StochasticLoadBalancer extends BaseLoadBalancer {
         "hbase.master.balancer.stochastic.memstoreSizeCost";
     private static final float DEFAULT_MEMSTORE_SIZE_COST = 5;
 
-    MemstoreSizeCostFunction(Configuration conf) {
-      super(conf);
+    MemstoreSizeCostFunction(Configuration conf, int regionLoadCostIdx) {
+      super(conf, regionLoadCostIdx);
       this.setMultiplier(conf.getFloat(MEMSTORE_SIZE_COST_KEY, DEFAULT_MEMSTORE_SIZE_COST));
     }
 
@@ -2747,8 +2766,8 @@ public class StochasticLoadBalancer extends BaseLoadBalancer {
         "hbase.master.balancer.stochastic.storefileSizeCost";
     private static final float DEFAULT_STOREFILE_SIZE_COST = 5;
 
-    StoreFileCostFunction(Configuration conf) {
-      super(conf);
+    StoreFileCostFunction(Configuration conf, int regionLoadCostIdx) {
+      super(conf, regionLoadCostIdx);
       this.setMultiplier(conf.getFloat(STOREFILE_SIZE_COST_KEY, DEFAULT_STOREFILE_SIZE_COST));
     }
 
diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/balancer/TestStochasticLoadBalancer.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/balancer/TestStochasticLoadBalancer.java
index 04ffb8fc6..b2d5dcaae 100644
--- a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/balancer/TestStochasticLoadBalancer.java
+++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/balancer/TestStochasticLoadBalancer.java
@@ -346,7 +346,7 @@ public class TestStochasticLoadBalancer extends BalancerTestBase {
   public void testCostFromArray() {
     Configuration conf = HBaseConfiguration.create();
     StochasticLoadBalancer.CostFromRegionLoadFunction
-        costFunction = new StochasticLoadBalancer.MemstoreSizeCostFunction(conf);
+        costFunction = new StochasticLoadBalancer.MemstoreSizeCostFunction(conf, 0);
     costFunction.init(mockCluster(new int[]{0, 0, 0, 0, 1}));
 
     double[] statOne = new double[100];
-- 
2.24.3 (Apple Git-128)