SEZ9 commented on PR #12559: URL: https://github.com/apache/seatunnel/pull/12559#issuecomment-5924338911
Thanks for tracking this down. The problem description makes sense to me: with the state-key gate from the earlier change, a fresh job's `checkpoint_state_*` key is only ever created by `updateStatus` itself, so coordinator status never gets persisted, and the cleanup path removes those keys without coordinating against a late callback. Gating the write on the owning job key being a live, non-terminal `JobStatus` and running both the check-and-write and the cleanup's removal under the same `runningJobStateIMap` lock on `jobId` seems like the right direction. A few things I'd like to understand better before approving: 1. **Lock-timeout behaviour in `updateStatus`.** When the `tryLock` on `jobId` fails, is the transition dropped, logged, or retried? Silently dropping a RUNNING/FINISHED transition on a contended lock would reintroduce a rarer version of the original bug, so it would be good to spell out the intended behaviour in a comment and at least log it at WARN. 2. **Work done outside the lock in cleanup.** The cleanup removes pipeline/task-group keys and timestamps after releasing the lock. Could you confirm nothing in the coordinator reads or writes those keys gated on the job key? If there is such a path, it should probably move inside the same critical section. 3. **Test coverage.** Does this PR include a regression test for the fresh-job case (a new job ends up with its `checkpoint_state_*` key populated) and for the race described in the PR (cleanup removes the key, a late `updateStatus(FINISHED)` arrives, no orphan key is recreated)? If so, please point me to them; if not, it would be worth adding. Happy to take another pass once these are clarified. <!-- streview-comment:1451 --> -- 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]
