From c9bcd87b34a15d200a55ec7fdc2b1d86e3367a8c Mon Sep 17 00:00:00 2001 From: Duo Zhang Date: Sun, 16 Jan 2022 16:25:28 +0800 Subject: [PATCH] HBASE-26654 ModifyTableDescriptorProcedure shoud load TableDescriptor while executing (#4034) Signed-off-by: GeorryHuang --- .../server/master/MasterProcedure.proto | 2 +- .../master/migrate/RollingUpgradeChore.java | 2 +- .../ModifyTableDescriptorProcedure.java | 30 ++++++++++++++----- .../InitializeStoreFileTrackerProcedure.java | 5 ++-- .../rsgroup/MigrateRSGroupProcedure.java | 5 ++-- .../hbase/rsgroup/RSGroupInfoManagerImpl.java | 2 +- 6 files changed, 32 insertions(+), 14 deletions(-) diff --git a/hbase-protocol-shaded/src/main/protobuf/server/master/MasterProcedure.proto b/hbase-protocol-shaded/src/main/protobuf/server/master/MasterProcedure.proto index 77f2c60c56c..4f92e950a4f 100644 --- a/hbase-protocol-shaded/src/main/protobuf/server/master/MasterProcedure.proto +++ b/hbase-protocol-shaded/src/main/protobuf/server/master/MasterProcedure.proto @@ -655,6 +655,6 @@ enum ModifyTableDescriptorState { } message ModifyTableDescriptorStateData { - required TableSchema unmodified_table_schema = 1; + required TableName table_name = 1; optional TableSchema modified_table_schema = 2; } diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/migrate/RollingUpgradeChore.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/migrate/RollingUpgradeChore.java index 0417b5c8b98..b7087dd352a 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/migrate/RollingUpgradeChore.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/migrate/RollingUpgradeChore.java @@ -121,7 +121,7 @@ public class RollingUpgradeChore extends ScheduledChore { for (Map.Entry entry : migrateSFTTables.entrySet()) { TableDescriptor tableDescriptor = entry.getValue(); InitializeStoreFileTrackerProcedure proc = new InitializeStoreFileTrackerProcedure( - procedureExecutor.getEnvironment(), tableDescriptor); + procedureExecutor.getEnvironment(), tableDescriptor.getTableName()); procedureExecutor.submitProcedure(proc); processingProcs.add(proc); } diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/ModifyTableDescriptorProcedure.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/ModifyTableDescriptorProcedure.java index e11b4aba248..57d7c234fd1 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/ModifyTableDescriptorProcedure.java +++ b/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 @@ public abstract class ModifyTableDescriptorProcedure try { switch (state) { case MODIFY_TABLE_DESCRIPTOR_PREPARE: - Optional 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 modified = modify(env, current); if (modified.isPresent()) { modifiedTableDescriptor = modified.get(); setNextState(ModifyTableDescriptorState.MODIFY_TABLE_DESCRIPTOR_UPDATE); @@ -108,6 +115,15 @@ public abstract class ModifyTableDescriptorProcedure 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 @@ public abstract class ModifyTableDescriptorProcedure 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 @@ public abstract class ModifyTableDescriptorProcedure 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()); } diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/storefiletracker/InitializeStoreFileTrackerProcedure.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/storefiletracker/InitializeStoreFileTrackerProcedure.java index bd4162c58b2..5a88f99588b 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/storefiletracker/InitializeStoreFileTrackerProcedure.java +++ b/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 diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/rsgroup/MigrateRSGroupProcedure.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/rsgroup/MigrateRSGroupProcedure.java index bca77c80aa0..3c03abc9594 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/rsgroup/MigrateRSGroupProcedure.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/rsgroup/MigrateRSGroupProcedure.java @@ -19,6 +19,7 @@ package org.apache.hadoop.hbase.rsgroup; import java.io.IOException; 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; @@ -38,8 +39,8 @@ public class MigrateRSGroupProcedure extends ModifyTableDescriptorProcedure { public MigrateRSGroupProcedure() { } - public MigrateRSGroupProcedure(MasterProcedureEnv env, TableDescriptor unmodified) { - super(env, unmodified); + public MigrateRSGroupProcedure(MasterProcedureEnv env, TableName tableName) { + super(env, tableName); } @Override diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupInfoManagerImpl.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupInfoManagerImpl.java index b13afef006b..6ec3746c01b 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupInfoManagerImpl.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupInfoManagerImpl.java @@ -528,7 +528,7 @@ final class RSGroupInfoManagerImpl implements RSGroupInfoManager { // master first and then region server, so after all the region servers has been reopened, // the new TableDescriptor will be loaded. MigrateRSGroupProcedure proc = - new MigrateRSGroupProcedure(procExec.getEnvironment(), oldTd); + new MigrateRSGroupProcedure(procExec.getEnvironment(), tableName); procExec.submitProcedure(proc); procs.add(proc); }