You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hbase.apache.org by zh...@apache.org on 2019/09/10 03:15:15 UTC
[hbase] 04/08: Revert "HBASE-22809 Allow creating table in group
when rs group contains no live servers (#464)"
This is an automated email from the ASF dual-hosted git repository.
zhangduo pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/hbase.git
commit 1c150e09470aa0fb9ee47c89bb260292822ab178
Author: Duo Zhang <zh...@apache.org>
AuthorDate: Tue Sep 10 11:14:10 2019 +0800
Revert "HBASE-22809 Allow creating table in group when rs group contains no live servers (#464)"
This reverts commit 928ecfb443630ee954896d90add14469c1f4ba9e.
---
.../hadoop/hbase/rsgroup/RSGroupAdminEndpoint.java | 119 +++++++++------------
.../hadoop/hbase/rsgroup/TestRSGroupsAdmin1.java | 4 +-
.../hadoop/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 a2a5623..3c4530f 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<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 RSGroupInfo checkGroupExists(Optional<String> optGroupName, Supplier<String> forWhom)
- throws IOException {
+ private void checkGroupExists(Optional<String> 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<String> 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<String> 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<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;
+ }
+ 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<MasterCoprocessorEnvironment> ctx,
- TableDescriptor desc, RegionInfo[] regions) throws IOException {
- if (desc.getTableName().isSystemTable()) {
- // do not check for system tables as we may block the bootstrap.
- return;
- }
- Supplier<String> 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);
+ 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);
}
- checkGroupNotEmpty(rsGroupInfo, forWhom);
}
@Override
public TableDescriptor preModifyTable(ObserverContext<MasterCoprocessorEnvironment> ctx,
- TableName tableName, TableDescriptor currentDescriptor, TableDescriptor newDescriptor)
- throws IOException {
- if (!currentDescriptor.getRegionServerGroup().equals(newDescriptor.getRegionServerGroup())) {
- Supplier<String> 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<String> forWhom = () -> "namespace " + nd.getName();
- RSGroupInfo rsGroupInfo = checkGroupExists(getNamespaceGroup(nd), forWhom);
- checkGroupNotEmpty(rsGroupInfo, forWhom);
- }
-
@Override
public void preCreateNamespace(ObserverContext<MasterCoprocessorEnvironment> 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<MasterCoprocessorEnvironment> 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 aff591f..7471458 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 67ed2a9..e3cb54e 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;
@@ -134,6 +137,45 @@ public class TestRSGroupsBasics extends TestRSGroupsBase {
}
@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<Address> iterator = appInfo.getServers().iterator();
+ List<ServerName> 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<Exception>() {
+ @Override
+ public boolean evaluate() throws Exception {
+ return getTableRegionMap().get(desc.getTableName()) != null;
+ }
+ });
+ }
+
+ @Test
public void testDefaultNamespaceCreateAndAssign() throws Exception {
LOG.info("testDefaultNamespaceCreateAndAssign");
String tableName = tablePrefix + "_testCreateAndAssign";