[GitHub] [hbase] infraio commented on a change in pull request #464: HBASE-22809 Allow creating table in group when rs group contains no l…

[GitBox <gi...@apache.org>]

infraio commented on a change in pull request #464: HBASE-22809 Allow creating table in group when rs group contains no l…
URL: https://github.com/apache/hbase/pull/464#discussion_r311835695
 
 

 ##########
 File path: hbase-server/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupAdminEndpoint.java
 ##########
 @@ -108,85 +108,119 @@ RSGroupAdminServiceImpl getGroupAdminService() {
 
   @Override
   public void postClearDeadServers(ObserverContext<MasterCoprocessorEnvironment> ctx,
-      List<ServerName> servers, List<ServerName> notClearedServers) throws IOException {
+    List<ServerName> servers, List<ServerName> notClearedServers) throws IOException {
     Set<Address> clearedServer =
-        servers.stream().filter(server -> !notClearedServers.contains(server))
-            .map(ServerName::getAddress).collect(Collectors.toSet());
+      servers.stream().filter(server -> !notClearedServers.contains(server))
+        .map(ServerName::getAddress).collect(Collectors.toSet());
     if (!clearedServer.isEmpty()) {
       groupAdminServer.removeServers(clearedServer);
     }
   }
 
-  private void checkGroupExists(Optional<String> optGroupName) throws IOException {
+  private RSGroupInfo checkGroupExists(Optional<String> optGroupName) throws IOException {
     if (optGroupName.isPresent()) {
       String groupName = optGroupName.get();
-      if (groupAdminServer.getRSGroupInfo(groupName) == null) {
+      RSGroupInfo group = groupAdminServer.getRSGroupInfo(groupName);
+      if (group == null) {
         throw new ConstraintException("Region server group " + groupName + " does not exit");
       }
+      return group;
     }
+    return null;
   }
 
-  private boolean rsgroupHasServersOnline(TableDescriptor desc) throws IOException {
+  // Do not allow creating new tables which has an empty rs group, expect the default rs group.
+  // Notice that we do not check for online servers, as this is not stable because region server can
+  // die at any time.
+  private void checkForEmptyGroup(TableDescriptor desc) throws IOException {
+    if (desc.getTableName().isSystemTable()) {
+      // do not check for system tables as we may block the bootstrap.
+      return;
+    }
     RSGroupInfo rsGroupInfo;
     Optional<String> optGroupName = desc.getRegionServerGroup();
     if (optGroupName.isPresent()) {
       String groupName = optGroupName.get();
       if (groupName.equals(RSGroupInfo.DEFAULT_GROUP)) {
         // do not check for default group
-        return true;
+        return;
       }
       rsGroupInfo = groupAdminServer.getRSGroupInfo(groupName);
       if (rsGroupInfo == null) {
         throw new ConstraintException(
-            "RSGroup " + groupName + " for table " + desc.getTableName() + " does not exist");
+          "RSGroup " + groupName + " for table " + desc.getTableName() + " does not exist");
       }
     } else {
       NamespaceDescriptor nd =
-          master.getClusterSchema().getNamespace(desc.getTableName().getNamespaceAsString());
+        master.getClusterSchema().getNamespace(desc.getTableName().getNamespaceAsString());
       String groupNameOfNs = nd.getConfigurationValue(RSGroupInfo.NAMESPACE_DESC_PROP_GROUP);
       if (groupNameOfNs == null || groupNameOfNs.equals(RSGroupInfo.DEFAULT_GROUP)) {
         // do not check for default group
-        return true;
+        return;
       }
       rsGroupInfo = groupAdminServer.getRSGroupInfo(groupNameOfNs);
       if (rsGroupInfo == null) {
         throw new ConstraintException("RSGroup " + groupNameOfNs + " for table " +
-            desc.getTableName() + "(inherit from namespace) does not exist");
+          desc.getTableName() + "(inherit from namespace) does not exist");
       }
     }
-    return master.getServerManager().createDestinationServersList().stream()
-        .anyMatch(onlineServer -> rsGroupInfo.containsServer(onlineServer.getAddress()));
+    if (rsGroupInfo.getServers().isEmpty()) {
+      throw new ConstraintException(
+        "No servers in the rsgroup " + rsGroupInfo.getName() + " for " + desc);
+    }
   }
 
   @Override
   public void preCreateTableAction(ObserverContext<MasterCoprocessorEnvironment> ctx,
-      TableDescriptor desc, RegionInfo[] regions) throws IOException {
+    TableDescriptor desc, RegionInfo[] regions) throws IOException {
     checkGroupExists(desc.getRegionServerGroup());
-    if (!desc.getTableName().isSystemTable() && !rsgroupHasServersOnline(desc)) {
-      throw new HBaseIOException("No online servers in the rsgroup for " + desc);
-    }
+    checkForEmptyGroup(desc);
   }
 
   @Override
   public TableDescriptor preModifyTable(ObserverContext<MasterCoprocessorEnvironment> ctx,
-      TableName tableName, TableDescriptor currentDescriptor, TableDescriptor newDescriptor)
-      throws IOException {
-    checkGroupExists(newDescriptor.getRegionServerGroup());
+    TableName tableName, TableDescriptor currentDescriptor, TableDescriptor newDescriptor)
+    throws IOException {
+    if (!currentDescriptor.getRegionServerGroup().equals(newDescriptor.getRegionServerGroup())) {
+      RSGroupInfo group = checkGroupExists(newDescriptor.getRegionServerGroup());
+      if (group != null && group.getServers().isEmpty()) {
+        throw new ConstraintException(
+          "No servers in the rsgroup " + group.getName() + " for " + newDescriptor);
+      }
+    }
     return MasterObserver.super.preModifyTable(ctx, tableName, currentDescriptor, newDescriptor);
   }
 
+  private void checkNamespaceGroup(NamespaceDescriptor ns) throws IOException {
+    String groupName = ns.getConfigurationValue(RSGroupInfo.NAMESPACE_DESC_PROP_GROUP);
+    if (groupName == null) {
 
 Review comment:
   || groupName.equals(RSGroupInfo.DEFAULT_GROUP)?

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services