DanielLeens commented on PR #11271: URL: https://github.com/apache/seatunnel/pull/11271#issuecomment-5385675957
Thanks @SEZ9 — this is a fair, well-evidenced catch, and I want to concede it directly rather than defend my own "Ready to merge" conclusion from earlier today. I re-checked the current head (`79db105ea259`) myself: `pruneRestoredIncrementalSplit()` (`IncrementalSourceReader.java:246-253`) does call `dataSourceDialect.discoverDataCollections(sourceConfig)` unguarded inside `addSplits()`, with no try/catch and no fallback if it throws or returns a partial result. Before this PR, `addSplits()` for a restored incremental split was a pure in-memory operation; this PR adds a live DB round trip directly onto the restore path. If that call throws on a transient DB issue, the recovery attempt itself fails (restart-loop risk on Zeta/Flink). If it returns a partial/empty set (replica lag, a momentary privilege issue), `pruneTables()` will silently drop `completedSnapshotSplitInfos`/`checkpointTables`/`historyTableChanges` for a table that was never actually removed from the job config, and the next checkpoint persists that loss irreversibly. Both failure modes are real and match what you described. That is a legitimate High-severity blocker my last review missed — I was focused on confirming the `tableWatermarks` exclusion revert and did not re-examine `discoverDataCollections()`'s error-handling contract on this now-hot restore path. I am retracting the "no blockers" framing from my prior review on this specific point; your Issue 1 is the blocking item here. Issues 2, 3, and 5 (the ad-hoc `TableId` reconstruction, discovery-vs-config as the source of truth, and the metadata-presence restore heuristic) are also worth the author's attention as solid non-blocking follow-ups in the same area. @hutiefang76 — once Issue 1 is addressed (wrapping the discovery call with a safe fallback, or deriving the captured set from the configured table list instead of live discovery, per SEZ9's suggestion), I will do a fresh full pass over the updated 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]
