SEZ9 commented on PR #12567: URL: https://github.com/apache/seatunnel/pull/12567#issuecomment-6159877519
@Asthenia0412 thanks for tracing the restart-position problem so carefully — your reading is right, and it is exactly the concern behind PR12567-F1: the fallback watermark is the split-level minimum, not each restored table's own checkpoint offset, so with incremental parallelism > 1 a restored table can re-emit already-delivered events under exactly_once. That is the one piece still open on the logic side. To answer your question: please sketch it right here on this PR. Also, head `c490b75` differs from `e18f31c` only by an empty "retrigger CI" commit, so there is no new code to re-review yet; the three previous findings stand as written. The smallest shape I would try: 1. `IncrementalSplitAssigner#addSplits` (`:184-199`): stop reading `incrementalSplit.getTableStartupOffsets()`; restore `tableWatermarks.put(tableId, startupOffset)` as on dev, so every restored table keeps the newest position its old split delivered, while the new split still starts at the minimum across splits. 2. `IncrementalSplitAssigner#createIncrementalSplit` (`:289-324`): keep what you have — ship the captured tables' `tableWatermarks` as `tableStartupOffsets`. That map is the per-table boundary the fetcher needs. 3. `IncrementalSourceStreamFetcher#configureFilter` (`:338-359`): keep as is; it already seeds the exactly-once filter from `tableStartupOffsets`. 4. Drop the reader-side tracking (the emitter change and the per-table map in `IncrementalSplitState`); keep only the monotonic guard on the split-level offset. 5. Add one assigner-level test: two restored splits, one with an older startup offset and one newer, a table in each. Assert the new split starts at the older offset (not earlier) and that each table's `tableStartupOffsets` entry is its own split's offset. Remaining asks on the test findings: - PR12567-F2: you mentioned a dedicated restore-regression IT — please confirm (or adjust) that it runs with exactly_once enabled and mutates the pre-existing table after restore; the existing restore-with-new-table e2e does neither, so it does not exercise the fixed path. - PR12567-F3: drive the fetcher through its public `submitTask` entry point or a test-visible hook rather than reaching into private state via reflection. Once the fix is pushed I will re-review the full head in one pass. <!-- streview-comment:1739 --> -- 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]
