Vivek1106-04 opened a new issue, #12441:
URL: https://github.com/apache/seatunnel/issues/12441
### What happened
`CheckpointCoordinator.startSavepoint()` waits for the in-flight checkpoint
to drain while
holding the coordinator `lock`, and the periodic checkpoint trigger takes
that same lock. A
trigger that fires during a savepoint window therefore blocks on the
savepoint for as long as
the in-flight checkpoint takes, which is seconds under backpressure.
`CheckpointCoordinator.java:917-920` on `dev`:
```java
CompletableFuture<PendingCheckpoint> savepoint;
synchronized (lock) {
while (pendingCounter.get() > 0 && !shutdown) {
Thread.sleep(500);
}
```
`tryTriggerPendingCheckpoint` (`CheckpointCoordinator.java:771`) enters
`synchronized (lock)`
before it can check anything, so it cannot fail fast and cannot re-arm
itself; it simply waits.
`CheckpointManager.triggerSavePoints()` (`CheckpointManager.java:229-233`)
fans out over every
coordinator of the job in a `parallelStream`, so one job suspend puts all of
that job's
pipelines into this state at the same time.
### Why this is worth a dedicated issue
On `dev` today, the blocked trigger occupies a thread from that pipeline's
own
`newScheduledThreadPool(2, ...)`, so the cost is contained: a savepointing
pipeline can only
delay itself.
#12165 (STIP-40, #12339) replaces those per-pipeline pools with one
member-wide bounded
dispatch pool. The blocking is unchanged, but its blast radius is not:
enough concurrently
savepointing pipelines can occupy dispatch threads that unrelated pipelines
need for their own
triggers and timeout watchdogs. That makes this a cross-pipeline coupling
rather than a
per-pipeline one.
The mechanism is verified from source and was independently confirmed by
@DanielLeens in review.
Reaching simultaneous occupancy of the whole dispatch pool has **not** been
reproduced - it needs
periodic triggers to coincide with savepoint windows, a narrow coincidence
at the default
`checkpoint.interval` of 300000 ms, though the 500 ms re-arm at
`CheckpointCoordinator.java:841`
widens it. I am reporting the lock-holding, not a reproduced incident.
### Expected behavior
A savepoint should not hold the coordinator lock while it sleeps. Two
directions were discussed
in the #12165 review:
- **(a) Move the wait outside the lock.** Drain first, then take the lock to
create the savepoint
checkpoint. The larger and more correct change, and the one that fixes it
for `dev` as well as
for the shared scheduler.
- **(b) Make the trigger non-blocking.** `tryLock` plus reschedule in the
dispatch body, so a
trigger that cannot get the lock re-arms itself instead of waiting.
Smaller, and keeps the
change inside the new scheduler, but leaves the lock-holding in place for
every other caller.
Either wants deterministic coverage: with dispatch capacity filled by
savepoint-contended
coordinators, an unrelated pipeline still receives its trigger and watchdog
work on time.
### Context
- STIP-40: #12339
- PR: #12165, where the class Javadoc now documents this case explicitly
rather than claiming a
dispatched body is always short-lived
- Benchmark: #12418 measures the scheduling delay this path would show up in
--
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]
