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
The following commit(s) were added to refs/heads/master by this push: new e5d59cadc5d HBASE-28481 Prompting table already exists after failing to create table with many region replications (#5789) e5d59cadc5d is described below commit e5d59cadc5dc9ac7d6f15555be1a8defc05862ad Author: guluo <lupeng_n...@qq.com> AuthorDate: Sun Apr 7 17:27:28 2024 +0800 HBASE-28481 Prompting table already exists after failing to create table with many region replications (#5789) Signed-off-by: Duo Zhang <zhang...@apache.org> Reviewed-by: Vineet Kumar Maheshwari <vineet.4...@gmail.com> --- .../hadoop/hbase/client/MutableRegionInfo.java | 2 +- .../master/procedure/CreateTableProcedure.java | 14 +++++++++-- .../master/procedure/TestCreateTableProcedure.java | 29 ++++++++++++++++++++++ 3 files changed, 42 insertions(+), 3 deletions(-) diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/MutableRegionInfo.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/MutableRegionInfo.java index a9382f3a9be..4217201b85e 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/MutableRegionInfo.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/MutableRegionInfo.java @@ -96,7 +96,7 @@ class MutableRegionInfo implements RegionInfo { private static int checkReplicaId(int regionId) { if (regionId > MAX_REPLICA_ID) { - throw new IllegalArgumentException("ReplicaId cannot be greater than" + MAX_REPLICA_ID); + throw new IllegalArgumentException("ReplicaId cannot be greater than " + MAX_REPLICA_ID); } return regionId; } diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/CreateTableProcedure.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/CreateTableProcedure.java index 533b6fffcc4..17998fec7bd 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/CreateTableProcedure.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/CreateTableProcedure.java @@ -56,6 +56,8 @@ import org.apache.hadoop.hbase.shaded.protobuf.generated.MasterProcedureProtos.C public class CreateTableProcedure extends AbstractStateMachineTableProcedure<CreateTableState> { private static final Logger LOG = LoggerFactory.getLogger(CreateTableProcedure.class); + private static final int MAX_REGION_REPLICATION = 0x10000; + private TableDescriptor tableDescriptor; private List<RegionInfo> newRegions; @@ -84,10 +86,10 @@ public class CreateTableProcedure extends AbstractStateMachineTableProcedure<Cre switch (state) { case CREATE_TABLE_PRE_OPERATION: // Verify if we can create the table - boolean exists = !prepareCreate(env); + boolean success = prepareCreate(env); releaseSyncLatch(); - if (exists) { + if (!success) { assert isFailed() : "the delete should have an exception here"; return Flow.NO_MORE_STATE; } @@ -262,6 +264,14 @@ public class CreateTableProcedure extends AbstractStateMachineTableProcedure<Cre "Table " + getTableName().toString() + " should have at least one column family.")); return false; } + + int regionReplicationCount = tableDescriptor.getRegionReplication(); + if (regionReplicationCount > MAX_REGION_REPLICATION) { + setFailure("master-create-table", new IllegalArgumentException( + "Region Replication cannot exceed " + MAX_REGION_REPLICATION + ".")); + return false; + } + if (!tableName.isSystemTable()) { // do not check rs group for system tables as we may block the bootstrap. Supplier<String> forWhom = () -> "table " + tableName; diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestCreateTableProcedure.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestCreateTableProcedure.java index 618a4a45a04..bed41f4da86 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestCreateTableProcedure.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestCreateTableProcedure.java @@ -19,6 +19,7 @@ package org.apache.hadoop.hbase.master.procedure; import static org.apache.hadoop.hbase.regionserver.storefiletracker.StoreFileTrackerFactory.TRACKER_IMPL; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; @@ -284,4 +285,32 @@ public class TestCreateTableProcedure extends TestTableDDLProcedureBase { new CreateTableProcedureOnHDFSFailure(procExec.getEnvironment(), htd, regions)); ProcedureTestingUtility.assertProcNotFailed(procExec, procId); } + + @Test + public void testCreateTableWithManyRegionReplication() throws IOException { + final int EXCEED_MAX_REGION_REPLICATION = 0x10001; + TableName tableName = TableName.valueOf(name.getMethodName()); + ProcedureExecutor<MasterProcedureEnv> procExec = getMasterProcedureExecutor(); + + TableDescriptor tableWithManyRegionReplication = TableDescriptorBuilder.newBuilder(tableName) + .setColumnFamily(ColumnFamilyDescriptorBuilder.newBuilder(Bytes.toBytes("f1")).build()) + .setRegionReplication(EXCEED_MAX_REGION_REPLICATION).build(); + RegionInfo[] regions01 = + ModifyRegionUtils.createRegionInfos(tableWithManyRegionReplication, null); + long procId01 = ProcedureTestingUtility.submitAndWait(procExec, new CreateTableProcedure( + procExec.getEnvironment(), tableWithManyRegionReplication, regions01)); + Procedure<?> result01 = procExec.getResult(procId01); + assertTrue(result01.getException().getCause() instanceof IllegalArgumentException); + assertFalse(UTIL.getAdmin().tableExists(tableName)); + + TableDescriptor tdesc = TableDescriptorBuilder.newBuilder(tableName) + .setColumnFamily(ColumnFamilyDescriptorBuilder.newBuilder(Bytes.toBytes("f1")).build()) + .build(); + RegionInfo[] regions02 = ModifyRegionUtils.createRegionInfos(tdesc, null); + long procId02 = ProcedureTestingUtility.submitAndWait(procExec, + new CreateTableProcedure(procExec.getEnvironment(), tdesc, regions02)); + Procedure<?> result02 = procExec.getResult(procId02); + assertTrue(result02.isSuccess()); + assertTrue(UTIL.getAdmin().tableExists(tableName)); + } }