li3zhi4 commented on PR #11677: URL: https://github.com/apache/seatunnel/pull/11677#issuecomment-5219707861
Thanks for the sharpened trace — I re-verified the same engine ordering and agree with the conclusion, and I've updated the PR description's "Reachability note" to reflect it precisely. **Engine-level re-assessment (matching your Issue 3):** in a clean full-pipeline restart — the exact scenario the e2e drives with `parallelism = 1` — each reader's `restoreState()` synchronously delivers its restored splits before that same reader registers, and `run()` fires only after all readers register, so `running` is `false` when `addSplitsBack()` is invoked and the new branch is a no-op there; the split is dispatched via the pre-existing `handleSplitRequest()` reassignment. The trigger for the new branch is a partial/regional restart where the enumerator task survives (`running == true`) while an individual reader task restarts and hands its restored splits back — which is now called out explicitly in the PR description. **Test evidence (nothing claimed beyond what the tests actually prove):** - `IncrementalSourceEnumeratorTest#shouldAssignRestoredSplitsToWaitingReader` deterministically proves the enumerator-level contract defect independent of engine timing: with the enumerator already running and a reader parked in `readersAwaitingSplit`, `addSplitsBack()` on the old code leaves the restored split unassigned (assertion fails) and on the new code dispatches it immediately (assertion passes). This is the direct, engine-independent proof of the branch the review was concerned the e2e couldn't exercise. - The e2e (`testMysqlCdcContinuesAfterSinkFailure`, `parallelism = 1`) validates the full sink-failure → checkpoint-restore → CDC-resumes path end-to-end through the real engine, real MySQL CDC source, and a real injected JDBC sink failure — i.e. "recovery still works" for the complete-restart configuration, as you characterized it. - spotless:check passes on all affected modules; the unit test passes (1/1); the e2e module test-compiles. Both tests together cover what this fix can be held to: the branch itself is proven at the enumerator boundary, and the end-to-end recovery path is proven through the real engine. The PR description now states the reachability honestly rather than overclaiming the literal ordering as the sole mechanism. A debug-log trace of the exact interleaving on a real partial-restart run remains a worthwhile follow-up, as you suggested — happy to add it in a follow-up PR if you'd prefer it in-tree before merge. -- 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]
