HBASE-25549 A new hbase shell command: 'alter_lazy'

GeorryHuang · GeorryHuang · commit 9ca71f6b1208 · 2021-02-04T21:14:24.000+08:00
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
@@ -1033,7 +1033,21 @@ 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);
+  }
+
+  /**
+   * Same as {@link #modifyTableAsync(TableDescriptor td)}. except the boolean
+   * {@code shouldReopenRegions} will control whether reopen regions after modifying done
+   * @param td description of the table
+   * @param shouldReopenRegions If false, only the TableDescriptor will be updated and
+   *                            regions belonging to the table will not be reopened
+   * @throws IOException if a remote or network exception occurs
+   * @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, boolean shouldReopenRegions) throws IOException;
 
   /**
    * Shuts down the HBase cluster.
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
@@ -470,6 +470,11 @@ public Future<Void> modifyTableAsync(TableDescriptor td) throws IOException {
     return admin.modifyTable(td);
   }
 
+  @Override
+  public Future<Void> modifyTableAsync(TableDescriptor td, boolean shouldReopenRegions) throws IOException {
+    return admin.modifyTable(td, shouldReopenRegions);
+  }
+
   @Override
   public void shutdown() throws IOException {
     get(admin.shutdown());
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
@@ -180,7 +180,19 @@ 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);
+  }
+
+  /**
+   * Same as {@link #modifyTable(TableDescriptor td)}. except the boolean {@code shouldReopenRegions}
+   * will control whether reopen regions after modifying done
+   * @param desc description of the table
+   * @param shouldReopenRegions If false, only the TableDescriptor will be updated and
+   *                            the regions belonging to the table will not be reopened
+   */
+  CompletableFuture<Void> modifyTable(TableDescriptor desc, boolean shouldReopenRegions);
+
 
   /**
    * Deletes a 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
@@ -143,7 +143,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 shouldReopenRegions) {
+    return wrap(rawAdmin.modifyTable(desc, shouldReopenRegions));
   }
 
   @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
@@ -656,9 +656,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 shouldReopenRegions) {
     return this.<ModifyTableRequest, ModifyTableResponse> procedureCall(desc.getTableName(),
       RequestConverter.buildModifyTableRequest(desc.getTableName(), desc, ng.getNonceGroup(),
-        ng.newNonce()), (s, c, req, done) -> s.modifyTable(c, req, done),
+        ng.newNonce(), shouldReopenRegions), (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
@@ -1203,15 +1203,17 @@ public static CreateTableRequest buildCreateTableRequest(
    * @return a ModifyTableRequest
    */
   public static ModifyTableRequest buildModifyTableRequest(
-      final TableName tableName,
-      final TableDescriptor tableDesc,
-      final long nonceGroup,
-      final long nonce) {
+    final TableName tableName,
+    final TableDescriptor tableDesc,
+    final long nonceGroup,
+    final long nonce,
+    final boolean shouldReopenRegions) {
     ModifyTableRequest.Builder builder = ModifyTableRequest.newBuilder();
     builder.setTableName(ProtobufUtil.toProtoTableName((tableName)));
     builder.setTableSchema(ProtobufUtil.toTableSchema(tableDesc));
     builder.setNonceGroup(nonceGroup);
     builder.setNonce(nonce);
+    builder.setShouldReopenRegions(shouldReopenRegions);
     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
@@ -194,6 +194,7 @@ message ModifyTableRequest {
   required TableSchema table_schema = 2;
   optional uint64 nonce_group = 3 [default = 0];
   optional uint64 nonce = 4 [default = 0];
+  optional bool should_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 should_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
@@ -2353,9 +2353,15 @@ 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) throws IOException {
+      final boolean shouldCheckDescriptor, final boolean shouldReopenRegions) throws IOException {
     return MasterProcedureUtil
         .submitProcedure(new MasterProcedureUtil.NonceProcedureRunnable(this, nonceGroup, nonce) {
           @Override
@@ -2374,7 +2380,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, shouldReopenRegions));
             latch.await();
 
             getMaster().getMasterCoprocessorHost().postModifyTable(tableName, oldDescriptor,
@@ -2391,14 +2397,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 shouldReopenRegions) throws IOException {
     checkInitialized();
     return modifyTable(tableName, new TableDescriptorGetter() {
       @Override
       public TableDescriptor get() throws IOException {
         return newDescriptor;
       }
-    }, nonceGroup, nonce, false);
+    }, nonceGroup, nonce, false, shouldReopenRegions);
 
   }
 
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
@@ -1464,7 +1464,8 @@ public ModifyTableResponse modifyTable(RpcController controller,
         ProtobufUtil.toTableName(req.getTableName()),
         ProtobufUtil.toTableDescriptor(req.getTableSchema()),
         req.getNonceGroup(),
-        req.getNonce());
+        req.getNonce(),
+        req.getShouldReopenRegions());
       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
