HBASE-25549 Provide a switch that allows avoiding reopening all regio…

…ns when modifying a table to prevent RIT storms. (#2924) Co-authored-by: Huangzhuoyue <huangzhuoyue@apache.org> Signed-off-by: Bryan Beaudreault <bbeaudreault@apache.org> Signed-off-by: Wellington Chevreuil <wchevreuil@apache.org> Signed-off-by: Viraj Jasani <vjasani@apache.org> Signed-off-by: Bharath Vissapragada <bharathv@apache.org> Signed-off-by: Esteban Gutierrez <esteban@apache.org> Reviewed-by: Sean Busbey <busbey@apache.org> Reviewed-by: Gourab Taparia
apache · Dec 1, 2023 · 9c80716 · 9c80716
1 parent 3b72456
commit 9c80716
Show file tree

Hide file tree

Showing 20 changed files with 332 additions and 23 deletions.
diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/Admin.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/Admin.java
@@ -1058,7 +1058,24 @@ default void modifyTable(TableDescriptor td) throws IOException {
    * @return the result of the async modify. You can use Future.get(long, TimeUnit) to wait on the
    *         operation to complete
    */
-  Future<Void> modifyTableAsync(TableDescriptor td) throws IOException;
+  default Future<Void> modifyTableAsync(TableDescriptor td) throws IOException {
+    return modifyTableAsync(td, true);
+  }
+
+  /**
+   * The same as {@link #modifyTableAsync(TableDescriptor td)}, except for the reopenRegions
+   * parameter, which controls whether the process of modifying the table should reopen all regions.
+   * @param td            description of the table
+   * @param reopenRegions By default, 'modifyTable' reopens all regions, potentially causing a RIT
+   *                      (Region In Transition) storm in large tables. If set to 'false', regions
+   *                      will remain unaware of the modification until they are individually
+   *                      reopened. Please note that this may temporarily result in configuration
+   *                      inconsistencies among regions.
+   * @return the result of the async modify. You can use Future.get(long, TimeUnit) to wait on the
+   *         operation to complete
+   * @throws IOException if a remote or network exception occurs
+   */
+  Future<Void> modifyTableAsync(TableDescriptor td, boolean reopenRegions) throws IOException;
 
   /**
    * Change the store file tracker of the given table.

diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/AdminOverAsyncAdmin.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/AdminOverAsyncAdmin.java
@@ -502,7 +502,13 @@ public Future<Void> truncateRegionAsync(byte[] regionName) {
 
   @Override
   public Future<Void> modifyTableAsync(TableDescriptor td) throws IOException {
-    return admin.modifyTable(td);
+    return modifyTableAsync(td, true);
+  }
+
+  @Override
+  public Future<Void> modifyTableAsync(TableDescriptor td, boolean reopenRegions)
+    throws IOException {
+    return admin.modifyTable(td, reopenRegions);
   }
 
   @Override

diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncAdmin.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncAdmin.java
@@ -198,7 +198,20 @@ CompletableFuture<Void> createTable(TableDescriptor desc, byte[] startKey, byte[
    * Modify an existing table, more IRB friendly version.
    * @param desc modified description of the table
    */
-  CompletableFuture<Void> modifyTable(TableDescriptor desc);
+  default CompletableFuture<Void> modifyTable(TableDescriptor desc) {
+    return modifyTable(desc, true);
+  }
+
+  /**
+   * Modify an existing table, more IRB friendly version.
+   * @param desc          description of the table
+   * @param reopenRegions By default, 'modifyTable' reopens all regions, potentially causing a RIT
+   *                      (Region In Transition) storm in large tables. If set to 'false', regions
+   *                      will remain unaware of the modification until they are individually
+   *                      reopened. Please note that this may temporarily result in configuration
+   *                      inconsistencies among regions.
+   */
+  CompletableFuture<Void> modifyTable(TableDescriptor desc, boolean reopenRegions);
 
   /**
    * Change the store file tracker of the given table.

diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncHBaseAdmin.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncHBaseAdmin.java
@@ -153,7 +153,12 @@ public CompletableFuture<Void> createTable(TableDescriptor desc, byte[][] splitK
 
   @Override
   public CompletableFuture<Void> modifyTable(TableDescriptor desc) {
-    return wrap(rawAdmin.modifyTable(desc));
+    return modifyTable(desc, true);
+  }
+
+  @Override
+  public CompletableFuture<Void> modifyTable(TableDescriptor desc, boolean reopenRegions) {
+    return wrap(rawAdmin.modifyTable(desc, reopenRegions));
   }
 
   @Override

diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/RawAsyncHBaseAdmin.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/RawAsyncHBaseAdmin.java
@@ -694,9 +694,14 @@ private CompletableFuture<Void> createTable(TableName tableName, CreateTableRequ
 
   @Override
   public CompletableFuture<Void> modifyTable(TableDescriptor desc) {
+    return modifyTable(desc, true);
+  }
+
+  @Override
+  public CompletableFuture<Void> modifyTable(TableDescriptor desc, boolean reopenRegions) {
     return this.<ModifyTableRequest, ModifyTableResponse> procedureCall(desc.getTableName(),
       RequestConverter.buildModifyTableRequest(desc.getTableName(), desc, ng.getNonceGroup(),
-        ng.newNonce()),
+        ng.newNonce(), reopenRegions),
       (s, c, req, done) -> s.modifyTable(c, req, done), (resp) -> resp.getProcId(),
       new ModifyTableProcedureBiConsumer(this, desc.getTableName()));
   }

diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/shaded/protobuf/RequestConverter.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/shaded/protobuf/RequestConverter.java
@@ -1108,12 +1108,14 @@ public static CreateTableRequest buildCreateTableRequest(final TableDescriptor t
    * @return a ModifyTableRequest
    */
   public static ModifyTableRequest buildModifyTableRequest(final TableName tableName,
-    final TableDescriptor tableDesc, final long nonceGroup, final long nonce) {
+    final TableDescriptor tableDesc, final long nonceGroup, final long nonce,
+    final boolean reopenRegions) {
     ModifyTableRequest.Builder builder = ModifyTableRequest.newBuilder();
     builder.setTableName(ProtobufUtil.toProtoTableName(tableName));
     builder.setTableSchema(ProtobufUtil.toTableSchema(tableDesc));
     builder.setNonceGroup(nonceGroup);
     builder.setNonce(nonce);
+    builder.setReopenRegions(reopenRegions);
     return builder.build();
   }
 

diff --git a/hbase-protocol-shaded/src/main/protobuf/server/master/Master.proto b/hbase-protocol-shaded/src/main/protobuf/server/master/Master.proto
@@ -204,6 +204,7 @@ message ModifyTableRequest {
   required TableSchema table_schema = 2;
   optional uint64 nonce_group = 3 [default = 0];
   optional uint64 nonce = 4 [default = 0];
+  optional bool reopen_regions = 5 [default = true];
 }
 
 message ModifyTableResponse {

diff --git a/hbase-protocol-shaded/src/main/protobuf/server/master/MasterProcedure.proto b/hbase-protocol-shaded/src/main/protobuf/server/master/MasterProcedure.proto
@@ -82,6 +82,7 @@ message ModifyTableStateData {
   required TableSchema modified_table_schema = 3;
   required bool delete_column_family_in_modify = 4;
   optional bool should_check_descriptor = 5;
+  optional bool reopen_regions = 6;
 }
 
 enum TruncateTableState {

diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java
@@ -1164,7 +1164,7 @@ private void finishActiveMasterInitialization() throws IOException, InterruptedE
           procedureExecutor.submitProcedure(new ModifyTableProcedure(
             procedureExecutor.getEnvironment(), TableDescriptorBuilder.newBuilder(metaDesc)
               .setRegionReplication(replicasNumInConf).build(),
-            null, metaDesc, false));
+            null, metaDesc, false, true));
         }
       }
     }
