Vivek1106-04 commented on issue #12339:
URL: https://github.com/apache/seatunnel/issues/12339#issuecomment-5776679169
@SEZ9 Thanks for going through the paired run. Three things done, and one
question I
would rather ask than assume.
**1. The ~3 us is now in the STIP text, not rounded away.** The cost below
the P=100
crossover is stated as the extra timer-to-dispatcher hop, with the crossover
named, in
the framing you asked for.
**2. The dispatcher contract is corrected in the STIP body, as a marked
Correction.**
The section on bounding the dispatch pool previously said a dispatched body
is always
short-lived because the barrier work runs on `thenApplyAsync`. That was too
strong. It
now names both paths that were checked against the source rather than
assumed:
- The timeout watchdog ran its expiry inline on the dispatch thread -
`updateStatus`
drives IMap reads and writes through a `RetryUtils` loop, and
`cleanPendingCheckpoint`
takes the coordinator lock. That is fixed in #12165 at `da0cbbf52`: the
dispatch thread
keeps the pending-checkpoint lookup, the expiry goes to the coordinator
executor, and
because that executor aborts when saturated and a dropped expiry would
leave the
checkpoint pending with nothing left to fail it, a rejection falls back to
running
inline with a WARN.
- `startSavepoint` holds the coordinator lock across its 500 ms sleep-poll
while
`tryTriggerPendingCheckpoint` takes the same lock on a dispatch thread,
with
`triggerSavePoints` fanning out over every coordinator of a job at once.
Stated as a
new cross-pipeline coupling and as a tracked follow-up rather than
something this
design already handles.
I also amended "Alternatives Considered" item 1 so the STIP is not
self-contradictory:
dispatching the scheduling onto the coordinator executor stays rejected, but
the
document now says it is the right destination for an individual piece of
work where a
rejection can be caught rather than lost, which is what the expiry hop does.
The new
regression is in the test plan.
**3. The saturation figures are attached to the residual-cost paragraph**,
so the
"flat to 31%, four orders of magnitude past 100%" shape is in the design
document rather
than only in this thread.
**The question.** @DanielLeens replied on #12165 a few hours after your
comment here and
split the gate: he wants the watchdog hop in that PR, which is done, and is
willing to
treat the savepoint lock-holding as a tracked follow-up with corrected docs,
given the
simultaneous-occupancy case needs a coincidence I have not been able to
demonstrate.
Your comment here keeps the regression - dispatch capacity filled with
savepoint-contended coordinators, unrelated pipeline still served - as a
gate on
acceptance.
I have implemented the reading that is already actionable and left the other
open rather
than pick between you. If the regression is still required before #12165 is
accepted, I
will write it red against the current design and then fix with `tryLock`
plus reschedule
inside the dispatch body, so the change stays inside this STIP's scheduler
instead of
touching `dev`'s savepoint path. Tell me which and I will start.
--
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]
