DanielLeens commented on PR #12165: URL: https://github.com/apache/seatunnel/pull/12165#issuecomment-5593934880
@nzw921rx Thanks — good question, and worth spelling out since it's exactly the kind of interaction an engine/checkpoint reviewer should check on this PR. Short answer: the savepoint API path does **not** go through the periodic re-arm loop (`scheduleTriggerPendingCheckpoint` / `tryTriggerPendingCheckpoint`) at all, so there's no conflict with `PipelineCheckpointScheduler`'s periodic-trigger role. It only touches the shared scheduler for the same single purpose every checkpoint type uses it for: arming that checkpoint's timeout watchdog. Tracing it on the current head (`85efedd`): 1. `CheckpointCoordinator.startSavepoint()` (`CheckpointCoordinator.java:824`) busy-waits under `synchronized (lock)` while `pendingCounter.get() > 0` (`:840`) — i.e. it waits for any in-flight checkpoint (periodic or otherwise) on *this pipeline* to drain — then calls `createPendingCheckpoint(..., SAVEPOINT_TYPE)` and `startTriggerPendingCheckpoint(savepoint)` directly (`:847-848`). 2. That's a completely separate code path from `tryTriggerPendingCheckpoint()` (the one that calls `scheduler.schedule(...)` for periodic re-arming at `:523` and the `pendingCounter > 0` back-off reschedule at line ~792 on `dev`'s equivalent). A savepoint never enters that method, so it never asks the scheduler to re-arm a periodic trigger. 3. The only place a savepoint touches `scheduler` at all is inside `startTriggerPendingCheckpoint()` (`:901`), which every checkpoint type shares: it calls `scheduler.schedule(...)` once (`:973`) purely to arm the checkpoint-timeout watchdog for *that* checkpoint id. So in concrete terms: if you call the savepoint API while a periodic checkpoint is already pending on that pipeline, the savepoint call blocks (sleeping 500ms at a time) until `pendingCounter` drops to 0, then fires immediately and synchronously — it doesn't wait in the scheduler's timer queue the way a periodic re-arm does. Once it fires, its timeout watchdog is scheduled on the exact same per-pipeline lease a periodic checkpoint's watchdog would use, but since `pendingCounter` (untouched by this PR) already guarantees only one pending checkpoint per pipeline at a time, there's no scenario where a savepoint's watchdog and a periodic checkpoint's watchdog are both live for the same pipeline simultaneously. That invariant is exactly what I traced and confirmed in my review above, and this PR doesn't change any of it — it only changes *where the timer pool lives*, not when/whether a checkpoint of any type gets triggered. -- 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]
