[alpass163]

Summary

Fix the procedure frame that prohibiting rollback of a procedure which has passed its commit point

• Procedure.isRollbackSupported() lets a procedure declare that its executed states can no
  longer be undone. The executor asks it once, before any state is rolled back, and finishes
  such a procedure without touching its states; the client still sees the failure.
• The decision is persisted by clearing the stack indexes only, so the procedure store format
  is unchanged and rolling upgrades are not affected.
• Each table procedure declares its commit point: the state which applies its change on the
  ConfigNode (ADD_COLUMN, RENAME_TABLE, SET_PROPERTIES, ...), or the point after which
  the data is gone (DELETE_DATA).

[Caideyipi]

1. Better reuse the original "isRollbackSupported", or else other people may not know which to override, and the incoming reviewers may be confused and let the wrong implementation passed too.
2. If you have time, you can also seek are there any other procedures which does not support rollback previously and can be supported in some stages after this PR

[alpass163]

1. Better reuse the original "isRollbackSupported", or else other people may not know which to override, and the incoming reviewers may be confused and let the wrong implementation passed too.
2. If you have time, you can also seek are there any other procedures which does not support rollback previously and can be supported in some stages after this PR

Ok, I reuse the original api

[Caideyipi]

Found a recovery lock leak and a small localization issue; details are inline.

Validation: the PR's five new state-machine tests pass. Two additional lock-recovery cases, including one using the real AddConfigNodeProcedure and LockQueue, fail on 754625cce26 and pass on the base d1aaa0dd909. This used targeted compilation and JUnit with cached dependencies; cluster integration tests were not run.

[Caideyipi]

The recovery lock leak and retry-operation localization issue from the previous review are fixed. I found no further blocking issues in the current change.

Validation: all five new state-machine tests and two targeted lock-release cases using the real AddConfigNodeProcedure and LockQueue pass on eed5db9. These used targeted compilation with cached dependencies; the local reactor run stopped before ConfigNode because DataNode Checkstyle ran out of heap. CI unit tests on Linux/Windows and zh-locale compilation passed.

One CI integration shard remains red: IoTDBPipePermissionIT.testSourcePermission observes unexpected replicated rows. This still needs investigation or a successful rerun before merging.

[JackieTien97]

Requesting changes for a recovery ordering bug in finishWithoutRollback(): releasing the lock can persist a completed root before its child records are deleted, leaving a durable state that a new executor cannot recover. Details are inline.

Validation:

• A clean Maven reactor build and 22 targeted state-machine, executor, and lock tests passed.
• Two additional serialized-snapshot recovery tests using the real RemoveDataNodesProcedure and RegionMigrateProcedure fail on eed5db94367 and pass with the changed production classes rebuilt from base d1aaa0dd909.
• Cluster integration tests were not run locally.

[JackieTien97]

[P1] Delete child records before persisting the root's terminal state

releaseLock() is not just an in-memory unlock: Procedure.doReleaseLock() calls store.update(), and this method has already marked the procedure ROLLEDBACK. For a root that skips rollback, such as RemoveDataNodesProcedure, this persists the completed parent before the later store.delete(childProcIds) call. A crash or leader change between those writes can therefore leave a completed root alongside its child records.

I reproduced this using real RemoveDataNodesProcedure and RegionMigrateProcedure serialization: with one failed child and one pending child, take a snapshot at the first write of the completed root and initialize a new executor from it. The snapshot still contains a ROLLEDBACK child, and ProcedureExecutor.recover() throws UnsupportedOperationException("Unexpected state"), preventing executor initialization. The same snapshot-boundary tests pass on the base implementation.

Please make the child cleanup durable before the unlock persists the root's terminal state, or make recovery safely handle this intermediate state.

[alpass163]

fixed it

[JackieTien97]

The previous ordering issue is fixed when child cleanup completes, and the original reproduction now passes. Keeping changes requested because an interrupted cleanup retry can still leave a completed root alongside a child record and prevent executor recovery. Details are inline.

Validation on 7fd9f8bd83:

• Clean Maven reactor build and all 22 targeted state-machine, executor, and lock tests passed.
• Both original snapshot-recovery tests passed, as did both control tests with a transient deletion failure and uninterrupted retry.
• Both tests that interrupt the child-deletion retry failed: the completed-root snapshot retains the child, and recovery throws UnsupportedOperationException("Unexpected state"). These used the current clean build's classes and real RemoveDataNodesProcedure / RegionMigrateProcedure serialization.
• Cluster integration tests were not run locally.

[JackieTien97]

[P1] Abort root finalization when the child-cleanup retry is interrupted

RetryUtils.executeWithEndlessBackoffRetry() returns normally when its backoff sleep is interrupted: it restores the interrupt flag but neither rethrows nor reports that the operation did not succeed. Consequently, returning from this call does not guarantee that store.delete(childProcId) completed. If cleanup fails and the worker is interrupted during retry, this method still proceeds to mark the root ROLLEDBACK, discard its history, and attempt to persist its terminal state. If that update is accepted, a child record remains beside the completed root.

I reproduced this with the clean build of this revision and real RemoveDataNodesProcedure / RegionMigrateProcedure serialization: inject one child-deletion failure, then interrupt the thread during the retry's backoff. The final snapshot retains the ROLLEDBACK child, and initializing a new executor from it throws UnsupportedOperationException("Unexpected state"). The same tests pass when the transient deletion failure is allowed to retry without interruption.

Please explicitly distinguish successful cleanup from interrupted cleanup and propagate cancellation out of the entire rollback path, so neither the terminal-state write nor rootProcedureCleanup() runs after incomplete child deletion. Simply returning from finishWithoutRollback() would still allow its caller to perform root cleanup.

[JackieTien97]

The previously reported premature root-finalization and interrupted-cleanup issues are addressed in 9fa62fc8d1. Interruption now propagates through the rollback call chain instead of falling through to root cleanup, and rollback-stack entries are retained until finalization completes. I found no further blocking regression in the latest changes.

Validation:

• A clean Maven reactor build and all 22 targeted state-machine, executor, and lock tests passed.
• Six additional tests passed against the current clean build's classes: the original completed-root snapshot checks, interrupted child cleanup preserving the active root and its history, recovery with a remaining pending child, interrupted terminal-write recovery, and successful retry after a transient deletion failure.
• The three cancellation regression cases fail on 7fd9f8bd83 and pass on this revision.
• Cluster integration tests were not run locally.