DanielLeens commented on PR #10874: URL: https://github.com/apache/seatunnel/pull/10874#issuecomment-5633040451
Thanks @SEZ9 for consolidating this — agreed that's an accurate summary of where things stand, and I don't have anything to add on the technical substance beyond what's already in the thread. To be explicit about ownership, these three are asks for @davidzollo, not open review disagreements between us: - **F2** — still the blocker from my side: `closeTable()` (`MultiTableSinkWriter.java:769-838`) closes sub-writers without an interposed `prepareCommit`, so uncommitted 2PC data for a closed table is dropped. Needs a short paragraph in the PR description stating that behavior and why it's acceptable (or the mitigation), before I'd consider the merge bar clear. - **F3** — deferral is fine given it's a checkpoint-recoverable task failure rather than silent data loss, but the tracked follow-up issue for the queue-poll/`closeTable()` writer-removal race still needs to be filed and linked from the PR description. Not yet done as far as I can see. - **F4/F6 `Math::max` question** — still an open question for @davidzollo: can a stale, larger `expectedSourceEventCount` from an older reader generation pin `requiredCount` (`MultiTableSinkWriter.java` `handleCloseTableEvent`) above what will ever arrive, silently falling back to close-at-task-end? A warn log on that path, or a sentence in the PR description explaining why it can't happen, resolves this for me either way. On the sync/CI ask: I checked the current head (`ce9c17406b7`) against `dev` again — `ahead_by=21`, `behind_by=122`, compare status `diverged`, and `mergeable_state` is now `dirty` (real merge conflicts reported by GitHub, not just a stale-CI-signal situation). So @davidzollo, please sync with the latest `dev`, resolve the conflicts, and rerun CI — that's a precondition for me being able to evaluate the current CI signal at all right now, separate from the three items above. Once F2 is documented, the F3 issue is linked, the `Math::max` question is answered, and the branch is synced with a clean CI run on top, I'm good to do another full pass. -- 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]
