HBASE-25818 Move StochasticLoadBalancer to hbase-balancer module

Author: Apache9

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

@@ -22,13 +22,15 @@
 import java.util.List;
 import java.util.Map;
 import java.util.function.Predicate;
+import java.util.function.Supplier;
 import org.apache.hadoop.conf.Configuration;
 import org.apache.hadoop.hbase.HDFSBlocksDistribution;
 import org.apache.hadoop.hbase.ServerMetrics;
 import org.apache.hadoop.hbase.ServerName;
 import org.apache.hadoop.hbase.TableName;
 import org.apache.hadoop.hbase.client.RegionInfo;
 import org.apache.hadoop.hbase.client.TableDescriptor;
+import org.apache.hadoop.hbase.master.RegionPlan;
 import org.apache.yetus.audience.InterfaceAudience;
 
 /**
@@ -78,4 +80,18 @@ List<ServerName> getOnlineServersListWithPredicator(List<ServerName> servers,
    * Get a snapshot of the current assignment status.
    */
   Map<ServerName, List<RegionInfo>> getSnapShotOfAssignment(Collection<RegionInfo> regions);
+
+  /**
+   * Test whether we are in off peak hour.
+   * <p/>
+   * For peak and off peak hours we may have different cost for the same balancing operation.
+   * @return
+   */
+  boolean isOffPeakHour();
+
+  /**
+   * Record the given region plans.
+   */
+  void recordBalancerDecision(List<RegionPlan> plans, double currentCost, double initCost,
+    String initFunctionTotalCosts, Supplier<String> totalCostsPerFunc, long step);
 }

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

@@ -28,6 +28,7 @@
 import java.util.Map;
 import java.util.Objects;
 import java.util.concurrent.ThreadLocalRandom;
+import java.util.function.Supplier;
 import java.util.stream.Collectors;
 import org.apache.hadoop.conf.Configuration;
 import org.apache.hadoop.hbase.ClusterMetrics;
@@ -36,13 +37,9 @@
 import org.apache.hadoop.hbase.ServerMetrics;
 import org.apache.hadoop.hbase.ServerName;
 import org.apache.hadoop.hbase.TableName;
-import org.apache.hadoop.hbase.client.BalancerDecision;
 import org.apache.hadoop.hbase.client.RegionInfo;
 import org.apache.hadoop.hbase.master.RegionPlan;
 import org.apache.hadoop.hbase.master.balancer.BalancerClusterState.LocalityType;
-import org.apache.hadoop.hbase.namequeues.BalancerDecisionDetails;
-import org.apache.hadoop.hbase.namequeues.NamedQueueRecorder;
-import org.apache.hadoop.hbase.regionserver.compactions.OffPeakHours;
 import org.apache.hadoop.hbase.util.EnvironmentEdgeManager;
 import org.apache.hadoop.hbase.util.ReflectionUtils;
 import org.apache.yetus.audience.InterfaceAudience;
