zhangshenghang opened a new pull request, #12559: URL: https://github.com/apache/seatunnel/pull/12559
### Purpose of this pull request Close #12558. Since #10687, `CheckpointCoordinator#updateStatus` skips a status transition when the `checkpoint_state_<job>_<pipeline>` key is absent, and the job state cleanup removes that key without coordination with the write. This PR fixes both sides of that split: 1. **Gate on the owning job state instead of the state key.** `updateStatus` now persists a transition only while the owning job is alive (the job key in `runningJobStateIMap` is a `JobStatus` and not an end state). This keeps the cleaned-job protection of #10687 (a cleaned job has no job key anymore) but restores what #10687 accidentally broke: a live job can create its `checkpoint_state_*` key on the first transition, so coordinator status persistence works again for jobs that start fresh. Without this, no transition was ever persisted for fresh jobs, and the coordinator-status based restore / `isNoErrorCompleted` paths could never take effect for them. 2. **Coordinate the write and the cleanup with the per-job lock.** The check-and-write in `updateStatus` and the removal of the job key + `checkpoint_state_*` keys in `CoordinatorService#cleanupPendingJobStateMaps` now run while holding the same per-job lock of `runningJobStateIMap`, so a late coordinator callback can never recreate a key the cleanup already removed. Both waits are bounded (5s on the checkpoint path, 30s on the cleanup path); the remaining cleanup keys (pipeline/task-group locations, state timestamps) are still removed outside the lock to keep the critical section short. When the cleanup cannot acquire the lock it keeps the pending cleanup record and reschedules it, so the cleanup is retried later instead of being lost. 3. **`isNoErrorCompleted` treats a missing state as not no-error-completed** with an explicit error log instead of silently evaluating the expression (upstream was already null-safe, this only makes the degraded case visible). ### Does this PR introduce _any_ user-facing change? No API or configuration change. Behavior differences: - Live jobs persist their `checkpoint_state_*` status again (this stopped working after #10687 for jobs that started fresh). - Late coordinator callbacks after job cleanup no longer leave orphaned `checkpoint_state_*` entries in the IMap. - A job state cleanup that would race with an in-flight transition is retried slightly later instead of racing. ### How was this patch tested? - `CheckpointCoordinatorTest#testUpdateStatusShouldPersistStateOnlyForLiveJob`: a live job creates the state key on the first transition; terminal and cleaned jobs no longer persist/resurrect it. - `CheckpointCoordinatorTest#testUpdateStatusWaitsForJobStateLockAndCannotResurrectCleanedState`: while the job state lock is held the way the cleanup holds it, a late callback cannot write; after the cleanup removes the job key and releases the lock, the callback does not resurrect the key; a live job persists again afterwards. - `CheckpointCoordinatorTest#testIsNoErrorCompletedShouldTolerateMissingStateKey`: a missing state key is treated as not no-error-completed; persisted FINISHED of the same final checkpoint is no-error-completed; other statuses are not. - `CoordinatorServiceJobCleanupTest#testCleanupWaitsForJobStateLockAndCompletesAfterLockReleased`: the pending cleanup removes nothing and keeps its record while the lock is held, and completes after the lock is released. - Full module suite: `mvn -pl seatunnel-engine/seatunnel-engine-server test`. -- 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]