@@ -213,11 +213,31 @@ public long truncateTable(
    * @param nonce
    * @throws IOException
    */
+  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 nonceGroup
+   * @param nonce
+   * @param shouldReopenRegions
+   * @throws IOException
+   */
   long modifyTable(
       final TableName tableName,
       final TableDescriptor descriptor,
       final long nonceGroup,
-      final long nonce)
+      final long nonce,
+      final boolean shouldReopenRegions)
       throws IOException;
 
   /**
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/ModifyTableProcedure.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/ModifyTableProcedure.java
@@ -59,6 +59,7 @@ public class ModifyTableProcedure
   private TableDescriptor modifiedTableDescriptor;
   private boolean deleteColumnFamilyInModify;
   private boolean shouldCheckDescriptor;
+  private boolean shouldReopenRegions = true;
   /**
    * 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
@@ -95,6 +96,16 @@ public ModifyTableProcedure(final MasterProcedureEnv env,
     preflightChecks(env, null/*No table checks; if changing peers, table can be online*/);
   }
 
+  public ModifyTableProcedure(final MasterProcedureEnv env,
+    final TableDescriptor newTableDescriptor, final ProcedurePrepareLatch latch,
+    final TableDescriptor oldTableDescriptor, final boolean shouldCheckDescriptor,
+    final boolean shouldReopenRegions) throws HBaseIOException {
+    this(env, newTableDescriptor, latch, oldTableDescriptor, shouldCheckDescriptor);
+    this.shouldReopenRegions = shouldReopenRegions;
+  }
+
+
+
   @Override
   protected void preflightChecks(MasterProcedureEnv env, Boolean enabled) throws HBaseIOException {
     super.preflightChecks(env, enabled);
@@ -151,7 +162,7 @@ protected Flow executeFromState(final MasterProcedureEnv env, final ModifyTableS
           setNextState(ModifyTableState.MODIFY_TABLE_REOPEN_ALL_REGIONS);
           break;
         case MODIFY_TABLE_REOPEN_ALL_REGIONS:
-          if (isTableEnabled(env)) {
+          if (isTableEnabled(env) && shouldReopenRegions) {
             addChildProcedure(new ReopenTableRegionsProcedure(getTableName()));
           }
           setNextState(ModifyTableState.MODIFY_TABLE_ASSIGN_NEW_REPLICAS);
@@ -242,7 +253,8 @@ protected void serializeStateData(ProcedureStateSerializer serializer)
             .setUserInfo(MasterProcedureUtil.toProtoUserInfo(getUser()))
             .setModifiedTableSchema(ProtobufUtil.toTableSchema(modifiedTableDescriptor))
             .setDeleteColumnFamilyInModify(deleteColumnFamilyInModify)
-            .setShouldCheckDescriptor(shouldCheckDescriptor);
+            .setShouldCheckDescriptor(shouldCheckDescriptor)
+            .setShouldReopenRegions(shouldReopenRegions);
 
     if (unmodifiedTableDescriptor != null) {
       modifyTableMsg
@@ -264,6 +276,8 @@ protected void deserializeStateData(ProcedureStateSerializer serializer)
     deleteColumnFamilyInModify = modifyTableMsg.getDeleteColumnFamilyInModify();
     shouldCheckDescriptor = modifyTableMsg.hasShouldCheckDescriptor()
         ? modifyTableMsg.getShouldCheckDescriptor() : false;
+    shouldReopenRegions = modifyTableMsg.hasShouldReopenRegions()
+      ? modifyTableMsg.getShouldReopenRegions() : 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
@@ -268,6 +268,16 @@ public long modifyTable(
     return -1;
   }
 
+  @Override
+  public long modifyTable(
+    final TableName tableName,
+    final TableDescriptor descriptor,
+    final long nonceGroup,
+    final long nonce,
+    final boolean shouldReopenRegions) throws IOException {
+    return -1;
+  }
+
   @Override
   public long enableTable(
       final TableName tableName,
diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestModifyTableProcedure.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestModifyTableProcedure.java
@@ -19,9 +19,11 @@
 
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotEquals;
+import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertTrue;
-
 import java.io.IOException;
+import java.util.List;
 import org.apache.hadoop.hbase.ConcurrentTableModificationException;
 import org.apache.hadoop.hbase.DoNotRetryIOException;
 import org.apache.hadoop.hbase.HBaseClassTestRule;
@@ -37,6 +39,7 @@
 import org.apache.hadoop.hbase.procedure2.Procedure;
 import org.apache.hadoop.hbase.procedure2.ProcedureExecutor;
 import org.apache.hadoop.hbase.procedure2.ProcedureTestingUtility;
+import org.apache.hadoop.hbase.regionserver.HRegion;
 import org.apache.hadoop.hbase.testclassification.LargeTests;
 import org.apache.hadoop.hbase.testclassification.MasterTests;
 import org.apache.hadoop.hbase.util.Bytes;
@@ -594,4 +597,35 @@ public void run() {
     t2.join();
     assertFalse("Expected ConcurrentTableModificationException.", (t1.exception || t2.exception));
   }
+
+  @Test public void testModifyTableNotReopenRegions() throws IOException, InterruptedException {
+    final TableName tableName = TableName.valueOf(name.getMethodName());
+    final ProcedureExecutor<MasterProcedureEnv> procExec = getMasterProcedureExecutor();
+    //Create table
+    MasterProcedureTestingUtility.createTable(procExec, tableName, null, "cf");
+    List<HRegion> onlineRegions = UTIL.getHBaseCluster().getRegions(tableName);
+    assertNotEquals(0, onlineRegions.size());
+    //Modify table by setting custom values
+    TableDescriptor newHtd =
+      TableDescriptorBuilder.newBuilder(onlineRegions.get(0).getTableDescriptor())
+        .setValue("MyTestConf", "MyTestValue").build();
+    //Should not reopen regions
+    long procId = ProcedureTestingUtility.submitAndWait(procExec,
+      new ModifyTableProcedure(procExec.getEnvironment(), newHtd, null, null, false, false));
+    ProcedureTestingUtility.assertProcNotFailed(procExec.getResult(procId));
+    onlineRegions = UTIL.getHBaseCluster().getRegions(tableName);
+    //Modifying didn't reopen regions, so can't see the new configuration
+    for (HRegion hr : onlineRegions) {
+      assertNull(hr.getTableDescriptor().getValue("MyTestConf"));
+    }
+    //Move regions to force regions reopen
+    for (HRegion hr : onlineRegions) {
+      getMaster().getAssignmentManager().move(hr.getRegionInfo());
+    }
+    //After reopening, regions should got the new modification
+    onlineRegions = UTIL.getHBaseCluster().getRegions(tableName);
+    for (HRegion hr : onlineRegions) {
+      assertEquals("MyTestValue", hr.getTableDescriptor().getValue("MyTestConf"));
+    }
+  }
 }
diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/rsgroup/VerifyingRSGroupAdmin.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/rsgroup/VerifyingRSGroupAdmin.java
@@ -407,6 +407,10 @@ public Future<Void> modifyTableAsync(TableDescriptor td) throws IOException {
     return admin.modifyTableAsync(td);
   }
 
+  public Future<Void> modifyTableAsync(TableDescriptor td, boolean shouldReopenRegions) throws IOException {
+    return admin.modifyTableAsync(td, shouldReopenRegions);
+  }
+
   public void shutdown() throws IOException {
     admin.shutdown();
   }
diff --git a/hbase-shell/src/main/ruby/hbase/admin.rb b/hbase-shell/src/main/ruby/hbase/admin.rb
@@ -754,7 +754,7 @@ def coprocessor_descriptor_from_hash(spec)
 
     #----------------------------------------------------------------------------------------------
     # Change table structure or table options
-    def alter(table_name_str, wait = true, *args)
+    def alter(table_name_str, wait = true, reopen_regions = true, *args)
       # Table name should be a string
       raise(ArgumentError, 'Table name must be of type String') unless
           table_name_str.is_a?(String)
@@ -891,7 +891,7 @@ def alter(table_name_str, wait = true, *args)
 
       # Bulk apply all table modifications.
       if hasTableUpdate
-        future = @admin.modifyTableAsync(tdb.build)
+        future = @admin.modifyTableAsync(tdb.build, reopen_regions)
 
         if wait == true
           puts 'Updating all regions with the new schema...'
diff --git a/hbase-shell/src/main/ruby/shell.rb b/hbase-shell/src/main/ruby/shell.rb
@@ -394,6 +394,7 @@ def self.exception_handler(hide_traceback)
     show_filters
     alter_status
     alter_async
+    alter_lazy
     get_table
     locate_region
     list_regions
diff --git a/hbase-shell/src/main/ruby/shell/commands/alter.rb b/hbase-shell/src/main/ruby/shell/commands/alter.rb
@@ -100,7 +100,7 @@ def help
       end
 
       def command(table, *args)
-        admin.alter(table, true, *args)
+        admin.alter(table, true, true, *args)
       end
     end
   end
diff --git a/hbase-shell/src/main/ruby/shell/commands/alter_async.rb b/hbase-shell/src/main/ruby/shell/commands/alter_async.rb
@@ -56,7 +56,7 @@ def help
       end
 
       def command(table, *args)
-        admin.alter(table, false, *args)
+        admin.alter(table, false, true, *args)
       end
     end
   end
diff --git a/hbase-shell/src/main/ruby/shell/commands/alter_lazy.rb b/hbase-shell/src/main/ruby/shell/commands/alter_lazy.rb
diff --git a/hbase-thrift/src/main/java/org/apache/hadoop/hbase/thrift2/client/ThriftAdmin.java b/hbase-thrift/src/main/java/org/apache/hadoop/hbase/thrift2/client/ThriftAdmin.java

Original file line number	Diff line number	Diff line change
`@@ -194,6 +194,7 @@ message ModifyTableRequest {`
`194`	`194`	`required TableSchema table_schema = 2;`
`195`	`195`	`optional uint64 nonce_group = 3 [default = 0];`
`196`	`196`	`optional uint64 nonce = 4 [default = 0];`
	`197`	`+ optional bool should_reopen_regions = 5 [default = true];`
`197`	`198`	`}`
`198`	`199`
`199`	`200`	`message ModifyTableResponse {`
Original file line number	Diff line number	Diff line change
`@@ -82,6 +82,7 @@ message ModifyTableStateData {`
`82`	`82`	`required TableSchema modified_table_schema = 3;`
`83`	`83`	`required bool delete_column_family_in_modify = 4;`
`84`	`84`	`optional bool should_check_descriptor = 5;`
	`85`	`+ optional bool should_reopen_regions = 6;`
`85`	`86`	`}`
`86`	`87`
`87`	`88`	`enum TruncateTableState {`