Apache9 commented on a change in pull request #426: HBASE-22695 Store the
rsgroup of a table in table configuration
URL: https://github.com/apache/hbase/pull/426#discussion_r308626014
##########
File path:
hbase-server/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupAdminEndpoint.java
##########
@@ -110,117 +95,91 @@ RSGroupAdminServiceImpl getGroupAdminService() {
return groupAdminService;
}
- private void assignTableToGroup(TableDescriptor desc) throws IOException {
- String groupName =
-
master.getClusterSchema().getNamespace(desc.getTableName().getNamespaceAsString())
- .getConfigurationValue(RSGroupInfo.NAMESPACE_DESC_PROP_GROUP);
- if (groupName == null) {
- groupName = RSGroupInfo.DEFAULT_GROUP;
- }
- RSGroupInfo rsGroupInfo = groupAdminServer.getRSGroupInfo(groupName);
- if (rsGroupInfo == null) {
- throw new ConstraintException(
- "Default RSGroup (" + groupName + ") for this table's namespace does
not exist.");
- }
- if (!rsGroupInfo.containsTable(desc.getTableName())) {
- LOG.debug("Pre-moving table " + desc.getTableName() + " to RSGroup " +
groupName);
- groupAdminServer.moveTables(Sets.newHashSet(desc.getTableName()),
groupName);
- }
- }
-
/////////////////////////////////////////////////////////////////////////////
// MasterObserver overrides
/////////////////////////////////////////////////////////////////////////////
- private boolean rsgroupHasServersOnline(TableDescriptor desc) throws
IOException {
- String groupName;
- try {
- groupName =
master.getClusterSchema().getNamespace(desc.getTableName().getNamespaceAsString())
- .getConfigurationValue(RSGroupInfo.NAMESPACE_DESC_PROP_GROUP);
- if (groupName == null) {
- groupName = RSGroupInfo.DEFAULT_GROUP;
- }
- } catch (MasterNotRunningException | PleaseHoldException e) {
- LOG.info("Master has not initialized yet; temporarily using default
RSGroup '" +
- RSGroupInfo.DEFAULT_GROUP + "' for deploy of system table");
- groupName = RSGroupInfo.DEFAULT_GROUP;
+ @Override
+ public void
postClearDeadServers(ObserverContext<MasterCoprocessorEnvironment> ctx,
+ List<ServerName> servers, List<ServerName> notClearedServers) throws
IOException {
+ Set<Address> clearedServer =
+ servers.stream().filter(server -> !notClearedServers.contains(server))
+ .map(ServerName::getAddress).collect(Collectors.toSet());
+ if (!clearedServer.isEmpty()) {
+ groupAdminServer.removeServers(clearedServer);
}
+ }
- RSGroupInfo rsGroupInfo = groupAdminServer.getRSGroupInfo(groupName);
- if (rsGroupInfo == null) {
- throw new ConstraintException(
- "Default RSGroup (" + groupName + ") for this table's " + "namespace
does not exist.");
+ private void checkGroupExists(Optional<String> optGroupName) throws
IOException {
+ if (optGroupName.isPresent()) {
+ String groupName = optGroupName.get();
+ if (groupAdminServer.getRSGroupInfo(groupName) == null) {
+ throw new ConstraintException("Region server group " + groupName + "
does not exit");
+ }
}
+ }
- for (ServerName onlineServer :
master.getServerManager().createDestinationServersList()) {
- if (rsGroupInfo.getServers().contains(onlineServer.getAddress())) {
+ 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 false;
+ return master.getServerManager().createDestinationServersList().stream()
+ .anyMatch(onlineServer ->
rsGroupInfo.containsServer(onlineServer.getAddress()));
}
@Override
- public void preCreateTableAction(final
ObserverContext<MasterCoprocessorEnvironment> ctx,
- final TableDescriptor desc, final RegionInfo[] regions) throws
IOException {
+ public void
preCreateTableAction(ObserverContext<MasterCoprocessorEnvironment> ctx,
+ TableDescriptor desc, RegionInfo[] regions) throws IOException {
+ checkGroupExists(desc.getRegionServerGroup());
if (!desc.getTableName().isSystemTable() &&
!rsgroupHasServersOnline(desc)) {
- throw new HBaseIOException("No online servers in the rsgroup, which
table " +
- desc.getTableName().getNameAsString() + " belongs to");
+ throw new HBaseIOException("No online servers in the rsgroup for " +
desc);
Review comment:
Just keep the old behavior. And in general, I do not think we need to check
for online servers here. Region servers can die at any time and cause the rs
group to have no servers in group, so users may get random behaviors and
confused...
----------------------------------------------------------------
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:
us...@infra.apache.org
With regards,
Apache Git Services
