DanielLeens commented on PR #11382:
URL: https://github.com/apache/seatunnel/pull/11382#issuecomment-5601213435
@SEZ9 same issue as before — the comment was not actually truncated in the
underlying data; I just re-pulled the raw body via the API and the full text is
there (4358 characters, ending cleanly). This looks like a recurring GitHub UI
"show more" collapse on long comments rather than a real cut-off, on both our
ends apparently. Reposting the missing continuation here, plus a direct answer
to your open question, since I've actually gone further on this since that
comment:
**The missing continuation** (right after "...nothing re-deserializes it —
because nothing in `cleanPendingCheckpoint`"):
> `cleanPendingCheckpoint` (`:1128-1131`, called at `:681` during
`restoreCoordinator`) resets it either; the only writes to
`latestCompletedCheckpoint` are the constructor (once, at coordinator creation)
and `notifyCheckpointCompleted`'s assignment (`:1330`) on each new completion.
So when the freshly-restarted sink task (fresh `SchemaChangeDrainGuard`,
`schemaChangeDrainReady=false`) comes back up and reports `READY_START`, it
drives exactly the `allTaskReady()` ->
`notifyCompleted(latestCompletedCheckpoint)` replay I described for F2 — same
mechanism, and if anything a stronger guarantee here than the full-failover
case, since there's no deserialize-from-storage step at all.
**Direct answer to your question ("what code path re-sends the typed
`CheckpointFinishedOperation` ... to the new task instances?")**: X is exactly
the sentence above — there is no *separate* re-delivery path, and none is
needed. The restarted task's own `READY_START` report is what retriggers
`allTaskReady()`'s existing fan-in check; once every subtask (including the
freshly-recreated one) has reported in, `allTaskReady()` calls
`notifyCompleted(latestCompletedCheckpoint)`, which fans out via
`notifyCheckpointCompleted` to every current subtask instance — that's the same
typed `CheckpointFinishedOperation` delivery F2's fix already relies on, just
triggered by a peer's `READY_START` report instead of the coordinator's own
explicit push (which is what happens instead in the
master-failover/`alreadyStarted=true` branch). So: F2 is not narrowed to the
coordinator-recreated shape only — both shapes route through the same
`notifyCompleted(latestCompletedCheckpoint)` call, they j
ust differ in what triggers it.
**One correction to davidzollo's `1ee22aced02` push, which is relevant to
your question**: davidzollo's last reply claimed the "coordinator survives
while one task restarts" scenario "isn't reachable" — that every task restart
is pipeline-scoped and goes through a cancel -> reset -> recreate cycle that
rebuilds the coordinator too. I traced this independently in my full re-review
of `1ee22aced02` (posted after your comment, so you may not have seen it yet:
https://github.com/apache/seatunnel/pull/11382#pullrequestreview-5148573950)
and that claim doesn't hold: `CheckpointManager.coordinatorMap` is populated
once in the constructor, and neither `cancelCheckpoint` nor
`reportedPipelineRunning` (the calls on `SubPlan.prepareRestorePipeline()`'s
ordinary task-restart path) remove or replace the map entry — they look up and
mutate the *existing* coordinator instance. So the
task-level-restart-while-coordinator-survives shape genuinely is reachable on
the current head, and it's exac
tly the shape your question is about. The new javadoc `1ee22aced02` added
asserts the opposite (coordinator "always recreated"), which I flagged as Issue
1 (Medium) in that review — the fix's actual behavior is correct either way
(the `READY_START`-triggered replay covers it), but the stated rationale in the
comment is wrong and should be corrected before merge so a future reader
doesn't rely on the incorrect "coordinator recreation" explanation.
So to close out your bullet points: F2 stays resolved (both restart shapes
replay correctly, for different reasons); F4/F5 aren't reopened by this — the
in-process guard state loss on task recreation is exactly what the
`READY_START`-triggered replay compensates for, whether or not the coordinator
itself was rebuilt.
--
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]
