HBASE-26654 ModifyTableDescriptorProcedure shoud load TableDescriptor…

… while executing (#4034)

Signed-off-by: GeorryHuang <huangzhuoyue@apache.org>

hbase-protocol-shaded/src/main/protobuf/MasterProcedure.proto

@@ -593,6 +593,6 @@ enum ModifyTableDescriptorState {
 }
 
 message ModifyTableDescriptorStateData {
-  required TableSchema unmodified_table_schema = 1;
+  required TableName table_name = 1;
   optional TableSchema modified_table_schema = 2;
 }

hbase-server/src/main/java/org/apache/hadoop/hbase/master/migrate/RollingUpgradeChore.java

@@ -121,7 +121,7 @@ private boolean isCompletelyMigrateSFT(int concurrentCount){
     for (Map.Entry<String, TableDescriptor> entry : migrateSFTTables.entrySet()) {
       TableDescriptor tableDescriptor = entry.getValue();
       InitializeStoreFileTrackerProcedure proc = new InitializeStoreFileTrackerProcedure(
-        procedureExecutor.getEnvironment(), tableDescriptor);
+        procedureExecutor.getEnvironment(), tableDescriptor.getTableName());
       procedureExecutor.submitProcedure(proc);
       processingProcs.add(proc);
     }

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

@@ -18,6 +18,7 @@
 package org.apache.hadoop.hbase.master.procedure;
 
 import java.io.IOException;
+import java.util.Objects;
 import java.util.Optional;
 import org.apache.hadoop.hbase.TableName;
 import org.apache.hadoop.hbase.client.TableDescriptor;
@@ -44,20 +45,21 @@ public abstract class ModifyTableDescriptorProcedure
 
   private static final Logger LOG = LoggerFactory.getLogger(ModifyTableDescriptorProcedure.class);
 
-  private TableDescriptor unmodifiedTableDescriptor;
+  private TableName tableName;
+
   private TableDescriptor modifiedTableDescriptor;
 
   protected ModifyTableDescriptorProcedure() {
   }
 
-  protected ModifyTableDescriptorProcedure(MasterProcedureEnv env, TableDescriptor unmodified) {
+  protected ModifyTableDescriptorProcedure(MasterProcedureEnv env, TableName tableName) {
     super(env);
-    this.unmodifiedTableDescriptor = unmodified;
+    this.tableName = Objects.requireNonNull(tableName);
   }
 
   @Override
   public TableName getTableName() {
-    return unmodifiedTableDescriptor.getTableName();
+    return tableName;
   }
 
   @Override
@@ -82,7 +84,12 @@ protected Flow executeFromState(MasterProcedureEnv env, ModifyTableDescriptorSta
     try {
       switch (state) {
         case MODIFY_TABLE_DESCRIPTOR_PREPARE:
-          Optional<TableDescriptor> modified = modify(env, unmodifiedTableDescriptor);
+          TableDescriptor current = env.getMasterServices().getTableDescriptors().get(tableName);
+          if (current == null) {
+            LOG.info("Table {} does not exist, skip modifying", tableName);
+            return Flow.NO_MORE_STATE;
+          }
+          Optional<TableDescriptor> modified = modify(env, current);
           if (modified.isPresent()) {
             modifiedTableDescriptor = modified.get();
             setNextState(ModifyTableDescriptorState.MODIFY_TABLE_DESCRIPTOR_UPDATE);
@@ -108,6 +115,15 @@ protected Flow executeFromState(MasterProcedureEnv env, ModifyTableDescriptorSta
     return Flow.HAS_MORE_STATE;
   }
 
+  @Override
+  protected boolean holdLock(MasterProcedureEnv env) {
+    // here we want to make sure that our modification result will not be overwrite by other MTPs,
+    // so we set holdLock to true. Since we do not need to schedule any sub procedures, especially
+    // no remote procedures, so it is OK for us a hold the lock all the time, it will not hurt the
+    // availability too much.
+    return true;
+  }
+
   @Override
   protected void rollbackState(MasterProcedureEnv env, ModifyTableDescriptorState state)
     throws IOException, InterruptedException {
@@ -141,7 +157,7 @@ protected ModifyTableDescriptorState getInitialState() {
   protected void serializeStateData(ProcedureStateSerializer serializer) throws IOException {
     super.serializeStateData(serializer);
     ModifyTableDescriptorStateData.Builder builder = ModifyTableDescriptorStateData.newBuilder()
-      .setUnmodifiedTableSchema(ProtobufUtil.toTableSchema(unmodifiedTableDescriptor));
+      .setTableName(ProtobufUtil.toProtoTableName(tableName));
     if (modifiedTableDescriptor != null) {
       builder.setModifiedTableSchema(ProtobufUtil.toTableSchema(modifiedTableDescriptor));
     }
@@ -153,7 +169,7 @@ protected void deserializeStateData(ProcedureStateSerializer serializer) throws
     super.deserializeStateData(serializer);
     ModifyTableDescriptorStateData data =
       serializer.deserialize(ModifyTableDescriptorStateData.class);
-    unmodifiedTableDescriptor = ProtobufUtil.toTableDescriptor(data.getUnmodifiedTableSchema());
+    tableName = ProtobufUtil.toTableName(data.getTableName());
     if (data.hasModifiedTableSchema()) {
       modifiedTableDescriptor = ProtobufUtil.toTableDescriptor(data.getModifiedTableSchema());
     }

hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/storefiletracker/InitializeStoreFileTrackerProcedure.java

@@ -18,6 +18,7 @@
 package org.apache.hadoop.hbase.regionserver.storefiletracker;
 
 import java.util.Optional;
+import org.apache.hadoop.hbase.TableName;
 import org.apache.hadoop.hbase.client.TableDescriptor;
 import org.apache.hadoop.hbase.client.TableDescriptorBuilder;
 import org.apache.hadoop.hbase.master.procedure.MasterProcedureEnv;
@@ -33,8 +34,8 @@ public class InitializeStoreFileTrackerProcedure extends ModifyTableDescriptorPr
 
   public InitializeStoreFileTrackerProcedure(){}
 
-  public InitializeStoreFileTrackerProcedure(MasterProcedureEnv env, TableDescriptor unmodified) {
-    super(env, unmodified);
+  public InitializeStoreFileTrackerProcedure(MasterProcedureEnv env, TableName tableName) {
+    super(env, tableName);
   }
 
   @Override