[hbase] branch branch-2.3 updated: HBASE-25726 MoveCostFunction is not included in the list of cost functions for StochasticLoadBalancer (#3116)

[ap...@apache.org]

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

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


The following commit(s) were added to refs/heads/branch-2.3 by this push:
     new 3381f3a  HBASE-25726 MoveCostFunction is not included in the list of cost functions for StochasticLoadBalancer (#3116)
3381f3a is described below

commit 3381f3a7f838841797ab70f3271b2dfa7942a58c
Author: d-c-manning <67...@users.noreply.github.com>
AuthorDate: Fri Apr 2 09:22:13 2021 -0700

    HBASE-25726 MoveCostFunction is not included in the list of cost functions for StochasticLoadBalancer (#3116)
    
    Signed-off-by: Andrew Purtell <ap...@apache.org>
---
 .../master/balancer/StochasticLoadBalancer.java    |  4 +++
 .../balancer/TestStochasticLoadBalancer.java       | 39 ++++++++++++++++++----
 2 files changed, 37 insertions(+), 6 deletions(-)

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 1283dad..dd4890d 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
@@ -857,6 +857,10 @@ public class StochasticLoadBalancer extends BaseLoadBalancer {
       this.conf = conf;
       // 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);
+
+      // Initialize the multiplier so that addCostFunction will add this cost function.
+      // It may change during later evaluations, due to OffPeakHours.
+      this.setMultiplier(conf.getFloat(MOVE_COST_KEY, DEFAULT_MOVE_COST));
     }
 
     @Override
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 f9e6a9e..4416b77 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
@@ -50,6 +50,7 @@ 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.apache.hbase.thirdparty.org.apache.commons.collections4.CollectionUtils;
 import org.junit.ClassRule;
 import org.junit.Test;
 import org.junit.experimental.categories.Category;
@@ -415,13 +416,39 @@ public class TestStochasticLoadBalancer extends BalancerTestBase {
 
   @Test
   public void testAdditionalCostFunction() {
-    conf.set(StochasticLoadBalancer.COST_FUNCTIONS_COST_FUNCTIONS_KEY,
-            DummyCostFunction.class.getName());
+    try {
+      conf.set(StochasticLoadBalancer.COST_FUNCTIONS_COST_FUNCTIONS_KEY,
+        DummyCostFunction.class.getName());
 
-    loadBalancer.setConf(conf);
-    assertTrue(Arrays.
-            asList(loadBalancer.getCostFunctionNames()).
-            contains(DummyCostFunction.class.getSimpleName()));
+      loadBalancer.setConf(conf);
+      assertTrue(Arrays.
+        asList(loadBalancer.getCostFunctionNames()).
+        contains(DummyCostFunction.class.getSimpleName()));
+    } finally {
+      conf.unset(StochasticLoadBalancer.COST_FUNCTIONS_COST_FUNCTIONS_KEY);
+      loadBalancer.setConf(conf);
+    }
+  }
+
+  @Test
+  public void testDefaultCostFunctionList() {
+    List<String> expected = Arrays.asList(
+      StochasticLoadBalancer.RegionCountSkewCostFunction.class.getSimpleName(),
+      StochasticLoadBalancer.PrimaryRegionCountSkewCostFunction.class.getSimpleName(),
+      StochasticLoadBalancer.MoveCostFunction.class.getSimpleName(),
+      StochasticLoadBalancer.RackLocalityCostFunction.class.getSimpleName(),
+      StochasticLoadBalancer.TableSkewCostFunction.class.getSimpleName(),
+      StochasticLoadBalancer.RegionReplicaHostCostFunction.class.getSimpleName(),
+      StochasticLoadBalancer.RegionReplicaRackCostFunction.class.getSimpleName(),
+      StochasticLoadBalancer.ReadRequestCostFunction.class.getSimpleName(),
+      StochasticLoadBalancer.WriteRequestCostFunction.class.getSimpleName(),
+      StochasticLoadBalancer.MemStoreSizeCostFunction.class.getSimpleName(),
+      StochasticLoadBalancer.StoreFileCostFunction.class.getSimpleName()
+    );
+
+    List<String> actual = Arrays.asList(loadBalancer.getCostFunctionNames());
+    assertTrue("ExpectedCostFunctions: " + expected + " ActualCostFunctions: " + actual,
+      CollectionUtils.isEqualCollection(expected, actual));
   }
 
   private boolean needsBalanceIdleRegion(int[] cluster){