zhangshenghang commented on PR #12162: URL: https://github.com/apache/seatunnel/pull/12162#issuecomment-5586625845
Thank you for the thorough review — the batch/no-checkpoint hang you traced is real, and the fix commit below addresses it along with the test gap. Point-by-point: **Issue 1 (High, blocker) — fixed in 879fb0ada5, by reverting the `currentParallelism == 1` removal.** I verified your trace end-to-end on the PR branch before changing anything: `CoordinatorService#isCheckpointEnabled` disables checkpointing for `job.mode=BATCH` without `checkpoint.interval`, `CheckpointCoordinator#allTaskReady` then never schedules a trigger, so `SourceSplitEnumeratorTask#notifyCheckpointComplete` (the only path into `SnapshotSplitAssigner#notifyCheckpointComplete`) is never invoked and `assignerCompleted` stays false — `HybridSplitAssigner#getNext` returns empty forever and `IncrementalSourceEnumerator#assignSplits` keeps breaking on `waitingForCompletedSplits()` without signaling no-more-splits. The job hangs exactly where it previously completed. We fixed it by restoring the single-parallelism immediate completion rather than guarding the wait behind a checkpoint-enabled flag (your Option A), because exposing checkpoint-enabled state through `SourceSplitEnumerator.Context` (plus the Zeta engine plumbing to populate it) is an SPI/engine change well beyond this PR's scope, and the multi-parallelism batch-without-interval hang predates this PR (it has been there since #10859). With the revert, no previously-working configuration regresses. Why the revert does not re-open the failover gap the removed branch was meant to close: single-parallelism durability no longer relies on the checkpoint wait. `IncrementalSourceReader#snapshotState` includes all `finishedUnackedSplits` in the reader's own checkpoint state, restores them into `finishedUnackedSplits` on restart, and re-reports them via `reportFinishedSnapshotSplitsIfNeed`; on the enumerator side, `restoreCompletedSnapshotSplit` back-fills any missing watermark from the split itself instead of replaying the split. So a preempted single reader still converges without a checkpoint pinning the completion state. **Issue 2 (Low) — the mechanism, with pointers:** The finished-but-unacked split survives a reader-level static-slot restart through the reader's own checkpoint: `IncrementalSourceReader#snapshotState` (connector-cdc-base) appends `finishedUnackedSplits.values()` to the state list returned by `super.snapshotState`, and `IncrementalSourceReader#pollNext`/restore path puts them back and re-reports them (`reportFinishedSnapshotSplitsIfNeed`) until the enumerator acks. `SourceReaderBase#snapshotState` alone indeed drops finished splits, but the CDC reader overrides it precisely to carry the finished-unacked set. The enumerator-side `addSplits` back-fill in this PR then covers the add-back path where the split object itself carries the watermark. **Issue 3 (Medium) — tests added in 879fb0ada5:** - `testSingleParallelismShouldCompleteImmediatelyWhenAllSplitsCompleted`: a parallelism-1 assigner completes immediately once all watermarks arrive, with no checkpoint involved — this documents the batch-safe behavior and guards the restored branch. - `testMultiParallelismShouldWaitForCheckpointBeforeCompleted`: a parallelism-10 assigner stays incomplete after `onCompletedSplits`, only completes after `snapshotState` pins `checkpointIdToFinish` and a `notifyCheckpointComplete` with `checkpointId >= checkpointIdToFinish` arrives. Since the parallelism-1 latency change is gone, no user-facing completion-latency delta remains, so no CDC doc change is needed for it. Full connector-cdc-base suite passes locally (`clean spotless:check test`, 85 tests, 0 failures), which also resolves the code-style CI failure on the previous head. -- 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]