@@ -151,11 +148,6 @@ public class StochasticLoadBalancer extends BaseLoadBalancer {
   private RegionReplicaHostCostFunction regionReplicaHostCostFunction;
   private RegionReplicaRackCostFunction regionReplicaRackCostFunction;
 
-  /**
-   * Use to add balancer decision history to ring-buffer
-   */
-  NamedQueueRecorder namedQueueRecorder;
-
   /**
    * The constructor that pass a MetricsStochasticBalancer to BaseLoadBalancer to replace its
    * default MetricsBalancer
@@ -205,7 +197,7 @@ public synchronized void setConf(Configuration conf) {
     costFunctions = new ArrayList<>();
     addCostFunction(new RegionCountSkewCostFunction(conf));
     addCostFunction(new PrimaryRegionCountSkewCostFunction(conf));
-    addCostFunction(new MoveCostFunction(conf));
+    addCostFunction(new MoveCostFunction(conf, () -> provider));
     addCostFunction(localityCost);
     addCostFunction(rackLocalityCost);
     addCostFunction(new TableSkewCostFunction(conf));
@@ -221,13 +213,6 @@ public synchronized void setConf(Configuration conf) {
     curFunctionCosts= new Double[costFunctions.size()];
     tempFunctionCosts= new Double[costFunctions.size()];
 
-    boolean isBalancerDecisionRecording = getConf()
-      .getBoolean(BaseLoadBalancer.BALANCER_DECISION_BUFFER_ENABLED,
-        BaseLoadBalancer.DEFAULT_BALANCER_DECISION_BUFFER_ENABLED);
-    if (this.namedQueueRecorder == null && isBalancerDecisionRecording) {
-      this.namedQueueRecorder = NamedQueueRecorder.getInstance(getConf());
-    }
-
     LOG.info("Loaded config; maxSteps=" + maxSteps + ", stepsPerRegion=" + stepsPerRegion +
             ", maxRunningTime=" + maxRunningTime + ", isByTable=" + isByTable + ", CostFunctions=" +
             Arrays.toString(getCostFunctionNames()) + " etc.");
@@ -483,24 +468,9 @@ public synchronized List<RegionPlan> balanceTable(TableName tableName, Map<Serve
   }
 
   private void sendRegionPlansToRingBuffer(List<RegionPlan> plans, double currentCost,
-      double initCost, String initFunctionTotalCosts, long step) {
-    if (this.namedQueueRecorder != null) {
-      List<String> regionPlans = new ArrayList<>();
-      for (RegionPlan plan : plans) {
-        regionPlans.add(
-          "table: " + plan.getRegionInfo().getTable() + " , region: " + plan.getRegionName()
-            + " , source: " + plan.getSource() + " , destination: " + plan.getDestination());
-      }
-      BalancerDecision balancerDecision =
-        new BalancerDecision.Builder()
-          .setInitTotalCost(initCost)
-          .setInitialFunctionCosts(initFunctionTotalCosts)
-          .setComputedTotalCost(currentCost)
-          .setFinalFunctionCosts(totalCostsPerFunc())
-          .setComputedSteps(step)
-          .setRegionPlans(regionPlans).build();
-      namedQueueRecorder.addRecord(new BalancerDecisionDetails(balancerDecision));
-    }
+    double initCost, String initFunctionTotalCosts, long step) {
+    provider.recordBalancerDecision(plans, currentCost, initCost, initFunctionTotalCosts,
+      this::totalCostsPerFunc, step);
   }
 
   /**
@@ -837,10 +807,15 @@ static class MoveCostFunction extends CostFunction {
 
     private final float maxMovesPercent;
     private final Configuration conf;
+    // we call setConf before setClusterInfoProvider, so when we create MoveCostFunction, the
+    // provider is still null, thus here we need to provide a supplier instead of passing the
+    // provider directly.
+    private final Supplier<ClusterInfoProvider> provider;
 
-    MoveCostFunction(Configuration conf) {
+    MoveCostFunction(Configuration conf, Supplier<ClusterInfoProvider> provider) {
       super(conf);
       this.conf = conf;
+      this.provider = provider;
       // What percent of the number of regions a single run of the balancer can move.
       maxMovesPercent = conf.getFloat(MAX_MOVES_PERCENT_KEY, DEFAULT_MAX_MOVE_PERCENT);
 
@@ -853,7 +828,8 @@ static class MoveCostFunction extends CostFunction {
     protected double cost() {
       // Move cost multiplier should be the same cost or higher than the rest of the costs to ensure
       // that large benefits are need to overcome the cost of a move.
-      if (OffPeakHours.getInstance(conf).isOffPeakHour()) {
+      ClusterInfoProvider p = provider.get(); 
+      if (p != null && p.isOffPeakHour()) {
         this.setMultiplier(conf.getFloat(MOVE_COST_OFFPEAK_KEY, DEFAULT_MOVE_COST_OFFPEAK));
       } else {
         this.setMultiplier(conf.getFloat(MOVE_COST_KEY, DEFAULT_MOVE_COST));

hbase-server/src/test/java/org/apache/hadoop/hbase/master/balancer/DummyCostFunction.java renamed to hbase-balancer/src/test/java/org/apache/hadoop/hbase/master/balancer/DummyCostFunction.java

@@ -19,6 +19,7 @@
 
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertNull;
+import static org.mockito.Mockito.mock;
 
 import java.util.List;
 import java.util.Map;
@@ -48,6 +49,7 @@ public static void beforeAllTests() throws Exception {
     conf.setFloat("hbase.master.balancer.stochastic.localityCost", 0);
     loadBalancer = new StochasticLoadBalancer();
     loadBalancer.setConf(conf);
+    loadBalancer.setClusterInfoProvider(mock(ClusterInfoProvider.class));
   }
 
   protected void testWithCluster(int numNodes, int numRegions, int numRegionsPerServer,

hbase-server/src/test/java/org/apache/hadoop/hbase/master/balancer/StochasticBalancerTestBase.java renamed to hbase-balancer/src/test/java/org/apache/hadoop/hbase/master/balancer/StochasticBalancerTestBase.java

@@ -34,6 +34,7 @@
 import java.util.Map;
 import java.util.Random;
 import java.util.function.Predicate;
+import java.util.function.Supplier;
 import org.apache.hadoop.conf.Configuration;
 import org.apache.hadoop.hbase.ClusterMetrics;
 import org.apache.hadoop.hbase.HBaseClassTestRule;
@@ -47,6 +48,7 @@
 import org.apache.hadoop.hbase.client.RegionInfoBuilder;
 import org.apache.hadoop.hbase.client.TableDescriptor;
 import org.apache.hadoop.hbase.client.TableDescriptorBuilder;
+import org.apache.hadoop.hbase.master.RegionPlan;
 import org.apache.hadoop.hbase.testclassification.MasterTests;
 import org.apache.hadoop.hbase.testclassification.SmallTests;
 import org.apache.hadoop.hbase.util.Bytes;
@@ -136,6 +138,17 @@ public List<ServerName> getOnlineServersListWithPredicator(List<ServerName> serv
       public int getNumberOfTables() {
         return 0;
       }
+
+      @Override
+      public boolean isOffPeakHour() {
+        return false;
+      }
+
+      @Override
+      public void recordBalancerDecision(List<RegionPlan> plans, double currentCost,
+        double initCost, String initFunctionTotalCosts, Supplier<String> totalCostsPerFunc,
+        long step) {
+      }
     });
   }
 

hbase-server/src/test/java/org/apache/hadoop/hbase/master/balancer/StochasticBalancerTestBase2.java renamed to hbase-balancer/src/test/java/org/apache/hadoop/hbase/master/balancer/StochasticBalancerTestBase2.java

@@ -32,7 +32,6 @@
 import java.util.Map;
 import java.util.Queue;
 import java.util.Set;
-import java.util.TimeZone;
 import java.util.TreeMap;
 import org.apache.hadoop.conf.Configuration;
 import org.apache.hadoop.hbase.ClusterMetrics;
@@ -45,13 +44,11 @@
 import org.apache.hadoop.hbase.Size;
 import org.apache.hadoop.hbase.TableName;
 import org.apache.hadoop.hbase.client.RegionInfo;
-import org.apache.hadoop.hbase.master.MockNoopMasterServices;
 import org.apache.hadoop.hbase.master.RegionPlan;
 import org.apache.hadoop.hbase.master.balancer.StochasticLoadBalancer.ServerLocalityCostFunction;
 import org.apache.hadoop.hbase.testclassification.MasterTests;
 import org.apache.hadoop.hbase.testclassification.MediumTests;
 import org.apache.hadoop.hbase.util.Bytes;
-import org.apache.hadoop.hbase.util.EnvironmentEdgeManager;
 import org.junit.ClassRule;
 import org.junit.Test;
 import org.junit.experimental.categories.Category;
@@ -196,7 +193,6 @@ public void testCPRequestCost() {
     assertEquals(2, regionsMoveFromServerA.size());
     assertEquals(2, targetServers.size());
     assertTrue(regionsOnServerA.containsAll(regionsMoveFromServerA));
-    assertNull(loadBalancer.namedQueueRecorder);
     // reset config
     conf.setFloat("hbase.master.balancer.stochastic.cpRequestCost", 5f);
     loadBalancer.setConf(conf);
@@ -233,7 +229,6 @@ public void testKeepRegionLoad() throws Exception {
     String regionNameAsString = RegionInfo.getRegionNameAsString(Bytes.toBytes(REGION_KEY));
     assertTrue(loadBalancer.loads.get(regionNameAsString) != null);
     assertTrue(loadBalancer.loads.get(regionNameAsString).size() == 15);
-    assertNull(loadBalancer.namedQueueRecorder);
 
     Queue<BalancerRegionLoad> loads = loadBalancer.loads.get(regionNameAsString);
     int i = 0;
@@ -273,8 +268,6 @@ public void testNeedBalance() {
 
   @Test
   public void testLocalityCost() throws Exception {
-    Configuration conf = HBaseConfiguration.create();
-    MockNoopMasterServices master = new MockNoopMasterServices();
     StochasticLoadBalancer.CostFunction
         costFunction = new ServerLocalityCostFunction(conf);
 
@@ -286,40 +279,33 @@ public void testLocalityCost() throws Exception {
       double expected = 1 - expectedLocalities[test];
       assertEquals(expected, cost, 0.001);
     }
-    assertNull(loadBalancer.namedQueueRecorder);
   }
 
   @Test
   public void testMoveCostMultiplier() throws Exception {
     Configuration conf = HBaseConfiguration.create();
-    StochasticLoadBalancer.CostFunction
-      costFunction = new StochasticLoadBalancer.MoveCostFunction(conf);
+    ClusterInfoProvider provider = mock(ClusterInfoProvider.class);
+    StochasticLoadBalancer.CostFunction costFunction =
+      new StochasticLoadBalancer.MoveCostFunction(conf, () -> provider);
+    when(provider.isOffPeakHour()).thenReturn(false);
     BalancerClusterState cluster = mockCluster(clusterStateMocks[0]);
     costFunction.init(cluster);
     costFunction.cost();
     assertEquals(StochasticLoadBalancer.MoveCostFunction.DEFAULT_MOVE_COST,
       costFunction.getMultiplier(), 0.01);
 
     // In offpeak hours, the multiplier of move cost should be lower
-    conf.setInt("hbase.offpeak.start.hour",0);
-    conf.setInt("hbase.offpeak.end.hour",23);
-    // Set a fixed time which hour is 15, so it will always in offpeak
-    // See HBASE-24898 for more info of the calculation here
-    long deltaFor15 = TimeZone.getDefault().getRawOffset() - 28800000;
-    long timeFor15 = 1597907081000L - deltaFor15;
-    EnvironmentEdgeManager.injectEdge(() -> timeFor15);
-    costFunction = new StochasticLoadBalancer.MoveCostFunction(conf);
-    costFunction.init(cluster);
+    when(provider.isOffPeakHour()).thenReturn(true);
     costFunction.cost();
-    assertEquals(StochasticLoadBalancer.MoveCostFunction.DEFAULT_MOVE_COST_OFFPEAK
-      , costFunction.getMultiplier(), 0.01);
+    assertEquals(StochasticLoadBalancer.MoveCostFunction.DEFAULT_MOVE_COST_OFFPEAK,
+      costFunction.getMultiplier(), 0.01);
   }
 
   @Test
   public void testMoveCost() throws Exception {
     Configuration conf = HBaseConfiguration.create();
-    StochasticLoadBalancer.CostFunction
-        costFunction = new StochasticLoadBalancer.MoveCostFunction(conf);
+    StochasticLoadBalancer.CostFunction costFunction =
+      new StochasticLoadBalancer.MoveCostFunction(conf, () -> null);
     for (int[] mockCluster : clusterStateMocks) {
       BalancerClusterState cluster = mockCluster(mockCluster);
       costFunction.init(cluster);