HBASE-28481 Prompting table already exists after failing to create table with many region replications (apache#5789)

Signed-off-by: Duo Zhang <zhangduo@apache.org>
Reviewed-by: Vineet Kumar Maheshwari <vineet.4008@gmail.com>

Author: guluo2016

hbase-client/src/main/java/org/apache/hadoop/hbase/client/MutableRegionInfo.java

@@ -96,7 +96,7 @@ private static TableName checkTableName(TableName tableName) {
 
   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;
   }

hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/CreateTableProcedure.java

@@ -56,6 +56,8 @@
 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 @@ protected Flow executeFromState(final MasterProcedureEnv env, final CreateTableS
       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 @@ private boolean prepareCreate(final MasterProcedureEnv env) throws IOException {
         "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;

hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestCreateTableProcedure.java

@@ -19,6 +19,7 @@
 
 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 void testOnHDFSFailure() throws Exception {
       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));
+  }
 }