HBASE-23564 RegionStates may has some expired serverinfo and make reg…

…ions do not balance. (#930) Signed-off-by: stack <[email protected]> Signed-off-by: Wellington Chevreuil <[email protected]>
apache · Dec 20, 2019 · ab40b96 · ab40b96
1 parent f4860d8
commit ab40b96
Show file tree

Hide file tree

Showing 3 changed files with 27 additions and 12 deletions.
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java
@@ -1797,7 +1797,8 @@ public boolean balance(boolean force) throws IOException {
       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());
       }

diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/RegionStates.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/RegionStates.java
@@ -539,7 +539,7 @@ public ServerName getRegionServerOfRegion(RegionInfo regionInfo) {
    * @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 Map<TableName, Map<ServerName, List<RegionInfo>>> getAssignmentsForBalanc
       }
       // 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()) {
-        ensemble.put(serverNode.getServerName(), serverNode.getRegionInfoList().stream()
-          .filter(region -> !isTableDisabled(tableStateManager, region.getTable()))
-          .collect(Collectors.toList()));
+      for (ServerName serverName : onlineServers) {
+        ServerStateNode serverNode = serverMap.get(serverName);
+        if (serverNode != null) {
+          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);

diff --git a/...master/TestBalancerWithDisabledTable.java → ...che/hadoop/hbase/master/TestBalancer.java b/...master/TestBalancerWithDisabledTable.java → ...che/hadoop/hbase/master/TestBalancer.java
@@ -32,6 +32,7 @@
 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 @@
  * 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 void testAssignmentsForBalancer() throws Exception {
 
     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 =
-      assignmentManager.getRegionStates().getAssignmentsForBalancer(tableStateManager, false);
+    assignments = assignmentManager.getRegionStates()
+      .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 void testAssignmentsForBalancer() throws Exception {
     }
     assertFalse(tableNameMap.containsKey(disableTableName));
     assertTrue(tableNameMap.containsKey(tableName));
+    assertFalse(tableNameMap.get(tableName).containsKey(sn1));
   }
 }