li3zhi4 commented on PR #11618: URL: https://github.com/apache/seatunnel/pull/11618#issuecomment-5159431390
Thanks for the detailed re-review, @DanielLeens. I've fixed the tail-data-loss race in the completion handshake (commit `5b088374e5`): **What changed in `IncrementalSourceStreamFetcher`:** - `pollSplitRecords()` now **always drains the queue first**; split completion (`return null`) is only signaled after the queue is empty AND the bounded split is finished. Previously it returned `null` before draining, which could drop the last queued batch. - `splitNormalStream()` guard now accepts the bounded-finished state (`isRunning() || isBoundedReadFinished()`), so the final batch is emitted even after `isRunning()` flipped false — previously the second `isRunning()` guard could filter out a just-drained tail batch. - Extracted `isBoundedReadFinished()` helper shared by both places. **New unit test** `testPollSplitRecordsDrainsQueueBeforeBoundedCompletion`: simulates a finished bounded fetch task with one last batch still in the queue — asserts the batch is returned first, and only the subsequent poll (empty queue) returns `null`. **Verification (all local):** - `spotless:check` ✅ - `IncrementalSourceStreamFetcherTest` → `Tests run: 2, Failures: 0` ✅ - `MysqlCDCStopModeSpecificIT` e2e (Docker, Zeta engine) → `Tests run: 1, Failures: 0, Errors: 0` ✅ — the e2e already asserts the pre-stop-offset rows (21/22) are synced, which covers exactly this tail-batch scenario - Branch is up to date with `dev` (merged `1b4d6c2ef8`) I believe all previously raised points are now addressed. Happy to iterate further if anything else comes up. -- 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]
