SEZ9 commented on issue #12441: URL: https://github.com/apache/seatunnel/issues/12441#issuecomment-5807100783
@Vivek1106-04 thanks for recording this here. Agreed on the red-first regression as the closing condition for this issue. On part 1 (same coordinator): this is the direct check for the problem under discussion — `startSavepoint()` sleep-polling `pendingCounter` while holding the coordinator lock, which blocks `tryTriggerPendingCheckpoint()` for the whole drain. Running it against current `dev` before the fix and showing the blocking behavior is the right way to prove the test actually exercises the lock, so please treat that as a hard requirement for the PR: hold `pendingCounter > 0`, start the savepoint, call `tryTriggerPendingCheckpoint()`, and assert it returns well within the drain time, creates no new pending checkpoint, and re-arms. This also folds in the "no non-savepoint pending checkpoint while the gate is set" check from the constraints above, so one deterministic test covering both is fine. On part 2 (cross-pipeline): agreed that it can only go red on a base that includes the shared dispatch pool from #12165, so the ordering you laid out works for me — both parts in one PR if #12165 lands first, otherwise part 1 lands with the fix and part 2 follows once #12165 is in. Either way this issue stays open until part 2 has been shown red then green. Remaining asks for whoever opens the fix PR: 1. Implement the unlocked drain per the four constraints already recorded above (shared draining future installed under the lock, wait outside the lock, exceptional completion on every interrupt/shutdown/completed/create-failure path, gate honored by all non-savepoint trigger paths with re-arm). 2. Include the part 1 regression and, in the PR description, the red run: which test, which base revision, and the observed failure. 3. If part 2 is included, add the same red-run record for it; if not, note in the description that part 2 is pending #12165 so we don't lose track. 4. Keep the #12442 trigger-failure policy out of this change. <!-- streview-comment:1288 --> -- 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]
