Skip to content

HBASE-26654 ModifyTableDescriptorProcedure shoud load TableDescriptor… #4034

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jan 16, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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
Expand All @@ -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);
Expand All @@ -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 {
Expand Down Expand Up @@ -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));
}
Expand All @@ -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());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

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;
Expand All @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -528,7 +528,7 @@ private void migrate(Collection<RSGroupInfo> groupList) {
// 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);
}
Expand Down