From 1c150e09470aa0fb9ee47c89bb260292822ab178 Mon Sep 17 00:00:00 2001 From: Duo Zhang Date: Tue, 10 Sep 2019 11:14:10 +0800 Subject: [PATCH] Revert "HBASE-22809 Allow creating table in group when rs group contains no live servers (#464)" This reverts commit 928ecfb443630ee954896d90add14469c1f4ba9e. --- .../hbase/rsgroup/RSGroupAdminEndpoint.java | 119 ++++++++---------- .../hbase/rsgroup/TestRSGroupsAdmin1.java | 4 +- .../hbase/rsgroup/TestRSGroupsBasics.java | 42 +++++++ 3 files changed, 94 insertions(+), 71 deletions(-) diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupAdminEndpoint.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupAdminEndpoint.java index a2a5623bfa1..3c4530f0da9 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupAdminEndpoint.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupAdminEndpoint.java @@ -21,12 +21,11 @@ import com.google.protobuf.Service; import java.io.IOException; import java.util.Collections; import java.util.List; -import java.util.Objects; import java.util.Optional; import java.util.Set; -import java.util.function.Supplier; import java.util.stream.Collectors; import org.apache.hadoop.hbase.CoprocessorEnvironment; +import org.apache.hadoop.hbase.HBaseIOException; import org.apache.hadoop.hbase.HConstants; import org.apache.hadoop.hbase.NamespaceDescriptor; import org.apache.hadoop.hbase.ServerName; @@ -69,7 +68,7 @@ public class RSGroupAdminEndpoint implements MasterCoprocessor, MasterObserver { groupInfoManager = RSGroupInfoManagerImpl.getInstance(master); groupAdminServer = new RSGroupAdminServer(master, groupInfoManager); Class clazz = - master.getConfiguration().getClass(HConstants.HBASE_MASTER_LOADBALANCER_CLASS, null); + master.getConfiguration().getClass(HConstants.HBASE_MASTER_LOADBALANCER_CLASS, null); if (!RSGroupableBalancer.class.isAssignableFrom(clazz)) { throw new IOException("Configured balancer does not support RegionServer groups."); } @@ -109,101 +108,85 @@ public class RSGroupAdminEndpoint implements MasterCoprocessor, MasterObserver { @Override public void postClearDeadServers(ObserverContext ctx, - List servers, List notClearedServers) throws IOException { + List servers, List notClearedServers) throws IOException { Set
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 RSGroupInfo checkGroupExists(Optional optGroupName, Supplier forWhom) - throws IOException { + private void checkGroupExists(Optional optGroupName) throws IOException { if (optGroupName.isPresent()) { String groupName = optGroupName.get(); - RSGroupInfo group = groupAdminServer.getRSGroupInfo(groupName); - if (group == null) { - throw new ConstraintException( - "Region server group " + groupName + " for " + forWhom.get() + " does not exit"); + if (groupAdminServer.getRSGroupInfo(groupName) == null) { + throw new ConstraintException("Region server group " + groupName + " does not exit"); } - return group; } - return null; } - private Optional getNamespaceGroup(NamespaceDescriptor namespaceDesc) { - return Optional - .ofNullable(namespaceDesc.getConfigurationValue(RSGroupInfo.NAMESPACE_DESC_PROP_GROUP)); - } - - // Do not allow creating new tables/namespaces 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 - // servers can die at any time. - private void checkGroupNotEmpty(RSGroupInfo rsGroupInfo, Supplier forWhom) - throws ConstraintException { - if (rsGroupInfo == null || rsGroupInfo.getName().equals(RSGroupInfo.DEFAULT_GROUP)) { - // we do not have a rs group config or we explicitly set the rs group to default, then no need - // to check. - return; - } - if (rsGroupInfo.getServers().isEmpty()) { - throw new ConstraintException( - "No servers in the rsgroup " + rsGroupInfo.getName() + " for " + forWhom.get()); + private boolean rsgroupHasServersOnline(TableDescriptor desc) throws IOException { + RSGroupInfo rsGroupInfo; + Optional optGroupName = desc.getRegionServerGroup(); + if (optGroupName.isPresent()) { + String groupName = optGroupName.get(); + if (groupName.equals(RSGroupInfo.DEFAULT_GROUP)) { + // do not check for default group + return true; + } + rsGroupInfo = groupAdminServer.getRSGroupInfo(groupName); + if (rsGroupInfo == null) { + throw new ConstraintException( + "RSGroup " + groupName + " for table " + desc.getTableName() + " does not exist"); + } + } else { + NamespaceDescriptor nd = + 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; + } + rsGroupInfo = groupAdminServer.getRSGroupInfo(groupNameOfNs); + if (rsGroupInfo == null) { + throw new ConstraintException("RSGroup " + groupNameOfNs + " for table " + + desc.getTableName() + "(inherit from namespace) does not exist"); + } } + return master.getServerManager().createDestinationServersList().stream() + .anyMatch(onlineServer -> rsGroupInfo.containsServer(onlineServer.getAddress())); } @Override public void preCreateTableAction(ObserverContext ctx, - TableDescriptor desc, RegionInfo[] regions) throws IOException { - if (desc.getTableName().isSystemTable()) { - // do not check for system tables as we may block the bootstrap. - return; + 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); } - Supplier forWhom = () -> "table " + desc.getTableName(); - RSGroupInfo rsGroupInfo = checkGroupExists(desc.getRegionServerGroup(), forWhom); - if (rsGroupInfo == null) { - // we do not set rs group info on table, check if we have one on namespace - String namespace = desc.getTableName().getNamespaceAsString(); - NamespaceDescriptor nd = master.getClusterSchema().getNamespace(namespace); - forWhom = () -> "table " + desc.getTableName() + "(inherit from namespace)"; - rsGroupInfo = checkGroupExists(getNamespaceGroup(nd), forWhom); - } - checkGroupNotEmpty(rsGroupInfo, forWhom); } @Override public TableDescriptor preModifyTable(ObserverContext ctx, - TableName tableName, TableDescriptor currentDescriptor, TableDescriptor newDescriptor) - throws IOException { - if (!currentDescriptor.getRegionServerGroup().equals(newDescriptor.getRegionServerGroup())) { - Supplier forWhom = () -> "table " + newDescriptor.getTableName(); - RSGroupInfo rsGroupInfo = checkGroupExists(newDescriptor.getRegionServerGroup(), forWhom); - checkGroupNotEmpty(rsGroupInfo, forWhom); - } + TableName tableName, TableDescriptor currentDescriptor, TableDescriptor newDescriptor) + throws IOException { + checkGroupExists(newDescriptor.getRegionServerGroup()); return MasterObserver.super.preModifyTable(ctx, tableName, currentDescriptor, newDescriptor); } - private void checkNamespaceGroup(NamespaceDescriptor nd) throws IOException { - Supplier forWhom = () -> "namespace " + nd.getName(); - RSGroupInfo rsGroupInfo = checkGroupExists(getNamespaceGroup(nd), forWhom); - checkGroupNotEmpty(rsGroupInfo, forWhom); - } - @Override public void preCreateNamespace(ObserverContext ctx, - NamespaceDescriptor ns) throws IOException { - checkNamespaceGroup(ns); + NamespaceDescriptor ns) throws IOException { + checkGroupExists( + Optional.ofNullable(ns.getConfigurationValue(RSGroupInfo.NAMESPACE_DESC_PROP_GROUP))); } @Override public void preModifyNamespace(ObserverContext ctx, - NamespaceDescriptor currentNsDescriptor, NamespaceDescriptor newNsDescriptor) - throws IOException { - if (!Objects.equals( - currentNsDescriptor.getConfigurationValue(RSGroupInfo.NAMESPACE_DESC_PROP_GROUP), - newNsDescriptor.getConfigurationValue(RSGroupInfo.NAMESPACE_DESC_PROP_GROUP))) { - checkNamespaceGroup(newNsDescriptor); - } + NamespaceDescriptor currentNsDescriptor, NamespaceDescriptor newNsDescriptor) + throws IOException { + checkGroupExists(Optional + .ofNullable(newNsDescriptor.getConfigurationValue(RSGroupInfo.NAMESPACE_DESC_PROP_GROUP))); } } diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/rsgroup/TestRSGroupsAdmin1.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/rsgroup/TestRSGroupsAdmin1.java index aff591fec3f..74714586301 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/rsgroup/TestRSGroupsAdmin1.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/rsgroup/TestRSGroupsAdmin1.java @@ -152,14 +152,12 @@ public class TestRSGroupsAdmin1 extends TestRSGroupsBase { String nsName = tablePrefix + "_foo"; String groupName = tablePrefix + "_foo"; LOG.info("testNamespaceConstraint"); - addGroup(groupName, 1); + rsGroupAdmin.addRSGroup(groupName); assertTrue(observer.preAddRSGroupCalled); assertTrue(observer.postAddRSGroupCalled); admin.createNamespace(NamespaceDescriptor.create(nsName) .addConfiguration(RSGroupInfo.NAMESPACE_DESC_PROP_GROUP, groupName).build()); - RSGroupInfo rsGroupInfo = rsGroupAdmin.getRSGroupInfo(groupName); - rsGroupAdmin.moveServers(rsGroupInfo.getServers(), RSGroupInfo.DEFAULT_GROUP); // test removing a referenced group try { rsGroupAdmin.removeRSGroup(groupName); diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/rsgroup/TestRSGroupsBasics.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/rsgroup/TestRSGroupsBasics.java index 67ed2a90224..e3cb54e1cdf 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/rsgroup/TestRSGroupsBasics.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/rsgroup/TestRSGroupsBasics.java @@ -20,8 +20,11 @@ package org.apache.hadoop.hbase.rsgroup; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; import java.io.IOException; +import java.util.ArrayList; +import java.util.Iterator; import java.util.List; import java.util.Set; import org.apache.hadoop.hbase.HBaseClassTestRule; @@ -133,6 +136,45 @@ public class TestRSGroupsBasics extends TestRSGroupsBase { Assert.assertEquals(1, admin.getRegions(targetServer).size()); } + @Test + public void testCreateWhenRsgroupNoOnlineServers() throws Exception { + LOG.info("testCreateWhenRsgroupNoOnlineServers"); + + // set rsgroup has no online servers and test create table + final RSGroupInfo appInfo = addGroup("appInfo", 1); + Iterator
iterator = appInfo.getServers().iterator(); + List serversToDecommission = new ArrayList<>(); + ServerName targetServer = getServerName(iterator.next()); + assertTrue(master.getServerManager().getOnlineServers().containsKey(targetServer)); + serversToDecommission.add(targetServer); + admin.decommissionRegionServers(serversToDecommission, true); + assertEquals(1, admin.listDecommissionedRegionServers().size()); + + final TableName tableName = TableName.valueOf(tablePrefix + "_ns", name.getMethodName()); + admin.createNamespace(NamespaceDescriptor.create(tableName.getNamespaceAsString()) + .addConfiguration(RSGroupInfo.NAMESPACE_DESC_PROP_GROUP, appInfo.getName()).build()); + final TableDescriptor desc = TableDescriptorBuilder.newBuilder(tableName) + .setColumnFamily(ColumnFamilyDescriptorBuilder.of("f")).build(); + try { + admin.createTable(desc); + fail("Shouldn't create table successfully!"); + } catch (Exception e) { + LOG.debug("create table error", e); + } + + // recommission and test create table + admin.recommissionRegionServer(targetServer, null); + assertEquals(0, admin.listDecommissionedRegionServers().size()); + admin.createTable(desc); + // wait for created table to be assigned + TEST_UTIL.waitFor(WAIT_TIMEOUT, new Waiter.Predicate() { + @Override + public boolean evaluate() throws Exception { + return getTableRegionMap().get(desc.getTableName()) != null; + } + }); + } + @Test public void testDefaultNamespaceCreateAndAssign() throws Exception { LOG.info("testDefaultNamespaceCreateAndAssign");