HBASE-24139 : Balancer should avoid leaving idle region servers (#1511)

Co-authored-by: Viraj Jasani <vjasani@apache.org>

Signed-off-by: Viraj Jasani <vjasani@apache.org>
Signed-off-by: Sean Busbey <busbey@apache.org>
Signed-off-by: Anoop Sam John <anoopsamjohn@apache.org>

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

@@ -1297,6 +1297,10 @@ public abstract class BaseLoadBalancer implements LoadBalancer {
       return false;
     }
     if(areSomeRegionReplicasColocated(c)) return true;
+    if(idleRegionServerExist(c)) {
+      return true;
+    }
+
     // Check if we even need to do any load balancing
     // HBASE-3681 check sloppiness first
     float average = cs.getLoadAverage(); // for logging
@@ -1328,6 +1332,20 @@ public abstract class BaseLoadBalancer implements LoadBalancer {
     return false;
   }
 
+  protected final boolean idleRegionServerExist(Cluster c){
+    boolean isServerExistsWithMoreRegions = false;
+    boolean isServerExistsWithZeroRegions = false;
+    for (int[] serverList: c.regionsPerServer){
+      if (serverList.length > 1) {
+        isServerExistsWithMoreRegions = true;
+      }
+      if (serverList.length == 0) {
+        isServerExistsWithZeroRegions = true;
+      }
+    }
+    return isServerExistsWithMoreRegions && isServerExistsWithZeroRegions;
+  }
+
   /**
    * Generates a bulk assignment plan to be used on cluster startup using a
    * simple round-robin assignment.

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

@@ -329,6 +329,10 @@ public class StochasticLoadBalancer extends BaseLoadBalancer {
       return true;
     }
 
+    if (idleRegionServerExist(cluster)){
+      return true;
+    }
+
     double total = 0.0;
     float sumMultiplier = 0.0f;
     for (CostFunction c : costFunctions) {

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

@@ -39,6 +39,7 @@ import org.apache.commons.logging.LogFactory;
 import org.apache.hadoop.conf.Configuration;
 import org.apache.hadoop.hbase.ClusterStatus;
 import org.apache.hadoop.hbase.HBaseConfiguration;
+import org.apache.hadoop.hbase.HConstants;
 import org.apache.hadoop.hbase.HRegionInfo;
 import org.apache.hadoop.hbase.RegionLoad;
 import org.apache.hadoop.hbase.ServerLoad;
@@ -161,11 +162,17 @@ public class TestStochasticLoadBalancer extends BalancerTestBase {
     float minCost = conf.getFloat("hbase.master.balancer.stochastic.minCostNeedBalance", 0.05f);
     conf.setFloat("hbase.master.balancer.stochastic.minCostNeedBalance", 1.0f);
     try {
+      // Test with/without per table balancer.
+      boolean[] perTableBalancerConfigs = {true, false};
+      for (boolean isByTable : perTableBalancerConfigs) {
+        conf.setBoolean(HConstants.HBASE_MASTER_LOADBALANCE_BYTABLE, isByTable);
         loadBalancer.setConf(conf);
         for (int[] mockCluster : clusterStateMocks) {
           Map<ServerName, List<HRegionInfo>> servers = mockClusterServers(mockCluster);
           List<RegionPlan> plans = loadBalancer.balanceCluster(servers);
-        assertNull(plans);
+          boolean emptyPlans = plans == null || plans.isEmpty();
+          assertTrue(emptyPlans || needsBalanceIdleRegion(mockCluster));
+        }
       }
     } finally {
       // reset config
@@ -684,6 +691,19 @@ public class TestStochasticLoadBalancer extends BalancerTestBase {
             contains(DummyCostFunction.class.getSimpleName()));
   }
 
+  private boolean needsBalanceIdleRegion(int[] clusters) {
+    boolean b1 = false;
+    boolean b2 = false;
+    for (int cluster : clusters) {
+      if (cluster > 1) {
+        b1 = true;
+      } else {
+        b2 = true;
+      }
+    }
+    return b1 && b2;
+  }
+
   // This mock allows us to test the LocalityCostFunction
   private class MockCluster extends BaseLoadBalancer.Cluster {