Rangsh opened a new pull request, #12493: URL: https://github.com/apache/seatunnel/pull/12493
## Purpose Fixes #12441. `CheckpointCoordinator.startSavepoint()` drained in-flight checkpoints with `Thread.sleep` **inside** `synchronized (lock)`. Periodic / completed-point / schema-change `tryTriggerPendingCheckpoint` calls take that same lock before they can skip or re-arm, so a trigger that fires during a savepoint window blocks for the full drain. This PR moves the drain wait outside the lock, installs one shared savepoint request/future + drain gate under the lock (DanielLeens constraints), and makes every non-savepoint trigger path re-arm while the gate is set. #12442 unexpected-trigger-failure policy is intentionally not touched. ## Changes - `startSavepoint()`: install shared `savepointRequestFuture` + `savepointDraining` under the lock, sleep-poll **outside** the lock, short critical section only for create/start; clear the gate and complete the shared future exceptionally on interrupt / shutdown / completed / create failure. - `tryTriggerPendingCheckpoint()`: while `savepointDraining`, skip create and re-arm (500ms) for general / completed-point / schema-change. - `createPendingCheckpoint()`: defensive reject of non-savepoint creates while draining. - Unit coverage in `CheckpointCoordinatorTest` (part 1 same-coordinator regression + gate / coalesce / interrupt-cleanup). ## Red / green evidence (part 1) Per SEZ9 / Vivek: regression written red-first against current blocking lock acquisition. | | | |---|---| | **Test** | `CheckpointCoordinatorTest#testTryTriggerNotBlockedBySavepointDrain` | | **Base revision (red)** | `c3f06f9a82c119e427df0fb1f440bae583148f62` (`upstream/dev`) | | **Red failure** | `AssertionFailedError: tryTriggerPendingCheckpoint must return while savepoint drain is still held (pendingCounter>0); blocked for the full drain indicates the lock is held across sleep-poll ==> expected: <true> but was: <false>` | | **Green (this PR)** | `72fd3eff2` — same test + `testSavepointDrainGateReArmsWhenPendingCounterZero` / `testConcurrentStartSavepointSharesOneRequest` / `testSavepointDrainInterruptClearsGateAndAllowsTrigger` — `Tests run: 4, Failures: 0` | Drain lifetime is owned by the test via `pendingCounter` (kept `> 0` until assertions finish), not wall-clock sleeps. The same test covers general, completed-point, and schema-change re-arm. ## Part 2 (cross-pipeline) **Pending #12165.** Shared dispatch pool is not on `dev` yet, so the cross-pipeline occupancy regression cannot go red on this base. Once #12165 lands, part 2 should be added (Vivek offered to write it); this issue stays open until part 2 is shown red then green. ## Check list ### Does this PR need Docs? - [ ] No. Docs for this feature already exist. - [ ] Yes. Docs have been added. - [x] No. This PR does not need Docs. ### Does this PR need any UT/E2E? - [x] Yes. This PR contains UT/E2E for the relevant changes. - [ ] No. This PR does not need UT/E2E. Made with [Cursor](https://cursor.com) -- 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]
