HBASE-23564 RegionStates may has some expired serverinfo and make regions do not balance. (#930)

Signed-off-by: stack <stack@apache.org>
Signed-off-by: Wellington Chevreuil <wchevreuil@apache.org>

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

@@ -1797,7 +1797,8 @@ public class HMaster extends HRegionServer implements MasterServices {
       boolean isByTable = getConfiguration().getBoolean("hbase.master.loadbalance.bytable", false);
       Map<TableName, Map<ServerName, List<RegionInfo>>> assignments =
         this.assignmentManager.getRegionStates()
-          .getAssignmentsForBalancer(tableStateManager, isByTable);
+          .getAssignmentsForBalancer(tableStateManager, this.serverManager.getOnlineServersList(),
+            isByTable);
       for (Map<ServerName, List<RegionInfo>> serverMap : assignments.values()) {
         serverMap.keySet().removeAll(this.serverManager.getDrainingServersList());
       }

hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/RegionStates.java

@@ -539,7 +539,7 @@ public class RegionStates {
    * @return A clone of current assignments.
    */
   public Map<TableName, Map<ServerName, List<RegionInfo>>> getAssignmentsForBalancer(
-      TableStateManager tableStateManager, boolean isByTable) {
+    TableStateManager tableStateManager, List<ServerName> onlineServers, boolean isByTable) {
     final Map<TableName, Map<ServerName, List<RegionInfo>>> result = new HashMap<>();
     if (isByTable) {
       for (RegionStateNode node : regionsMap.values()) {
@@ -559,16 +559,21 @@ public class RegionStates {
       }
       // Add online servers with no assignment for the table.
       for (Map<ServerName, List<RegionInfo>> table : result.values()) {
-        for (ServerName serverName : serverMap.keySet()) {
+        for (ServerName serverName : onlineServers) {
           table.computeIfAbsent(serverName, key -> new ArrayList<>());
         }
       }
     } else {
       final HashMap<ServerName, List<RegionInfo>> ensemble = new HashMap<>(serverMap.size());
-      for (ServerStateNode serverNode : serverMap.values()) {
+      for (ServerName serverName : onlineServers) {
-        ensemble.put(serverNode.getServerName(), serverNode.getRegionInfoList().stream()
+        ServerStateNode serverNode = serverMap.get(serverName);
-          .filter(region -> !isTableDisabled(tableStateManager, region.getTable()))
+        if (serverNode != null) {
-          .collect(Collectors.toList()));
+          ensemble.put(serverNode.getServerName(), serverNode.getRegionInfoList().stream()
+            .filter(region -> !isTableDisabled(tableStateManager, region.getTable()))
+            .collect(Collectors.toList()));
+        } else {
+          ensemble.put(serverName, new ArrayList<>());
+        }
       }
       // Use a fake table name to represent the whole cluster's assignments
       result.put(HConstants.ENSEMBLE_TABLE_NAME, ensemble);

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

@@ -32,6 +32,7 @@ import org.apache.hadoop.hbase.ServerName;
 import org.apache.hadoop.hbase.TableName;
 import org.apache.hadoop.hbase.client.RegionInfo;
 import org.apache.hadoop.hbase.master.assignment.AssignmentManager;
+import org.apache.hadoop.hbase.master.assignment.RegionStates;
 import org.apache.hadoop.hbase.testclassification.LargeTests;
 import org.apache.hadoop.hbase.testclassification.MasterTests;
 import org.junit.After;
@@ -46,11 +47,11 @@ import org.junit.rules.TestName;
  * Test balancer with disabled table
  */
 @Category({ MasterTests.class, LargeTests.class })
-public class TestBalancerWithDisabledTable {
+public class TestBalancer {
 
   @ClassRule
   public static final HBaseClassTestRule CLASS_RULE =
-      HBaseClassTestRule.forClass(TestBalancerWithDisabledTable.class);
+      HBaseClassTestRule.forClass(TestBalancer.class);
 
   private final HBaseTestingUtility TEST_UTIL = new HBaseTestingUtility();
 
@@ -78,14 +79,21 @@ public class TestBalancerWithDisabledTable {
 
     HMaster master = TEST_UTIL.getMiniHBaseCluster().getMaster();
     AssignmentManager assignmentManager = master.getAssignmentManager();
+    RegionStates regionStates = assignmentManager.getRegionStates();
+    ServerName sn1 = ServerName.parseServerName("asf903.gq1.ygridcore.net,52690,1517835491385");
+    regionStates.getOrCreateServer(sn1);
+
     TableStateManager tableStateManager = master.getTableStateManager();
+    ServerManager serverManager = master.getServerManager();
     Map<TableName, Map<ServerName, List<RegionInfo>>> assignments =
-      assignmentManager.getRegionStates().getAssignmentsForBalancer(tableStateManager, true);
+      assignmentManager.getRegionStates()
+        .getAssignmentsForBalancer(tableStateManager, serverManager.getOnlineServersList(), true);
     assertFalse(assignments.containsKey(disableTableName));
     assertTrue(assignments.containsKey(tableName));
+    assertFalse(assignments.get(tableName).containsKey(sn1));
 
-    assignments =
+    assignments = assignmentManager.getRegionStates()
-      assignmentManager.getRegionStates().getAssignmentsForBalancer(tableStateManager, false);
+      .getAssignmentsForBalancer(tableStateManager, serverManager.getOnlineServersList(), false);
     Map<TableName, Map<ServerName, List<RegionInfo>>> tableNameMap = new HashMap<>();
     for (Map.Entry<ServerName, List<RegionInfo>> entry : assignments
       .get(HConstants.ENSEMBLE_TABLE_NAME).entrySet()) {
@@ -100,5 +108,6 @@ public class TestBalancerWithDisabledTable {
     }
     assertFalse(tableNameMap.containsKey(disableTableName));
     assertTrue(tableNameMap.containsKey(tableName));
+    assertFalse(tableNameMap.get(tableName).containsKey(sn1));
   }
 }