@@ -2794,6 +2794,13 @@ protected String getDescription() {
   private long modifyTable(final TableName tableName,
     final TableDescriptorGetter newDescriptorGetter, final long nonceGroup, final long nonce,
     final boolean shouldCheckDescriptor) throws IOException {
+    return modifyTable(tableName, newDescriptorGetter, nonceGroup, nonce, shouldCheckDescriptor,
+      true);
+  }
+
+  private long modifyTable(final TableName tableName,
+    final TableDescriptorGetter newDescriptorGetter, final long nonceGroup, final long nonce,
+    final boolean shouldCheckDescriptor, final boolean reopenRegions) throws IOException {
     return MasterProcedureUtil
       .submitProcedure(new MasterProcedureUtil.NonceProcedureRunnable(this, nonceGroup, nonce) {
         @Override
@@ -2812,7 +2819,7 @@ protected void run() throws IOException {
           // checks. This will block only the beginning of the procedure. See HBASE-19953.
           ProcedurePrepareLatch latch = ProcedurePrepareLatch.createBlockingLatch();
           submitProcedure(new ModifyTableProcedure(procedureExecutor.getEnvironment(),
-            newDescriptor, latch, oldDescriptor, shouldCheckDescriptor));
+            newDescriptor, latch, oldDescriptor, shouldCheckDescriptor, reopenRegions));
           latch.await();
 
           getMaster().getMasterCoprocessorHost().postModifyTable(tableName, oldDescriptor,
@@ -2829,14 +2836,14 @@ protected String getDescription() {
 
   @Override
   public long modifyTable(final TableName tableName, final TableDescriptor newDescriptor,
-    final long nonceGroup, final long nonce) throws IOException {
+    final long nonceGroup, final long nonce, final boolean reopenRegions) throws IOException {
     checkInitialized();
     return modifyTable(tableName, new TableDescriptorGetter() {
       @Override
       public TableDescriptor get() throws IOException {
         return newDescriptor;
       }
-    }, nonceGroup, nonce, false);
+    }, nonceGroup, nonce, false, reopenRegions);
 
   }
 

diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterRpcServices.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterRpcServices.java
@@ -1544,7 +1544,8 @@ public ModifyTableResponse modifyTable(RpcController controller, ModifyTableRequ
     throws ServiceException {
     try {
       long procId = server.modifyTable(ProtobufUtil.toTableName(req.getTableName()),
-        ProtobufUtil.toTableDescriptor(req.getTableSchema()), req.getNonceGroup(), req.getNonce());
+        ProtobufUtil.toTableDescriptor(req.getTableSchema()), req.getNonceGroup(), req.getNonce(),
+        req.getReopenRegions());
       return ModifyTableResponse.newBuilder().setProcId(procId).build();
     } catch (IOException ioe) {
       throw new ServiceException(ioe);

diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterServices.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterServices.java
@@ -158,8 +158,19 @@ public long truncateTable(final TableName tableName, final boolean preserveSplit
    * @param tableName  The table name
    * @param descriptor The updated table descriptor
    */
+  default long modifyTable(final TableName tableName, final TableDescriptor descriptor,
+    final long nonceGroup, final long nonce) throws IOException {
+    return modifyTable(tableName, descriptor, nonceGroup, nonce, true);
+  }
+
+  /**
+   * Modify the descriptor of an existing table
+   * @param tableName     The table name
+   * @param descriptor    The updated table descriptor
+   * @param reopenRegions Whether to reopen regions after modifying the table descriptor
+   */
   long modifyTable(final TableName tableName, final TableDescriptor descriptor,
-    final long nonceGroup, final long nonce) throws IOException;
+    final long nonceGroup, final long nonce, final boolean reopenRegions) throws IOException;
 
   /**
    * Modify the store file tracker of an existing table

diff --git a/...e-server/src/main/java/org/apache/hadoop/hbase/master/procedure/ModifyTableProcedure.java b/...e-server/src/main/java/org/apache/hadoop/hbase/master/procedure/ModifyTableProcedure.java
@@ -20,6 +20,7 @@
 import java.io.IOException;
 import java.util.Arrays;
 import java.util.Collections;
+import java.util.HashSet;
 import java.util.List;
 import java.util.Set;
 import java.util.function.Supplier;
@@ -34,6 +35,7 @@
 import org.apache.hadoop.hbase.client.RegionInfo;
 import org.apache.hadoop.hbase.client.RegionReplicaUtil;
 import org.apache.hadoop.hbase.client.TableDescriptor;
+import org.apache.hadoop.hbase.client.TableDescriptorBuilder;
 import org.apache.hadoop.hbase.master.MasterCoprocessorHost;
 import org.apache.hadoop.hbase.master.zksyncer.MetaLocationSyncer;
 import org.apache.hadoop.hbase.procedure2.ProcedureStateSerializer;
@@ -56,6 +58,7 @@ public class ModifyTableProcedure extends AbstractStateMachineTableProcedure<Mod
   private TableDescriptor modifiedTableDescriptor;
   private boolean deleteColumnFamilyInModify;
   private boolean shouldCheckDescriptor;
+  private boolean reopenRegions;
   /**
    * List of column families that cannot be deleted from the hbase:meta table. They are critical to
    * cluster operation. This is a bit of an odd place to keep this list but then this is the tooling
@@ -77,14 +80,15 @@ public ModifyTableProcedure(final MasterProcedureEnv env, final TableDescriptor
 
   public ModifyTableProcedure(final MasterProcedureEnv env, final TableDescriptor htd,
     final ProcedurePrepareLatch latch) throws HBaseIOException {
-    this(env, htd, latch, null, false);
+    this(env, htd, latch, null, false, true);
   }
 
   public ModifyTableProcedure(final MasterProcedureEnv env,
     final TableDescriptor newTableDescriptor, final ProcedurePrepareLatch latch,
-    final TableDescriptor oldTableDescriptor, final boolean shouldCheckDescriptor)
-    throws HBaseIOException {
+    final TableDescriptor oldTableDescriptor, final boolean shouldCheckDescriptor,
+    final boolean reopenRegions) throws HBaseIOException {
     super(env, latch);
+    this.reopenRegions = reopenRegions;
     initialize(oldTableDescriptor, shouldCheckDescriptor);
     this.modifiedTableDescriptor = newTableDescriptor;
     preflightChecks(env, null/* No table checks; if changing peers, table can be online */);
@@ -104,6 +108,60 @@ protected void preflightChecks(MasterProcedureEnv env, Boolean enabled) throws H
         }
       }
     }
+    if (!reopenRegions) {
+      if (this.unmodifiedTableDescriptor == null) {
+        throw new HBaseIOException(
+          "unmodifiedTableDescriptor cannot be null when this table modification won't reopen regions");
+      }
+      if (
+        !this.unmodifiedTableDescriptor.getTableName()
+          .equals(this.modifiedTableDescriptor.getTableName())
+      ) {
+        throw new HBaseIOException(
+          "Cannot change the table name when this modification won't " + "reopen regions.");
+      }
+      if (
+        this.unmodifiedTableDescriptor.getColumnFamilyCount()
+            != this.modifiedTableDescriptor.getColumnFamilyCount()
+      ) {
+        throw new HBaseIOException(
+          "Cannot add or remove column families when this modification " + "won't reopen regions.");
+      }
+      if (
+        this.unmodifiedTableDescriptor.getCoprocessorDescriptors().hashCode()
+            != this.modifiedTableDescriptor.getCoprocessorDescriptors().hashCode()
+      ) {
+        throw new HBaseIOException(
+          "Can not modify Coprocessor when table modification won't reopen regions");
+      }
+      final Set<String> s = new HashSet<>(Arrays.asList(TableDescriptorBuilder.REGION_REPLICATION,
+        TableDescriptorBuilder.REGION_MEMSTORE_REPLICATION, RSGroupInfo.TABLE_DESC_PROP_GROUP));
+      for (String k : s) {
+        if (
+          isTablePropertyModified(this.unmodifiedTableDescriptor, this.modifiedTableDescriptor, k)
+        ) {
+          throw new HBaseIOException(
+            "Can not modify " + k + " of a table when modification won't reopen regions");
+        }
+      }
+    }
+  }
+
+  /**
+   * Comparing the value associated with a given key across two TableDescriptor instances'
+   * properties.
+   * @return True if the table property <code>key</code> is the same in both.
+   */
+  private boolean isTablePropertyModified(TableDescriptor oldDescriptor,
+    TableDescriptor newDescriptor, String key) {
+    String oldV = oldDescriptor.getValue(key);
+    String newV = newDescriptor.getValue(key);
+    if (oldV == null && newV == null) {
+      return false;
+    } else if (oldV != null && newV != null && oldV.equals(newV)) {
+      return false;
+    }
+    return true;
   }
 
   private void initialize(final TableDescriptor unmodifiedTableDescriptor,
@@ -125,7 +183,13 @@ protected Flow executeFromState(final MasterProcedureEnv env, final ModifyTableS
           break;
         case MODIFY_TABLE_PRE_OPERATION:
           preModify(env, state);
-          setNextState(ModifyTableState.MODIFY_TABLE_CLOSE_EXCESS_REPLICAS);
+          // We cannot allow changes to region replicas when 'reopenRegions==false',
+          // as this mode bypasses the state management required for modifying region replicas.
+          if (reopenRegions) {
+            setNextState(ModifyTableState.MODIFY_TABLE_CLOSE_EXCESS_REPLICAS);
+          } else {
+            setNextState(ModifyTableState.MODIFY_TABLE_UPDATE_TABLE_DESCRIPTOR);
+          }
           break;
         case MODIFY_TABLE_CLOSE_EXCESS_REPLICAS:
           if (isTableEnabled(env)) {
@@ -135,15 +199,23 @@ protected Flow executeFromState(final MasterProcedureEnv env, final ModifyTableS
           break;
         case MODIFY_TABLE_UPDATE_TABLE_DESCRIPTOR:
           updateTableDescriptor(env);
-          setNextState(ModifyTableState.MODIFY_TABLE_REMOVE_REPLICA_COLUMN);
+          if (reopenRegions) {
+            setNextState(ModifyTableState.MODIFY_TABLE_REMOVE_REPLICA_COLUMN);
+          } else {
+            setNextState(ModifyTableState.MODIFY_TABLE_POST_OPERATION);
+          }
           break;
         case MODIFY_TABLE_REMOVE_REPLICA_COLUMN:
           removeReplicaColumnsIfNeeded(env);
           setNextState(ModifyTableState.MODIFY_TABLE_POST_OPERATION);
           break;
         case MODIFY_TABLE_POST_OPERATION:
           postModify(env, state);
-          setNextState(ModifyTableState.MODIFY_TABLE_REOPEN_ALL_REGIONS);
+          if (reopenRegions) {
+            setNextState(ModifyTableState.MODIFY_TABLE_REOPEN_ALL_REGIONS);
+          } else {
+            return Flow.NO_MORE_STATE;
+          }
           break;
         case MODIFY_TABLE_REOPEN_ALL_REGIONS:
           if (isTableEnabled(env)) {
@@ -238,7 +310,7 @@ protected void serializeStateData(ProcedureStateSerializer serializer) throws IO
         .setUserInfo(MasterProcedureUtil.toProtoUserInfo(getUser()))
         .setModifiedTableSchema(ProtobufUtil.toTableSchema(modifiedTableDescriptor))
         .setDeleteColumnFamilyInModify(deleteColumnFamilyInModify)
-        .setShouldCheckDescriptor(shouldCheckDescriptor);
+        .setShouldCheckDescriptor(shouldCheckDescriptor).setReopenRegions(reopenRegions);
 
     if (unmodifiedTableDescriptor != null) {
       modifyTableMsg
@@ -260,6 +332,7 @@ protected void deserializeStateData(ProcedureStateSerializer serializer) throws
     deleteColumnFamilyInModify = modifyTableMsg.getDeleteColumnFamilyInModify();
     shouldCheckDescriptor =
       modifyTableMsg.hasShouldCheckDescriptor() ? modifyTableMsg.getShouldCheckDescriptor() : false;
+    reopenRegions = modifyTableMsg.hasReopenRegions() ? modifyTableMsg.getReopenRegions() : true;
 
     if (modifyTableMsg.hasUnmodifiedTableSchema()) {
       unmodifiedTableDescriptor =

diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/MockNoopMasterServices.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/MockNoopMasterServices.java
@@ -263,6 +263,12 @@ public long modifyTable(final TableName tableName, final TableDescriptor descrip
     return -1;
   }
 
+  @Override
+  public long modifyTable(TableName tableName, TableDescriptor descriptor, long nonceGroup,
+    long nonce, boolean reopenRegions) throws IOException {
+    return -1;
+  }
+
   @Override
   public long enableTable(final TableName tableName, final long nonceGroup, final long nonce)
     throws IOException {