Copilot commented on code in PR #18212:
URL: https://github.com/apache/iotdb/pull/18212#discussion_r3584800127
##########
iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/procedure/ProcedureExecutor.java:
##########
@@ -810,6 +810,13 @@ private long pushProcedure(Procedure<Env> procedure) {
// Update metrics on start of a procedure
procedure.updateMetricsOnSubmit(getEnvironment());
RootProcedureStack<Env> stack = new RootProcedureStack<>();
+ // Persisting a newly submitted procedure may serialize and deserialize it
through the
+ // consensus layer. If that process marks the procedure as failed before
it is scheduled, the
+ // rollback stack still needs an entry so the executor can finish the
failed procedure instead
+ // of leaving it in the active procedure map forever.
+ if (procedure.isFailed()) {
+ stack.addRollbackStep(procedure);
+ }
Review Comment:
`pushProcedure()` adds a rollback step when the procedure is already FAILED,
but that also mutates the procedure’s `stackIndexes` (via `addRollbackStep`).
Since this happens after the initial `store.update()` in `submitProcedure()`,
the rollback step isn’t persisted. If the ConfigNode leader crashes/restarts
before the rollback completes, recovery will load a FAILED root procedure with
no stack and `executeRootStackRollback()` will hit an NPE when it assumes
`RootProcedureStack.getSubproceduresStack()` is non-null.
Consider persisting the updated failed procedure (with the initialized
rollback stack entry) immediately after adding the rollback step so recovery
can safely roll it back.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
