Skip to content

HBASE-28248 Race between RegionRemoteProcedureBase and rollback opera… #5567

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.

Sign up for GitHub

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
Apache9 merged 1 commit into from
Dec 9, 2023
Merged

HBASE-28248 Race between RegionRemoteProcedureBase and rollback opera… #5567

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
16 changes: 15 additions & 1 deletion ...er/src/main/java/org/apache/hadoop/hbase/master/assignment/RegionRemoteProcedureBase.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@
import org.apache.hadoop.hbase.shaded.protobuf.generated.MasterProcedureProtos.RegionRemoteProcedureBaseState;
import org.apache.hadoop.hbase.shaded.protobuf.generated.MasterProcedureProtos.RegionRemoteProcedureBaseStateData;
import org.apache.hadoop.hbase.shaded.protobuf.generated.ProcedureProtos;
import org.apache.hadoop.hbase.shaded.protobuf.generated.ProcedureProtos.ProcedureState;
import org.apache.hadoop.hbase.shaded.protobuf.generated.RegionServerStatusProtos.RegionStateTransition.TransitionCode;

/**
Expand Down Expand Up @@ -183,7 +184,20 @@ protected abstract void updateTransitionWithoutPersistingToMeta(MasterProcedureE
// A bit strange but the procedure store will throw RuntimeException if we can not persist the
// state, so upper layer should take care of this...
private void persistAndWake(MasterProcedureEnv env, RegionStateNode regionNode) {
env.getMasterServices().getMasterProcedureExecutor().getStore().update(this);
// The synchronization here is to guard with ProcedureExecutor.executeRollback, as here we will
// not hold the procedure execution lock, but we should not persist a procedure in ROLLEDBACK
// state to the procedure store.
// The ProcedureStore.update must be inside the lock, so here the check for procedure state and
// update could be atomic. In ProcedureExecutor.cleanupAfterRollbackOneStep, we will set the
// state to ROLLEDBACK, which will hold the same lock too as the Procedure.setState method is
// synchronized. This is the key to keep us safe.
synchronized (this) {
if (getState() == ProcedureState.ROLLEDBACK) {
LOG.warn("Procedure {} has already been rolled back, skip persistent", this);
return;
}
env.getMasterServices().getMasterProcedureExecutor().getStore().update(this);
}
regionNode.getProcedureEvent().wake(env.getProcedureScheduler());
}

Expand Down