You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hbase.apache.org by zg...@apache.org on 2020/08/26 06:12:11 UTC

[hbase] branch branch-2.3 updated: HBASE-24928 balanceRSGroup should skip generating balance plan for disabled table and splitParent region (#2300)

This is an automated email from the ASF dual-hosted git repository.

zghao pushed a commit to branch branch-2.3
in repository https://gitbox.apache.org/repos/asf/hbase.git


The following commit(s) were added to refs/heads/branch-2.3 by this push:
     new b4e5702  HBASE-24928 balanceRSGroup should skip generating balance plan for disabled table and splitParent region (#2300)
b4e5702 is described below

commit b4e570210820f1c5aa44c6cb531c15df8b98613e
Author: niuyulin <ny...@163.com>
AuthorDate: Wed Aug 26 01:01:35 2020 -0500

    HBASE-24928 balanceRSGroup should skip generating balance plan for disabled table and splitParent region (#2300)
    
    Signed-off-by: Viraj Jasani <vj...@apache.org>
    Signed-off-by: Guanghao Zhang <zg...@apache.org>
---
 .../hadoop/hbase/rsgroup/RSGroupAdminEndpoint.java |  8 +++++--
 .../hadoop/hbase/rsgroup/RSGroupAdminServer.java   | 28 ++++++++++++++++++----
 .../hadoop/hbase/rsgroup/TestRSGroupsBalance.java  | 18 ++++++++++++++
 3 files changed, 47 insertions(+), 7 deletions(-)

diff --git a/hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupAdminEndpoint.java b/hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupAdminEndpoint.java
index ad4c1af..3838c14 100644
--- a/hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupAdminEndpoint.java
+++ b/hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupAdminEndpoint.java
@@ -30,7 +30,6 @@ import java.util.Optional;
 import java.util.Set;
 import java.util.stream.Collectors;
 
-import org.apache.hadoop.conf.Configuration;
 import org.apache.hadoop.hbase.CoprocessorEnvironment;
 import org.apache.hadoop.hbase.HBaseIOException;
 import org.apache.hadoop.hbase.HConstants;
@@ -86,10 +85,11 @@ import org.apache.hadoop.hbase.security.User;
 import org.apache.hadoop.hbase.security.UserProvider;
 import org.apache.hadoop.hbase.security.access.AccessChecker;
 import org.apache.hadoop.hbase.security.access.Permission.Action;
-import org.apache.hadoop.util.Shell.ShellCommandExecutor;
 import org.apache.yetus.audience.InterfaceAudience;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
+
+import org.apache.hbase.thirdparty.com.google.common.annotations.VisibleForTesting;
 import org.apache.hbase.thirdparty.com.google.common.collect.Sets;
 
 // TODO: Encapsulate MasterObserver functions into separate subclass.
@@ -147,6 +147,10 @@ public class RSGroupAdminEndpoint implements MasterCoprocessor, MasterObserver {
     return groupInfoManager;
   }
 
+  @VisibleForTesting
+  RSGroupAdminServer getGroupAdminServer() {
+    return groupAdminServer;
+  }
   /**
    * Implementation of RSGroupAdminService defined in RSGroupAdmin.proto.
    * This class calls {@link RSGroupAdminServer} for actual work, converts result to protocol
diff --git a/hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupAdminServer.java b/hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupAdminServer.java
index bf9f8488..a6ee3ae 100644
--- a/hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupAdminServer.java
+++ b/hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupAdminServer.java
@@ -32,6 +32,7 @@ import org.apache.hadoop.hbase.NamespaceDescriptor;
 import org.apache.hadoop.hbase.ServerName;
 import org.apache.hadoop.hbase.TableName;
 import org.apache.hadoop.hbase.client.RegionInfo;
+import org.apache.hadoop.hbase.client.TableState;
 import org.apache.hadoop.hbase.constraint.ConstraintException;
 import org.apache.hadoop.hbase.master.HMaster;
 import org.apache.hadoop.hbase.master.LoadBalancer;
@@ -39,6 +40,7 @@ import org.apache.hadoop.hbase.master.MasterServices;
 import org.apache.hadoop.hbase.master.RegionPlan;
 import org.apache.hadoop.hbase.master.RegionState;
 import org.apache.hadoop.hbase.master.ServerManager;
+import org.apache.hadoop.hbase.master.TableStateManager;
 import org.apache.hadoop.hbase.master.assignment.AssignmentManager;
 import org.apache.hadoop.hbase.master.assignment.RegionStateNode;
 import org.apache.hadoop.hbase.net.Address;
@@ -46,6 +48,7 @@ import org.apache.yetus.audience.InterfaceAudience;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import org.apache.hbase.thirdparty.com.google.common.annotations.VisibleForTesting;
 import org.apache.hbase.thirdparty.com.google.common.collect.Maps;
 
 /**
@@ -454,7 +457,7 @@ public class RSGroupAdminServer implements RSGroupAdmin {
 
       //We balance per group instead of per table
       Map<TableName, Map<ServerName, List<RegionInfo>>> assignmentsByTable =
-        getRSGroupAssignmentsByTable(groupName);
+          getRSGroupAssignmentsByTable(master.getTableStateManager(), groupName);
       List<RegionPlan> plans = balancer.balanceCluster(assignmentsByTable);
       boolean balancerRan = !plans.isEmpty();
       if (balancerRan) {
@@ -547,17 +550,32 @@ public class RSGroupAdminServer implements RSGroupAdmin {
     return rit;
   }
 
-  private Map<TableName, Map<ServerName, List<RegionInfo>>>
-      getRSGroupAssignmentsByTable(String groupName) throws IOException {
+  /**
+   * This is an EXPENSIVE clone. Cloning though is the safest thing to do. Can't let out original
+   * since it can change and at least the load balancer wants to iterate this exported list. Load
+   * balancer should iterate over this list because cloned list will ignore disabled table and split
+   * parent region cases. This method is invoked by {@link #balanceRSGroup}
+   * @return A clone of current assignments for this group.
+   */
+  @VisibleForTesting
+  Map<TableName, Map<ServerName, List<RegionInfo>>> getRSGroupAssignmentsByTable(
+      TableStateManager tableStateManager, String groupName) throws IOException {
     Map<TableName, Map<ServerName, List<RegionInfo>>> result = Maps.newHashMap();
     RSGroupInfo rsGroupInfo = getRSGroupInfo(groupName);
     Map<TableName, Map<ServerName, List<RegionInfo>>> assignments = Maps.newHashMap();
-    for(Map.Entry<RegionInfo, ServerName> entry:
-        master.getAssignmentManager().getRegionStates().getRegionAssignments().entrySet()) {
+    for (Map.Entry<RegionInfo, ServerName> entry : master.getAssignmentManager().getRegionStates()
+        .getRegionAssignments().entrySet()) {
       TableName currTable = entry.getKey().getTable();
       ServerName currServer = entry.getValue();
       RegionInfo currRegion = entry.getKey();
       if (rsGroupInfo.getTables().contains(currTable)) {
+        if (tableStateManager.isTableState(currTable, TableState.State.DISABLED,
+          TableState.State.DISABLING)) {
+          continue;
+        }
+        if (currRegion.isSplitParent()) {
+          continue;
+        }
         assignments.putIfAbsent(currTable, new HashMap<>());
         assignments.get(currTable).putIfAbsent(currServer, new ArrayList<>());
         assignments.get(currTable).get(currServer).add(currRegion);
diff --git a/hbase-rsgroup/src/test/java/org/apache/hadoop/hbase/rsgroup/TestRSGroupsBalance.java b/hbase-rsgroup/src/test/java/org/apache/hadoop/hbase/rsgroup/TestRSGroupsBalance.java
index 67f5c7e..e419ee0 100644
--- a/hbase-rsgroup/src/test/java/org/apache/hadoop/hbase/rsgroup/TestRSGroupsBalance.java
+++ b/hbase-rsgroup/src/test/java/org/apache/hadoop/hbase/rsgroup/TestRSGroupsBalance.java
@@ -24,6 +24,7 @@ import static org.junit.Assert.assertTrue;
 import java.util.List;
 import java.util.Map;
 import org.apache.hadoop.hbase.HBaseClassTestRule;
+import org.apache.hadoop.hbase.HConstants;
 import org.apache.hadoop.hbase.NamespaceDescriptor;
 import org.apache.hadoop.hbase.ServerName;
 import org.apache.hadoop.hbase.TableName;
@@ -33,6 +34,7 @@ import org.apache.hadoop.hbase.client.ColumnFamilyDescriptorBuilder;
 import org.apache.hadoop.hbase.client.RegionInfo;
 import org.apache.hadoop.hbase.client.TableDescriptor;
 import org.apache.hadoop.hbase.client.TableDescriptorBuilder;
+import org.apache.hadoop.hbase.master.HMaster;
 import org.apache.hadoop.hbase.testclassification.MediumTests;
 import org.apache.hadoop.hbase.util.Bytes;
 import org.junit.After;
@@ -180,4 +182,20 @@ public class TestRSGroupsBalance extends TestRSGroupsBase {
     });
   }
 
+  @Test
+  public void testGetRSGroupAssignmentsByTable() throws Exception {
+    final TableName tableName = TableName.valueOf(name.getMethodName());
+    TEST_UTIL.createMultiRegionTable(tableName, HConstants.CATALOG_FAMILY, 10);
+    // disable table
+    final TableName disableTableName = TableName.valueOf("testDisableTable");
+    TEST_UTIL.createMultiRegionTable(disableTableName, HConstants.CATALOG_FAMILY, 10);
+    TEST_UTIL.getAdmin().disableTable(disableTableName);
+
+    HMaster master = TEST_UTIL.getMiniHBaseCluster().getMaster();
+    Map<TableName, Map<ServerName, List<RegionInfo>>> assignments =
+        rsGroupAdminEndpoint.getGroupAdminServer()
+            .getRSGroupAssignmentsByTable(master.getTableStateManager(), RSGroupInfo.DEFAULT_GROUP);
+    assertFalse(assignments.containsKey(disableTableName));
+    assertTrue(assignments.containsKey(tableName));
+  }
 }