Vivek1106-04 commented on PR #12623: URL: https://github.com/apache/seatunnel/pull/12623#issuecomment-6539053075
@SEZ9 @DanielLeens The fork Build for `caa5f098f` finished green: https://github.com/Vivek1106-04/seatunnel/actions/runs/40727927696 (`success`, head `caa5f098fc0a5c6dd887ba8632dc43d463eb4580`). The head has not changed since then. **F1 (restore thread after the sleep).** The post-sleep re-check is in #12511. In `SubPlan.stateProcess()` at `a691fb195`, see [`SubPlan.java#L776-L779`](https://github.com/apache/seatunnel/blob/a691fb195d32381b3a43560eba4586861366a6d8/seatunnel-engine/seatunnel-engine-server/src/main/java/org/apache/seatunnel/engine/server/dag/physical/SubPlan.java#L776-L779): ```java if (prepareRestorePipeline()) { // returns after the pipelineRestoreIntervalSeconds sleep if (!jobMaster.isNeedRestore()) { abandonRestore(state, failure); // ends the pipeline in its terminal state return; // before releasePipelineResource / preApplyResources / restorePipeline } ... ``` `forceStopPipeline()` is called after `jobMaster.neverNeedRestore()`, so a stop that lands during the wait makes this check false. The restore thread then ends the pipeline instead of releasing and re-applying slots or restarting the state process. That is the guard you asked for, and it is why this PR has to merge after #12511. I have not added it here because it would duplicate the same check, and whichever PR landed second would have to remove it. #12511 is approved and green, and it is waiting only for a committer's merge. **F2 (pipeline-level proof).** The whole-job scenario from #12622 (stop during the restore wait, and the job leaves `DOING_SAVEPOINT`) is asserted in #12511: - `SavePointTest#testSavePointFailureDuringPipelineRestoreWaitEndsTheJob` (non-cancel path) - `CheckpointErrorRestoreEndTest#testCancelDuringPipelineRestoreWaitEndsTheJobCanceled` (cancel path) This PR keeps the tests for the window that remains after that re-check: a stop that lands after the check passes and before the restarted tasks are deployed. In that window the vertices are `CREATED` with a stopped state process, which is the state these tests pin: - `StateTransitionCleanupTest#testForceStopCompletesTheFutureOfATaskWhoseStateProcessIsStopped` - `StateTransitionCleanupTest#testForceStopCompletesTheFutureOfATaskResetForRestore` (FAILED -> `reset()` -> `forceStop()`, the post-recheck paused-vertex case) - `StateTransitionCleanupTest#testForceStopLeavesTheStateProcessStoppedWhenTheTransitionIsSkipped` All three fail if the `PhysicalVertex` change is reverted. Nothing is left to change in this PR. Merge order is unchanged: #12511 first, then this one. -- 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]
