From 6af38612915bed6a476bf63beb7f63351d3231dd Mon Sep 17 00:00:00 2001 From: Mohammad Arshad Date: Thu, 14 Jan 2021 00:07:02 +0530 Subject: [PATCH 1/5] HBASE-25492: Create table with rsgroup --- .../apache/hadoop/hbase/HTableDescriptor.java | 6 + .../hadoop/hbase/client/TableDescriptor.java | 8 + .../hbase/client/TableDescriptorBuilder.java | 19 + .../hadoop/hbase/rsgroup/RSGroupInfo.java | 1 + .../hbase/rsgroup/RSGroupAdminEndpoint.java | 72 +++- .../hbase/rsgroup/RSGroupAdminServer.java | 60 ++- .../TestTableDescriptorWithRSGroup.java | 381 ++++++++++++++++++ 7 files changed, 536 insertions(+), 11 deletions(-) create mode 100644 hbase-rsgroup/src/test/java/org/apache/hadoop/hbase/rsgroup/TestTableDescriptorWithRSGroup.java diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/HTableDescriptor.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/HTableDescriptor.java index 32bf71d37a95..7285846f91f6 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/HTableDescriptor.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/HTableDescriptor.java @@ -22,6 +22,7 @@ import java.util.Collection; import java.util.Collections; import java.util.Map; +import java.util.Optional; import java.util.Set; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -987,4 +988,9 @@ public ColumnFamilyDescriptor getColumnFamily(byte[] name) { protected ModifyableTableDescriptor getDelegateeForModification() { return delegatee; } + + @Override + public Optional getRegionServerGroup() { + return delegatee.getRegionServerGroup(); + } } diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/TableDescriptor.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/TableDescriptor.java index 2ae273d89899..02015e874f8a 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/TableDescriptor.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/TableDescriptor.java @@ -23,6 +23,7 @@ import java.util.Comparator; import java.util.Iterator; import java.util.Map; +import java.util.Optional; import java.util.Set; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -360,4 +361,11 @@ default boolean matchReplicationScope(boolean enabled) { } return !enabled; } + + /** + * Get the region server group this table belongs to. The regions of this table will be placed + * only on the region servers within this group. If not present, will be placed on + * {@link org.apache.hadoop.hbase.rsgroup.RSGroupInfo#DEFAULT_GROUP}. + */ + Optional getRegionServerGroup(); } diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/TableDescriptorBuilder.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/TableDescriptorBuilder.java index 6c3cd8dd4543..96ccfcad9a13 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/TableDescriptorBuilder.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/TableDescriptorBuilder.java @@ -41,6 +41,7 @@ import org.apache.hadoop.hbase.TableName; import org.apache.hadoop.hbase.exceptions.DeserializationException; import org.apache.hadoop.hbase.exceptions.HBaseException; +import org.apache.hadoop.hbase.rsgroup.RSGroupInfo; import org.apache.hadoop.hbase.security.User; import org.apache.hadoop.hbase.util.Bytes; import org.apache.hadoop.hbase.util.PrettyPrinter; @@ -192,6 +193,9 @@ public class TableDescriptorBuilder { private static final Bytes PRIORITY_KEY = new Bytes(Bytes.toBytes(PRIORITY)); + private static final Bytes RSGROUP_KEY = + new Bytes(Bytes.toBytes(RSGroupInfo.TABLE_DESC_PROP_GROUP)); + /** * Relative priority of the table used for rpc scheduling */ @@ -594,6 +598,11 @@ public TableDescriptorBuilder setReplicationScope(int scope) { return this; } + public TableDescriptorBuilder setRegionServerGroup(String group) { + desc.setValue(RSGROUP_KEY, group); + return this; + } + public TableDescriptor build() { return new ModifyableTableDescriptor(desc); } @@ -1647,6 +1656,16 @@ private static TableDescriptor parseFrom(final byte[] bytes) public int getColumnFamilyCount() { return families.size(); } + + @Override + public Optional getRegionServerGroup() { + Bytes value = values.get(RSGROUP_KEY); + if (value != null) { + return Optional.of(Bytes.toString(value.get(), value.getOffset(), value.getLength())); + } else { + return Optional.empty(); + } + } } private static Optional toCoprocessorDescriptor(String spec) { diff --git a/hbase-common/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupInfo.java b/hbase-common/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupInfo.java index e183a619ac77..65314cb10d04 100644 --- a/hbase-common/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupInfo.java +++ b/hbase-common/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupInfo.java @@ -38,6 +38,7 @@ public class RSGroupInfo { public static final String DEFAULT_GROUP = "default"; public static final String NAMESPACE_DESC_PROP_GROUP = "hbase.rsgroup.name"; + public static final String TABLE_DESC_PROP_GROUP = "hbase.rsgroup.name"; private final String name; // Keep servers in a sorted set so has an expected ordering when displayed. 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 55cf88d0e23d..bf26de170626 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 @@ -485,7 +485,7 @@ boolean rsgroupHasServersOnline(TableDescriptor desc) throws IOException { } void assignTableToGroup(TableDescriptor desc) throws IOException { - RSGroupInfo rsGroupInfo = groupInfoManager.determineRSGroupInfoForTable(desc.getTableName()); + RSGroupInfo rsGroupInfo = determineRSGroupInfoForTable(desc); if (rsGroupInfo == null) { throw new ConstraintException("Default RSGroup for this table " + desc.getTableName() + " does not exist."); @@ -508,21 +508,75 @@ public void preCreateTableAction( if (desc.getTableName().isSystemTable()) { return; } - RSGroupInfo rsGroupInfo = groupInfoManager.determineRSGroupInfoForTable(desc.getTableName()); + moveTableToValidRSGroup(desc); + } + + @Override + public void preModifyTableAction(ObserverContext ctx, + TableName tableName, TableDescriptor currentDescriptor, TableDescriptor newDescriptor) + throws IOException { + // If table's rsgroup is changed, it must be valid + if (!currentDescriptor.getRegionServerGroup().equals(newDescriptor.getRegionServerGroup())) { + RSGroupInfo rsGroupInfo = determineRSGroupInfoForTable(newDescriptor); + validateRSGroup(newDescriptor, rsGroupInfo); + } + } + + @Override + public void postCompletedModifyTableAction(ObserverContext ctx, + TableName tableName, TableDescriptor oldDescriptor, TableDescriptor currentDescriptor) + throws IOException { + // If table's rsgroup is changed, move table into the rsgroup. + if (!oldDescriptor.getRegionServerGroup().equals(currentDescriptor.getRegionServerGroup())) { + RSGroupInfo rsGroupInfo = determineRSGroupInfoForTable(currentDescriptor); + moveTableToRSGroup(currentDescriptor, rsGroupInfo); + } + } + + // Determine and validate rs group then move table to this valid rs group. + private void moveTableToValidRSGroup(TableDescriptor desc) throws IOException { + RSGroupInfo rsGroupInfo = determineRSGroupInfoForTable(desc); + validateRSGroup(desc, rsGroupInfo); + moveTableToRSGroup(desc, rsGroupInfo); + } + + private void validateRSGroup(TableDescriptor desc, RSGroupInfo rsGroupInfo) throws IOException { if (rsGroupInfo == null) { - throw new ConstraintException("Default RSGroup for this table " + desc.getTableName() - + " does not exist."); + throw new ConstraintException( + "Default RSGroup for this table " + desc.getTableName() + " does not exist."); } if (!RSGroupUtil.rsGroupHasOnlineServer(master, rsGroupInfo)) { - throw new HBaseIOException("No online servers in the rsgroup " + rsGroupInfo.getName() - + " which table " + desc.getTableName().getNameAsString() + " belongs to"); + throw new HBaseIOException( + "No online servers in the rsgroup " + rsGroupInfo.getName() + " which table " + desc + .getTableName().getNameAsString() + " belongs to"); } - synchronized (groupInfoManager) { - groupInfoManager.moveTables( - Collections.singleton(desc.getTableName()), rsGroupInfo.getName()); + } + + private void moveTableToRSGroup(final TableDescriptor desc, RSGroupInfo rsGroupInfo) + throws IOException { + // In case of modify table, when rs group is not changed, move is not required. + if (!rsGroupInfo.containsTable(desc.getTableName())) { + synchronized (groupInfoManager) { + groupInfoManager + .moveTables(Collections.singleton(desc.getTableName()), rsGroupInfo.getName()); + } } } + private RSGroupInfo determineRSGroupInfoForTable(final TableDescriptor desc) throws IOException { + Optional optGroupNameOfTable = desc.getRegionServerGroup(); + if (optGroupNameOfTable.isPresent()) { + final RSGroupInfo rsGroup = groupInfoManager.getRSGroup(optGroupNameOfTable.get()); + if (rsGroup == null) { + // When rs group is set in table descriptor then it must exist + throw new ConstraintException( + "Region server group " + optGroupNameOfTable.get() + " does not exist."); + } else { + return rsGroup; + } + } + return groupInfoManager.determineRSGroupInfoForTable(desc.getTableName()); + } // Remove table from its RSGroup. @Override public void postDeleteTable(ObserverContext ctx, 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 b284a2b7586a..013e17528133 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 @@ -27,11 +27,15 @@ import java.util.Map; import java.util.Set; import java.util.concurrent.Future; +import java.util.stream.Collectors; import org.apache.commons.lang3.StringUtils; +import org.apache.hadoop.hbase.HConstants; 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.TableDescriptor; +import org.apache.hadoop.hbase.client.TableDescriptorBuilder; import org.apache.hadoop.hbase.client.TableState; import org.apache.hadoop.hbase.constraint.ConstraintException; import org.apache.hadoop.hbase.master.HMaster; @@ -43,7 +47,9 @@ 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.master.procedure.ProcedureSyncWait; import org.apache.hadoop.hbase.net.Address; +import org.apache.hadoop.hbase.procedure2.Procedure; import org.apache.hadoop.hbase.util.Pair; import org.apache.hbase.thirdparty.com.google.common.collect.Maps; import org.apache.yetus.audience.InterfaceAudience; @@ -459,7 +465,7 @@ public void moveTables(Set tables, String targetGroup) throws IOExcep // targetGroup is null when a table is being deleted. In this case no further // action is required. if (targetGroup != null) { - moveTableRegionsToGroup(tables, rsGroupInfoManager.getRSGroup(targetGroup)); + moveOrModifyTables(tables, rsGroupInfoManager.getRSGroup(targetGroup)); } } } @@ -582,7 +588,7 @@ public void moveServersAndTables(Set
servers, Set tables, St rsGroupInfoManager.getRSGroup(srcGroup).getServers(), targetGroup, srcGroup); //move regions of these tables which are not on group servers - moveTableRegionsToGroup(tables, rsGroupInfoManager.getRSGroup(targetGroup)); + moveOrModifyTables(tables, rsGroupInfoManager.getRSGroup(targetGroup)); } LOG.info("Move servers and tables done. Severs: {}, Tables: {} => {}", servers, tables, targetGroup); @@ -609,6 +615,11 @@ public void removeServers(Set
servers) throws IOException { public void renameRSGroup(String oldName, String newName) throws IOException { synchronized (rsGroupInfoManager) { rsGroupInfoManager.renameRSGroup(oldName, newName); + Set updateTables = master.getTableDescriptors().getAll().values().stream() + .filter(t -> oldName.equals(t.getRegionServerGroup().orElse(null))) + .collect(Collectors.toSet()); + // Update rs group info into table descriptors + modifyTables(updateTables, newName); } } @@ -718,4 +729,49 @@ private void checkForDeadOrOnlineServers(Set
servers) throws Constraint } } } + + private void moveOrModifyTables(Set tables, RSGroupInfo targetGroup) + throws IOException { + Set tablesToBeMoved = new HashSet<>(tables.size()); + Set tablesToBeModified = new HashSet<>(tables.size()); + for (TableName tableName : tables) { + TableDescriptor descriptor = master.getTableDescriptors().get(tableName); + if (descriptor == null) { + LOG.error("TableDescriptor of table {} not found. " + + "Skipping the region movement of this table."); + continue; + } + if (descriptor.getRegionServerGroup().isPresent()) { + tablesToBeModified.add(descriptor); + } else { + tablesToBeMoved.add(tableName); + } + } + if (!tablesToBeMoved.isEmpty()) { + moveTableRegionsToGroup(tablesToBeMoved, targetGroup); + } + if (!tablesToBeModified.isEmpty()) { + modifyTables(tablesToBeModified, targetGroup.getName()); + } + } + + // Modify table internally moves the regions as well. So separate region movement is not needed + private void modifyTables(Set tableDescriptors, String targetGroup) + throws IOException { + List procIds = new ArrayList<>(tableDescriptors.size()); + for (TableDescriptor oldTd : tableDescriptors) { + TableDescriptor newTd = + TableDescriptorBuilder.newBuilder(oldTd).setRegionServerGroup(targetGroup).build(); + procIds.add(master + .modifyTable(oldTd.getTableName(), newTd, HConstants.NO_NONCE, HConstants.NO_NONCE)); + } + for (long procId : procIds) { + Procedure proc = master.getMasterProcedureExecutor().getProcedure(procId); + if (proc == null) { + continue; + } + ProcedureSyncWait + .waitForProcedureToCompleteIOE(master.getMasterProcedureExecutor(), proc, Long.MAX_VALUE); + } + } } diff --git a/hbase-rsgroup/src/test/java/org/apache/hadoop/hbase/rsgroup/TestTableDescriptorWithRSGroup.java b/hbase-rsgroup/src/test/java/org/apache/hadoop/hbase/rsgroup/TestTableDescriptorWithRSGroup.java new file mode 100644 index 000000000000..a711d5603d0a --- /dev/null +++ b/hbase-rsgroup/src/test/java/org/apache/hadoop/hbase/rsgroup/TestTableDescriptorWithRSGroup.java @@ -0,0 +1,381 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.hadoop.hbase.rsgroup; + +import org.apache.hadoop.hbase.DoNotRetryIOException; +import org.apache.hadoop.hbase.HBaseClassTestRule; +import org.apache.hadoop.hbase.TableName; +import org.apache.hadoop.hbase.Waiter; +import org.apache.hadoop.hbase.client.ColumnFamilyDescriptor; +import org.apache.hadoop.hbase.client.ColumnFamilyDescriptorBuilder; +import org.apache.hadoop.hbase.client.SnapshotDescription; +import org.apache.hadoop.hbase.client.TableDescriptor; +import org.apache.hadoop.hbase.client.TableDescriptorBuilder; +import org.apache.hadoop.hbase.constraint.ConstraintException; +import org.apache.hadoop.hbase.testclassification.LargeTests; +import org.apache.hadoop.hbase.util.Bytes; +import org.junit.After; +import org.junit.AfterClass; +import org.junit.Before; +import org.junit.BeforeClass; +import org.junit.ClassRule; +import org.junit.Test; +import org.junit.experimental.categories.Category; + +import java.io.IOException; +import java.util.List; +import java.util.Optional; +import java.util.regex.Pattern; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; +import org.apache.hbase.thirdparty.com.google.common.collect.Sets; + +@Category({ LargeTests.class }) +public class TestTableDescriptorWithRSGroup extends TestRSGroupsBase { + + @ClassRule + public static final HBaseClassTestRule CLASS_RULE = + HBaseClassTestRule.forClass(TestTableDescriptorWithRSGroup.class); + private final byte[] familyNameBytes = Bytes.toBytes("f1"); + + @BeforeClass + public static void setUp() throws Exception { + setUpTestBeforeClass(); + } + + @AfterClass + public static void tearDown() throws Exception { + tearDownAfterClass(); + } + + @Before + public void beforeMethod() throws Exception { + setUpBeforeMethod(); + } + + @After + public void afterMethod() throws Exception { + tearDownAfterMethod(); + } + + @Test + public void testCreateTableInTableDescriptorSpecificRSGroup() throws Exception { + final RSGroupInfo newGroup = addGroup(getGroupName(name.getMethodName()), 1); + assertEquals(0, newGroup.getTables().size()); + + // assertion is done in createTableInRSGroup + createTableWithRSGroupDetail(newGroup.getName()); + + // Create table should fail if specified rs group does not exist. + try { + TableDescriptor desc = TableDescriptorBuilder.newBuilder(TableName.valueOf("newTable")) + .setRegionServerGroup("nonExistingRSGroup") + .setColumnFamily(ColumnFamilyDescriptorBuilder.newBuilder(Bytes.toBytes("f1")).build()) + .build(); + admin.createTable(desc, getSpitKeys(6)); + } catch (ConstraintException e) { + assertEquals(e.getMessage(), "Region server group nonExistingRSGroup does not exist."); + } + } + + private void createTableWithRSGroupDetail(String newGroup) throws Exception { + // Create table + + ColumnFamilyDescriptor f1 = ColumnFamilyDescriptorBuilder.newBuilder(familyNameBytes).build(); + TableDescriptor desc = + TableDescriptorBuilder.newBuilder(tableName).setRegionServerGroup(newGroup) + .setColumnFamily(f1).build(); + admin.createTable(desc, getSpitKeys(5)); + + TEST_UTIL.waitFor(WAIT_TIMEOUT, (Waiter.Predicate) () -> { + List regions = getTableRegionMap().get(tableName); + if (regions == null) { + return false; + } + + return getTableRegionMap().get(tableName).size() >= 5; + }); + TableDescriptor descriptor = admin.getConnection().getTable(tableName).getDescriptor(); + Optional regionServerGroup = descriptor.getRegionServerGroup(); + assertTrue("RSGroup info is not updated into TableDescriptor when table is created.", + regionServerGroup.isPresent()); + assertEquals(newGroup, regionServerGroup.get()); + } + + // moveTables should update rs group info in table descriptor + @Test + public void testMoveTablesShouldUpdateTableDescriptor() throws Exception { + RSGroupInfo rsGroup1 = addGroup("rsGroup1", 1); + assertEquals(0, rsGroup1.getTables().size()); + + createTableWithRSGroupDetail(rsGroup1.getName()); + TableDescriptor descriptor = admin.getConnection().getTable(tableName).getDescriptor(); + Optional regionServerGroup = descriptor.getRegionServerGroup(); + assertTrue("RSGroup info is not updated into TableDescriptor when table created", + regionServerGroup.isPresent()); + assertEquals(rsGroup1.getName(), regionServerGroup.get()); + + // moveTables + RSGroupInfo rsGroup2 = addGroup("rsGroup2", 1); + rsGroupAdmin.moveTables(Sets.newHashSet(tableName), rsGroup2.getName()); + descriptor = admin.getConnection().getTable(tableName).getDescriptor(); + regionServerGroup = descriptor.getRegionServerGroup(); + assertTrue("RSGroup info is not updated into TableDescriptor when table moved", + regionServerGroup.isPresent()); + assertEquals(rsGroup2.getName(), regionServerGroup.get()); + } + + // moveServersAndTables should update rs group info in table descriptor + @Test + public void testMoveServersAndTablesShouldUpdateTableDescriptor() throws Exception { + RSGroupInfo rsGroup1 = addGroup("rsGroup1", 2); + assertEquals(0, rsGroup1.getTables().size()); + + createTableWithRSGroupDetail(rsGroup1.getName()); + TableDescriptor descriptor = admin.getConnection().getTable(tableName).getDescriptor(); + Optional regionServerGroup = descriptor.getRegionServerGroup(); + assertTrue("RSGroup info is not updated into TableDescriptor when table created", + regionServerGroup.isPresent()); + assertEquals(rsGroup1.getName(), regionServerGroup.get()); + + // moveServersAndTables + rsGroupAdmin.moveServersAndTables(Sets.newHashSet(rsGroup1.getServers().iterator().next()), + Sets.newHashSet(tableName), RSGroupInfo.DEFAULT_GROUP); + + descriptor = admin.getConnection().getTable(tableName).getDescriptor(); + regionServerGroup = descriptor.getRegionServerGroup(); + assertTrue("RSGroup info is not updated into TableDescriptor when table moved", + regionServerGroup.isPresent()); + assertEquals(RSGroupInfo.DEFAULT_GROUP, regionServerGroup.get()); + + } + + @Test + public void testRenameRSGroupUpdatesTableDescriptor() throws Exception { + RSGroupInfo oldGroup = addGroup("oldGroup", 1); + assertEquals(0, oldGroup.getTables().size()); + + createTableWithRSGroupDetail(oldGroup.getName()); + + final String newGroupName = "newGroup"; + rsGroupAdmin.renameRSGroup(oldGroup.getName(), newGroupName); + TableDescriptor descriptor = admin.getConnection().getTable(tableName).getDescriptor(); + Optional regionServerGroup = descriptor.getRegionServerGroup(); + assertTrue("RSGroup info is not updated into TableDescriptor when rs group renamed", + regionServerGroup.isPresent()); + assertEquals(newGroupName, regionServerGroup.get()); + } + + @Test + public void testCloneSnapshotWithRSGroup() throws Exception { + RSGroupInfo rsGroup1 = addGroup("rsGroup1", 1); + assertEquals(0, rsGroup1.getTables().size()); + // Creates table in rsGroup1 + createTableWithRSGroupDetail(rsGroup1.getName()); + + // Create snapshot + final String snapshotName = "snapShot2"; + admin.snapshot(snapshotName, tableName); + final List snapshotDescriptions = + admin.listSnapshots(Pattern.compile("snapShot2")); + assertEquals(1, snapshotDescriptions.size()); + assertEquals(snapshotName, snapshotDescriptions.get(0).getName()); + + // Move table to different rs group then delete the old rs group + RSGroupInfo rsGroup2 = addGroup("rsGroup2", 1); + rsGroupAdmin.moveTables(Sets.newHashSet(tableName), rsGroup2.getName()); + rsGroup2 = rsGroupAdmin.getRSGroupInfo(rsGroup2.getName()); + assertEquals(1, rsGroup2.getTables().size()); + assertEquals(rsGroup2.getTables().first(), tableName); + + // Clone Snapshot + final TableName clonedTable1 = TableName.valueOf("clonedTable1"); + admin.cloneSnapshot(snapshotName, clonedTable1); + + // Verify that cloned table is created into old rs group + final RSGroupInfo rsGroupInfoOfTable = rsGroupAdmin.getRSGroupInfoOfTable(clonedTable1); + assertEquals(rsGroup1.getName(), rsGroupInfoOfTable.getName()); + TableDescriptor descriptor = admin.getConnection().getTable(clonedTable1).getDescriptor(); + Optional regionServerGroup = descriptor.getRegionServerGroup(); + assertTrue("RSGroup info is not updated into TableDescriptor when table is cloned.", + regionServerGroup.isPresent()); + assertEquals(rsGroup1.getName(), regionServerGroup.get()); + + // Delete table's original rsgroup, clone should fail. + rsGroupAdmin + .moveServersAndTables(Sets.newHashSet(rsGroup1.getServers()), Sets.newHashSet(clonedTable1), + rsGroup2.getName()); + rsGroupAdmin.removeRSGroup(rsGroup1.getName()); + // Clone Snapshot + final TableName clonedTable2 = TableName.valueOf("clonedTable2"); + try { + admin.cloneSnapshot(snapshotName, clonedTable2); + } catch (ConstraintException e) { + assertTrue( + e.getCause().getMessage().contains("Region server group rsGroup1 does not exist.")); + } + } + + // Modify table should validate rs group existence and move rs group if required + @Test + public void testMoveTablesWhenModifyTableWithNewRSGroup() throws Exception { + RSGroupInfo rsGroup1 = addGroup("rsGroup1", 1); + assertEquals(0, rsGroup1.getTables().size()); + + createTableWithRSGroupDetail(rsGroup1.getName()); + TableDescriptor descriptor = admin.getConnection().getTable(tableName).getDescriptor(); + Optional regionServerGroup = descriptor.getRegionServerGroup(); + assertTrue("RSGroup info is not updated into TableDescriptor when table created", + regionServerGroup.isPresent()); + assertEquals(rsGroup1.getName(), regionServerGroup.get()); + + final TableDescriptor newTableDescriptor = + TableDescriptorBuilder.newBuilder(descriptor).setRegionServerGroup("rsGroup2").build(); + + // ConstraintException as rsGroup2 does not exits + try { + admin.modifyTable(newTableDescriptor); + } catch (ConstraintException e) { + assertTrue( + e.getCause().getMessage().contains("Region server group rsGroup2 does not exist.")); + } + + addGroup("rsGroup2", 1); + // Table creation should be successful as rsGroup2 exists. + admin.modifyTable(newTableDescriptor); + + // old group should not have table mapping now + rsGroup1 = rsGroupAdmin.getRSGroupInfo("rsGroup1"); + assertEquals(0, rsGroup1.getTables().size()); + + RSGroupInfo rsGroup2 = rsGroupAdmin.getRSGroupInfo("rsGroup2"); + assertEquals(1, rsGroup2.getTables().size()); + descriptor = admin.getConnection().getTable(tableName).getDescriptor(); + regionServerGroup = descriptor.getRegionServerGroup(); + assertTrue("RSGroup info is not updated into TableDescriptor when table is modified.", + regionServerGroup.isPresent()); + assertEquals("rsGroup2", regionServerGroup.get()); + } + + @Test + public void testTableShouldNotAddedIntoRSGroup_WhenModifyTableFails() throws Exception { + RSGroupInfo rsGroup1 = addGroup("rsGroup1", 1); + assertEquals(0, rsGroup1.getTables().size()); + + createTableWithRSGroupDetail(rsGroup1.getName()); + TableDescriptor descriptor = admin.getConnection().getTable(tableName).getDescriptor(); + + RSGroupInfo rsGroup2 = addGroup("rsGroup2", 1); + final TableDescriptor newTableDescriptor = + TableDescriptorBuilder.newBuilder(descriptor).setRegionServerGroup(rsGroup2.getName()) + .removeColumnFamily(familyNameBytes).build(); + + // Removed family to fail pre-check validation + try { + admin.modifyTable(newTableDescriptor); + } catch (DoNotRetryIOException e) { + assertTrue( + e.getCause().getMessage().contains("Table should have at least one column family")); + } + rsGroup2 = rsGroupAdmin.getRSGroupInfo(rsGroup2.getName()); + assertEquals("Table must not have moved to RSGroup as table modify failed", 0, + rsGroup2.getTables().size()); + } + + @Test + public void testTableShouldNotAddedIntoRSGroup_WhenTableCreationFails() throws Exception { + RSGroupInfo rsGroup1 = addGroup("rsGroup1", 1); + assertEquals(0, rsGroup1.getTables().size()); + + // Create TableDescriptor without a family so creation fails + TableDescriptor desc = + TableDescriptorBuilder.newBuilder(tableName).setRegionServerGroup(rsGroup1.getName()).build(); + try { + admin.createTable(desc, getSpitKeys(5)); + } catch (DoNotRetryIOException e) { + assertTrue( + e.getCause().getMessage().contains("Table should have at least one column family")); + } + rsGroup1 = rsGroupAdmin.getRSGroupInfo(rsGroup1.getName()); + assertEquals("Table must not have moved to RSGroup as table create operation failed", 0, + rsGroup1.getTables().size()); + } + + @Test + public void testSystemTablesCanBeMovedToNewRSGroupByModifyingTable() throws Exception { + RSGroupInfo rsGroup1 = addGroup("rsGroup1", 1); + assertEquals(0, rsGroup1.getTables().size()); + final TableName tableName = TableName.valueOf("hbase:meta"); + final TableDescriptor descriptor = admin.getConnection().getTable(tableName).getDescriptor(); + final TableDescriptor newTableDescriptor = + TableDescriptorBuilder.newBuilder(descriptor).setRegionServerGroup(rsGroup1.getName()) + .build(); + admin.modifyTable(newTableDescriptor); + final RSGroupInfo rsGroupInfoOfTable = rsGroupAdmin.getRSGroupInfoOfTable(tableName); + assertEquals(rsGroup1.getName(), rsGroupInfoOfTable.getName()); + } + + @Test + public void testUpdateTableDescriptorOnlyIfRSGroupInfoWasStoredInTableDescriptor() + throws Exception { + RSGroupInfo rsGroup1 = addGroup("rsGroup1", 1); + assertEquals(0, rsGroup1.getTables().size()); + // create table with rsgorup info stored in table descriptor + createTable(tableName, rsGroup1.getName()); + final TableName table2 = TableName.valueOf("table2"); + // create table with no rsgorup info + createTable(table2, null); + rsGroupAdmin.moveTables(Sets.newHashSet(tableName), rsGroup1.getName()); + assertTrue("RSGroup info is not updated into TableDescriptor when table created", + admin.getConnection().getTable(tableName).getDescriptor().getRegionServerGroup().isPresent()); + assertFalse("Table descriptor should not have been updated " + + "as rs group info was not stored in table descriptor.", + admin.getConnection().getTable(table2).getDescriptor().getRegionServerGroup().isPresent()); + + final String rsGroup2 = "rsGroup2"; + rsGroupAdmin.renameRSGroup(rsGroup1.getName(), rsGroup2); + assertEquals(rsGroup2, + admin.getConnection().getTable(tableName).getDescriptor().getRegionServerGroup().get()); + assertFalse("Table descriptor should not have been updated " + + "as rs group info was not stored in table descriptor.", + admin.getConnection().getTable(table2).getDescriptor().getRegionServerGroup().isPresent()); + } + + private void createTable(TableName tableName, String rsGroupName) throws IOException { + ColumnFamilyDescriptor f1 = ColumnFamilyDescriptorBuilder.newBuilder(familyNameBytes).build(); + final TableDescriptorBuilder builder = + TableDescriptorBuilder.newBuilder(tableName).setColumnFamily(f1); + if (rsGroupName != null) { + builder.setRegionServerGroup(rsGroupName); + } + TableDescriptor desc = builder.build(); + admin.createTable(desc); + } + + private byte[][] getSpitKeys(int numRegions) throws IOException { + if (numRegions < 3) { + throw new IOException("Must create at least 3 regions"); + } + byte[] startKey = Bytes.toBytes("aaaaa"); + byte[] endKey = Bytes.toBytes("zzzzz"); + return Bytes.split(startKey, endKey, numRegions - 3); + } +} From a9e455a474fd669f484e465302579636e825f95d Mon Sep 17 00:00:00 2001 From: Mohammad Arshad Date: Wed, 3 Mar 2021 17:32:16 +0530 Subject: [PATCH 2/5] Fixed review comment, executing move table and modify table in parallel --- .../hbase/rsgroup/RSGroupAdminServer.java | 36 ++++++++--- .../TestTableDescriptorWithRSGroup.java | 62 ++++++++++++++----- 2 files changed, 71 insertions(+), 27 deletions(-) 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 013e17528133..0c65dbf0f53f 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 @@ -465,7 +465,7 @@ public void moveTables(Set tables, String targetGroup) throws IOExcep // targetGroup is null when a table is being deleted. In this case no further // action is required. if (targetGroup != null) { - moveOrModifyTables(tables, rsGroupInfoManager.getRSGroup(targetGroup)); + modifyOrMoveTables(tables, rsGroupInfoManager.getRSGroup(targetGroup)); } } } @@ -588,7 +588,7 @@ public void moveServersAndTables(Set
servers, Set tables, St rsGroupInfoManager.getRSGroup(srcGroup).getServers(), targetGroup, srcGroup); //move regions of these tables which are not on group servers - moveOrModifyTables(tables, rsGroupInfoManager.getRSGroup(targetGroup)); + modifyOrMoveTables(tables, rsGroupInfoManager.getRSGroup(targetGroup)); } LOG.info("Move servers and tables done. Severs: {}, Tables: {} => {}", servers, tables, targetGroup); @@ -619,7 +619,7 @@ public void renameRSGroup(String oldName, String newName) throws IOException { .filter(t -> oldName.equals(t.getRegionServerGroup().orElse(null))) .collect(Collectors.toSet()); // Update rs group info into table descriptors - modifyTables(updateTables, newName); + modifyTablesAndWaitForCompletion(updateTables, newName); } } @@ -730,15 +730,16 @@ private void checkForDeadOrOnlineServers(Set
servers) throws Constraint } } - private void moveOrModifyTables(Set tables, RSGroupInfo targetGroup) - throws IOException { + // Modify table or move table's regions + void modifyOrMoveTables(Set tables, RSGroupInfo targetGroup) throws IOException { Set tablesToBeMoved = new HashSet<>(tables.size()); Set tablesToBeModified = new HashSet<>(tables.size()); + // Segregate tables into to be modified or to be moved category for (TableName tableName : tables) { TableDescriptor descriptor = master.getTableDescriptors().get(tableName); if (descriptor == null) { - LOG.error("TableDescriptor of table {} not found. " - + "Skipping the region movement of this table."); + LOG.error( + "TableDescriptor of table {} not found. Skipping the region movement of this table."); continue; } if (descriptor.getRegionServerGroup().isPresent()) { @@ -747,16 +748,27 @@ private void moveOrModifyTables(Set tables, RSGroupInfo targetGroup) tablesToBeMoved.add(tableName); } } + List procedureIds = null; + if (!tablesToBeModified.isEmpty()) { + procedureIds = modifyTables(tablesToBeModified, targetGroup.getName()); + } if (!tablesToBeMoved.isEmpty()) { moveTableRegionsToGroup(tablesToBeMoved, targetGroup); } - if (!tablesToBeModified.isEmpty()) { - modifyTables(tablesToBeModified, targetGroup.getName()); + // By this time moveTableRegionsToGroup is finished, lets wait for modifyTables completion + if (procedureIds != null) { + waitForProcedureCompletion(procedureIds); } } + private void modifyTablesAndWaitForCompletion(Set tableDescriptors, + String targetGroup) throws IOException { + final List procIds = modifyTables(tableDescriptors, targetGroup); + waitForProcedureCompletion(procIds); + } + // Modify table internally moves the regions as well. So separate region movement is not needed - private void modifyTables(Set tableDescriptors, String targetGroup) + private List modifyTables(Set tableDescriptors, String targetGroup) throws IOException { List procIds = new ArrayList<>(tableDescriptors.size()); for (TableDescriptor oldTd : tableDescriptors) { @@ -765,6 +777,10 @@ private void modifyTables(Set tableDescriptors, String targetGr procIds.add(master .modifyTable(oldTd.getTableName(), newTd, HConstants.NO_NONCE, HConstants.NO_NONCE)); } + return procIds; + } + + private void waitForProcedureCompletion(List procIds) throws IOException { for (long procId : procIds) { Procedure proc = master.getMasterProcedureExecutor().getProcedure(procId); if (proc == null) { diff --git a/hbase-rsgroup/src/test/java/org/apache/hadoop/hbase/rsgroup/TestTableDescriptorWithRSGroup.java b/hbase-rsgroup/src/test/java/org/apache/hadoop/hbase/rsgroup/TestTableDescriptorWithRSGroup.java index a711d5603d0a..aeb2ab0dceb0 100644 --- a/hbase-rsgroup/src/test/java/org/apache/hadoop/hbase/rsgroup/TestTableDescriptorWithRSGroup.java +++ b/hbase-rsgroup/src/test/java/org/apache/hadoop/hbase/rsgroup/TestTableDescriptorWithRSGroup.java @@ -86,10 +86,11 @@ public void testCreateTableInTableDescriptorSpecificRSGroup() throws Exception { // Create table should fail if specified rs group does not exist. try { - TableDescriptor desc = TableDescriptorBuilder.newBuilder(TableName.valueOf("newTable")) - .setRegionServerGroup("nonExistingRSGroup") - .setColumnFamily(ColumnFamilyDescriptorBuilder.newBuilder(Bytes.toBytes("f1")).build()) - .build(); + TableDescriptor desc = + TableDescriptorBuilder.newBuilder(TableName.valueOf(tableName.getNameAsString() + "_2")) + .setRegionServerGroup("nonExistingRSGroup") + .setColumnFamily(ColumnFamilyDescriptorBuilder.newBuilder(Bytes.toBytes("f1")).build()) + .build(); admin.createTable(desc, getSpitKeys(6)); } catch (ConstraintException e) { assertEquals(e.getMessage(), "Region server group nonExistingRSGroup does not exist."); @@ -207,8 +208,8 @@ public void testCloneSnapshotWithRSGroup() throws Exception { assertEquals(rsGroup2.getTables().first(), tableName); // Clone Snapshot - final TableName clonedTable1 = TableName.valueOf("clonedTable1"); - admin.cloneSnapshot(snapshotName, clonedTable1); + final TableName clonedTable1 = TableName.valueOf(tableName.getNameAsString() + "_1"); + admin.cloneSnapshot(Bytes.toBytes(snapshotName), clonedTable1); // Verify that cloned table is created into old rs group final RSGroupInfo rsGroupInfoOfTable = rsGroupAdmin.getRSGroupInfoOfTable(clonedTable1); @@ -219,15 +220,15 @@ public void testCloneSnapshotWithRSGroup() throws Exception { regionServerGroup.isPresent()); assertEquals(rsGroup1.getName(), regionServerGroup.get()); - // Delete table's original rsgroup, clone should fail. + // Delete table's original rs group, clone should fail. rsGroupAdmin .moveServersAndTables(Sets.newHashSet(rsGroup1.getServers()), Sets.newHashSet(clonedTable1), rsGroup2.getName()); rsGroupAdmin.removeRSGroup(rsGroup1.getName()); // Clone Snapshot - final TableName clonedTable2 = TableName.valueOf("clonedTable2"); + final TableName clonedTable2 = TableName.valueOf(tableName.getNameAsString() + "_2"); try { - admin.cloneSnapshot(snapshotName, clonedTable2); + admin.cloneSnapshot(Bytes.toBytes(snapshotName), clonedTable2); } catch (ConstraintException e) { assertTrue( e.getCause().getMessage().contains("Region server group rsGroup1 does not exist.")); @@ -307,7 +308,8 @@ public void testTableShouldNotAddedIntoRSGroup_WhenTableCreationFails() throws E // Create TableDescriptor without a family so creation fails TableDescriptor desc = - TableDescriptorBuilder.newBuilder(tableName).setRegionServerGroup(rsGroup1.getName()).build(); + TableDescriptorBuilder.newBuilder(tableName).setRegionServerGroup(rsGroup1.getName()) + .build(); try { admin.createTable(desc, getSpitKeys(5)); } catch (DoNotRetryIOException e) { @@ -338,14 +340,15 @@ public void testUpdateTableDescriptorOnlyIfRSGroupInfoWasStoredInTableDescriptor throws Exception { RSGroupInfo rsGroup1 = addGroup("rsGroup1", 1); assertEquals(0, rsGroup1.getTables().size()); - // create table with rsgorup info stored in table descriptor + // create table with rs group info stored in table descriptor createTable(tableName, rsGroup1.getName()); - final TableName table2 = TableName.valueOf("table2"); - // create table with no rsgorup info + final TableName table2 = TableName.valueOf(tableName.getNameAsString() + "_2"); + // create table with no rs group info createTable(table2, null); rsGroupAdmin.moveTables(Sets.newHashSet(tableName), rsGroup1.getName()); assertTrue("RSGroup info is not updated into TableDescriptor when table created", - admin.getConnection().getTable(tableName).getDescriptor().getRegionServerGroup().isPresent()); + admin.getConnection().getTable(tableName).getDescriptor().getRegionServerGroup() + .isPresent()); assertFalse("Table descriptor should not have been updated " + "as rs group info was not stored in table descriptor.", admin.getConnection().getTable(table2).getDescriptor().getRegionServerGroup().isPresent()); @@ -359,15 +362,40 @@ public void testUpdateTableDescriptorOnlyIfRSGroupInfoWasStoredInTableDescriptor admin.getConnection().getTable(table2).getDescriptor().getRegionServerGroup().isPresent()); } - private void createTable(TableName tableName, String rsGroupName) throws IOException { + @Test + public void testModifyAndMoveTableScenario() throws Exception { + RSGroupInfo rsGroup1 = addGroup("rsGroup1", 1); + assertEquals(0, rsGroup1.getTables().size()); + // create table with rs group info stored in table descriptor + createTable(tableName, rsGroup1.getName()); + final TableName table2 = TableName.valueOf(tableName.getNameAsString() + "_2"); + // create table with no rs group info + createTable(table2, null); + rsGroupAdmin.moveTables(Sets.newHashSet(table2), rsGroup1.getName()); + + RSGroupInfo rsGroup2 = addGroup("rsGroup2", 1); + rsGroupAdmin.moveTables(Sets.newHashSet(tableName, table2), rsGroup2.getName()); + rsGroup2 = rsGroupAdmin.getRSGroupInfo(rsGroup2.getName()); + assertEquals("Table movement failed.", 2, rsGroup2.getTables().size()); + } + + private void createTable(TableName tName, String rsGroupName) throws Exception { ColumnFamilyDescriptor f1 = ColumnFamilyDescriptorBuilder.newBuilder(familyNameBytes).build(); final TableDescriptorBuilder builder = - TableDescriptorBuilder.newBuilder(tableName).setColumnFamily(f1); + TableDescriptorBuilder.newBuilder(tName).setColumnFamily(f1); if (rsGroupName != null) { builder.setRegionServerGroup(rsGroupName); } TableDescriptor desc = builder.build(); - admin.createTable(desc); + admin.createTable(desc, getSpitKeys(10)); + TEST_UTIL.waitFor(WAIT_TIMEOUT, (Waiter.Predicate) () -> { + List regions = getTableRegionMap().get(tName); + if (regions == null) { + return false; + } + + return getTableRegionMap().get(tName).size() >= 5; + }); } private byte[][] getSpitKeys(int numRegions) throws IOException { From dab2c685db2ab8c23a56208c9dd64d0a041ba833 Mon Sep 17 00:00:00 2001 From: Mohammad Arshad Date: Fri, 5 Mar 2021 10:30:15 +0530 Subject: [PATCH 3/5] Fixed check style issues, corrected import order --- .../rsgroup/TestTableDescriptorWithRSGroup.java | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/hbase-rsgroup/src/test/java/org/apache/hadoop/hbase/rsgroup/TestTableDescriptorWithRSGroup.java b/hbase-rsgroup/src/test/java/org/apache/hadoop/hbase/rsgroup/TestTableDescriptorWithRSGroup.java index aeb2ab0dceb0..80d6098a45b7 100644 --- a/hbase-rsgroup/src/test/java/org/apache/hadoop/hbase/rsgroup/TestTableDescriptorWithRSGroup.java +++ b/hbase-rsgroup/src/test/java/org/apache/hadoop/hbase/rsgroup/TestTableDescriptorWithRSGroup.java @@ -18,6 +18,13 @@ 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 java.io.IOException; +import java.util.List; +import java.util.Optional; +import java.util.regex.Pattern; import org.apache.hadoop.hbase.DoNotRetryIOException; import org.apache.hadoop.hbase.HBaseClassTestRule; import org.apache.hadoop.hbase.TableName; @@ -37,15 +44,6 @@ import org.junit.ClassRule; import org.junit.Test; import org.junit.experimental.categories.Category; - -import java.io.IOException; -import java.util.List; -import java.util.Optional; -import java.util.regex.Pattern; - -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertTrue; import org.apache.hbase.thirdparty.com.google.common.collect.Sets; @Category({ LargeTests.class }) From 11e7c6669576c7c71fd0a0dabb3d79e2dd97c5a3 Mon Sep 17 00:00:00 2001 From: Mohammad Arshad Date: Fri, 5 Mar 2021 13:03:35 +0530 Subject: [PATCH 4/5] Fixed javac warnings in test class --- .../hbase/rsgroup/TestTableDescriptorWithRSGroup.java | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/hbase-rsgroup/src/test/java/org/apache/hadoop/hbase/rsgroup/TestTableDescriptorWithRSGroup.java b/hbase-rsgroup/src/test/java/org/apache/hadoop/hbase/rsgroup/TestTableDescriptorWithRSGroup.java index 80d6098a45b7..b25daa36ef4d 100644 --- a/hbase-rsgroup/src/test/java/org/apache/hadoop/hbase/rsgroup/TestTableDescriptorWithRSGroup.java +++ b/hbase-rsgroup/src/test/java/org/apache/hadoop/hbase/rsgroup/TestTableDescriptorWithRSGroup.java @@ -21,6 +21,7 @@ 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.List; import java.util.Optional; @@ -90,6 +91,7 @@ public void testCreateTableInTableDescriptorSpecificRSGroup() throws Exception { .setColumnFamily(ColumnFamilyDescriptorBuilder.newBuilder(Bytes.toBytes("f1")).build()) .build(); admin.createTable(desc, getSpitKeys(6)); + fail("Should have thrown ConstraintException but no exception thrown."); } catch (ConstraintException e) { assertEquals(e.getMessage(), "Region server group nonExistingRSGroup does not exist."); } @@ -227,6 +229,7 @@ public void testCloneSnapshotWithRSGroup() throws Exception { final TableName clonedTable2 = TableName.valueOf(tableName.getNameAsString() + "_2"); try { admin.cloneSnapshot(Bytes.toBytes(snapshotName), clonedTable2); + fail("Should have thrown ConstraintException but no exception thrown."); } catch (ConstraintException e) { assertTrue( e.getCause().getMessage().contains("Region server group rsGroup1 does not exist.")); @@ -252,6 +255,7 @@ public void testMoveTablesWhenModifyTableWithNewRSGroup() throws Exception { // ConstraintException as rsGroup2 does not exits try { admin.modifyTable(newTableDescriptor); + fail("Should have thrown ConstraintException but no exception thrown."); } catch (ConstraintException e) { assertTrue( e.getCause().getMessage().contains("Region server group rsGroup2 does not exist.")); @@ -290,6 +294,7 @@ public void testTableShouldNotAddedIntoRSGroup_WhenModifyTableFails() throws Exc // Removed family to fail pre-check validation try { admin.modifyTable(newTableDescriptor); + fail("Should have thrown DoNotRetryIOException but no exception thrown."); } catch (DoNotRetryIOException e) { assertTrue( e.getCause().getMessage().contains("Table should have at least one column family")); @@ -310,6 +315,7 @@ public void testTableShouldNotAddedIntoRSGroup_WhenTableCreationFails() throws E .build(); try { admin.createTable(desc, getSpitKeys(5)); + fail("Should have thrown DoNotRetryIOException but no exception thrown."); } catch (DoNotRetryIOException e) { assertTrue( e.getCause().getMessage().contains("Table should have at least one column family")); From b306f95a53356d357133b2cf9ba2ff5c53d0ef07 Mon Sep 17 00:00:00 2001 From: Mohammad Arshad Date: Fri, 5 Mar 2021 22:53:48 +0530 Subject: [PATCH 5/5] Added javadoc for setRegionServerGroup method --- .../apache/hadoop/hbase/client/TableDescriptorBuilder.java | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/TableDescriptorBuilder.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/TableDescriptorBuilder.java index 96ccfcad9a13..41444ecfde75 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/TableDescriptorBuilder.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/TableDescriptorBuilder.java @@ -598,6 +598,12 @@ public TableDescriptorBuilder setReplicationScope(int scope) { return this; } + /** + * Set the RSGroup for this table, specified RSGroup must exist before create or modify table. + * + * @param group rsgroup name + * @return a TableDescriptorBuilder + */ public TableDescriptorBuilder setRegionServerGroup(String group) { desc.setValue(RSGROUP_KEY, group); return this;