HBASE-25635 CandidateGenerator may miss some region balance actions

Author: sunhelly

hbase-common/src/main/java/org/apache/hadoop/hbase/HConstants.java

@@ -144,6 +144,9 @@ public enum OperationStatusCode {
   public static final String HBASE_MASTER_BALANCER_MAX_RIT_PERCENT =
       "hbase.master.balancer.maxRitPercent";
 
+  public static final String HBASE_MASTER_BALANCER_RANDOM_PICK_REGIONS_WITH_CONSIDER_COUNT_BALANCE =
+      "hbase.master.balancer.random.pick.regions.with.consider.count.balance";
+
   /** Default value for the max percent of regions in transition */
   public static final double DEFAULT_HBASE_MASTER_BALANCER_MAX_RIT_PERCENT = 1.0;
 

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

@@ -23,15 +23,17 @@
 import java.util.List;
 import java.util.Map;
 
+import org.apache.hadoop.hbase.BaseConfigurable;
 import org.apache.hadoop.hbase.client.RegionInfo;
 
 import org.apache.yetus.audience.InterfaceAudience;
+import static org.apache.hadoop.hbase.HConstants.HBASE_MASTER_BALANCER_RANDOM_PICK_REGIONS_WITH_CONSIDER_COUNT_BALANCE;
 
 /**
  * Generates a candidate action to be applied to the cluster for cost function search
  */
 @InterfaceAudience.Private
-abstract class CandidateGenerator {
+abstract class CandidateGenerator extends BaseConfigurable {
 
   abstract BaseLoadBalancer.Cluster.Action generate(BaseLoadBalancer.Cluster cluster);
 
@@ -104,14 +106,19 @@ BaseLoadBalancer.Cluster.Action pickRandomRegions(BaseLoadBalancer.Cluster clust
     if (thisServer < 0 || otherServer < 0) {
       return BaseLoadBalancer.Cluster.NullAction;
     }
-
-    // Decide who is most likely to need another region
-    int thisRegionCount = cluster.getNumRegions(thisServer);
-    int otherRegionCount = cluster.getNumRegions(otherServer);
-
-    // Assign the chance based upon the above
-    double thisChance = (thisRegionCount > otherRegionCount) ? 0 : 0.5;
-    double otherChance = (thisRegionCount <= otherRegionCount) ? 0 : 0.5;
+    double thisChance = 0.5;
+    double otherChance = 0.5;
+    if (getConf() == null || getConf().getBoolean(
+      HBASE_MASTER_BALANCER_RANDOM_PICK_REGIONS_WITH_CONSIDER_COUNT_BALANCE, true)) {
+
+      // Decide who is most likely to need another region
+      int thisRegionCount = cluster.getNumRegions(thisServer);
+      int otherRegionCount = cluster.getNumRegions(otherServer);
+
+      // Assign the chance based upon the above
+      thisChance = (thisRegionCount > otherRegionCount) ? 0 : 0.5;
+      otherChance = (thisRegionCount <= otherRegionCount) ? 0 : 0.5;
+    }
 
     int thisRegion = pickRandomRegion(cluster, thisServer, thisChance);
     int otherRegion = pickRandomRegion(cluster, otherServer, otherChance);
@@ -124,12 +131,12 @@ protected BaseLoadBalancer.Cluster.Action getAction(int fromServer, int fromRegi
     if (fromServer < 0 || toServer < 0) {
       return BaseLoadBalancer.Cluster.NullAction;
     }
-    if (fromRegion > 0 && toRegion > 0) {
+    if (fromRegion >= 0 && toRegion >= 0 && fromRegion != toRegion) {
       return new BaseLoadBalancer.Cluster.SwapRegionsAction(fromServer, fromRegion,
         toServer, toRegion);
-    } else if (fromRegion > 0) {
+    } else if (fromRegion >= 0) {
       return new BaseLoadBalancer.Cluster.MoveRegionAction(fromRegion, fromServer, toServer);
-    } else if (toRegion > 0) {
+    } else if (toRegion >= 0) {
       return new BaseLoadBalancer.Cluster.MoveRegionAction(toRegion, toServer, fromServer);
     } else {
       return BaseLoadBalancer.Cluster.NullAction;

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

@@ -189,7 +189,9 @@ public synchronized void setConf(Configuration conf) {
 
     if (this.candidateGenerators == null) {
       candidateGenerators = Lists.newArrayList();
-      candidateGenerators.add(new RandomCandidateGenerator());
+      RandomCandidateGenerator randomCandidateGenerator = new RandomCandidateGenerator();
+      randomCandidateGenerator.setConf(conf);
+      candidateGenerators.add(randomCandidateGenerator);
       candidateGenerators.add(new LoadCandidateGenerator());
       candidateGenerators.add(localityCandidateGenerator);
       candidateGenerators.add(new RegionReplicaRackCandidateGenerator());

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

@@ -67,6 +67,9 @@ public static void beforeAllTests() throws IOException {
     BalancerTestBase.conf.setBoolean("hbase.master.balancer.stochastic.runMaxSteps", true);
     BalancerTestBase.conf.set(StochasticLoadBalancer.COST_FUNCTIONS_COST_FUNCTIONS_KEY,
       HeterogeneousRegionCountCostFunction.class.getName());
+    BalancerTestBase.conf.setBoolean(
+      HConstants.HBASE_MASTER_BALANCER_RANDOM_PICK_REGIONS_WITH_CONSIDER_COUNT_BALANCE,
+      false);
     // Need to ensure test dir has been created.
     assertTrue(FileSystem.get(HTU.getConfiguration()).mkdirs(HTU.getDataTestDir()));
     RULES_FILE = HTU.getDataTestDir(