HBASE-25635 CandidateGenerator may miss some region balance actions

Author: sunhelly

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

@@ -26,7 +26,6 @@
 import org.apache.hadoop.hbase.client.RegionInfo;
 
 import org.apache.yetus.audience.InterfaceAudience;
-
 /**
  * Generates a candidate action to be applied to the cluster for cost function search
  */
@@ -124,12 +123,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) {
       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

@@ -263,6 +263,13 @@ protected void setCandidateGenerators(List<CandidateGenerator> customCandidateGe
     this.candidateGenerators = customCandidateGenerators;
   }
 
+  /**
+   * Exposed for Testing!
+   */
+  public List<CandidateGenerator> getCandidateGenerators() {
+    return this.candidateGenerators;
+  }
+
   @Override
   protected void setSlop(Configuration conf) {
     this.slop = conf.getFloat("hbase.regions.slop", 0.001F);

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

@@ -76,6 +76,7 @@ public static void beforeAllTests() throws IOException {
       RULES_FILE);
     BalancerTestBase.loadBalancer = new StochasticLoadBalancer();
     BalancerTestBase.loadBalancer.setConf(BalancerTestBase.conf);
+    BalancerTestBase.loadBalancer.getCandidateGenerators().add(new FairRandomCandidateGenerator());
   }
 
   @Test
@@ -279,4 +280,26 @@ private ServerAndLoad createServer(final String host) {
     ServerName sn = ServerName.valueOf(host, port, startCode);
     return new ServerAndLoad(sn, 0);
   }
+
+  static class FairRandomCandidateGenerator extends
+    StochasticLoadBalancer.RandomCandidateGenerator {
+
+    @Override
+    public BaseLoadBalancer.Cluster.Action pickRandomRegions(BaseLoadBalancer.Cluster cluster,
+      int thisServer, int otherServer) {
+      if (thisServer < 0 || otherServer < 0) {
+        return BaseLoadBalancer.Cluster.NullAction;
+      }
+
+      int thisRegion = pickRandomRegion(cluster, thisServer, 0.5);
+      int otherRegion = pickRandomRegion(cluster, otherServer, 0.5);
+
+      return getAction(thisServer, thisRegion, otherServer, otherRegion);
+    }
+
+    @Override
+    BaseLoadBalancer.Cluster.Action generate(BaseLoadBalancer.Cluster cluster) {
+      return super.generate(cluster);
+    }
+  }
 }