nzw921rx commented on PR #13006: URL: https://github.com/apache/seatunnel/pull/13006#issuecomment-6461141095
Thanks for the focused fix. I reviewed head `3904efc0bbce71f087f861bf6afebd0482b4d92e` and ran local validation after merging `dev` at `e649b2a319c9234ecf8a47bae219f7e65e2c26c2`. I found no blocking source-level issue within the reviewed scope. This is worth prioritizing for 3.1.0 because it fixes silent loss of existing-table changes during a savepoint → add tables → restore workflow. ### Implementation The change keeps the existing split restart-position selection and preserves each restored table's own boundary separately. I checked that these boundaries survive split reconstruction, reader state, checkpoint serialization, metadata copying, and table pruning. Tables without snapshot information receive an appropriate starting boundary, while tables still carrying snapshot information respect both their restored position and snapshot watermark. ### Local validation The real-database tests used MySQL 8.0.43 and actual MySQL-CDC → JDBC jobs under Zeta. The node and client ran in one JVM. Source parallelism was 2 on JDK 17 and 1 on JDK 11; `incremental.parallelism` was 1. For INSERT scenarios, an additional append-only target without a primary key checked that upsert behavior did not hide duplicate emissions. | Scenario | Result | |---|---| | Dev control: restore with a newly added table | Reproduced silent loss: the job remained RUNNING and checkpoints advanced, but existing tables had only 60 target rows instead of 90; the new table's 20 rows arrived normally | | PR: add a third table, then a fourth table, followed by another restore without adding tables | Three restores passed on JDK 11 and 17; source, upsert target, and append-only target matched exactly at 168 rows | | UPDATE, DELETE, and INSERT before and after restores | Passed on both JDKs; final 164 rows and field values matched | | `exactly_once=false` | Dev and PR controls passed the same three-restore workflow; final upsert contents matched | | CDC Base unit tests | 131 passed on each JDK, with no failures, errors, or skips | | MySQL CDC unit tests | 53 passed on each JDK, with no failures, errors, or skips | | Other CDC offset unit tests | 24 passed on JDK 17 | | Supplemental boundary checks using six actual offset implementations | 143 assertions per implementation passed | | Dev/PR split serialization compatibility | Both directions passed; the PR handles the absent field in old state, and the old class ignores the new field | The supplemental checks covered MySQL, PostgreSQL, Oracle, SQL Server, DB2, and MongoDB offset implementations, including: - Regrouping tables restored at different positions. - Suppressing replay before a table's own boundary. - Preserving pending boundaries through another checkpoint, serialization, reassignment, and restore. - Keeping records at the inclusive boundary eligible, including consecutive records sharing an offset. - Removing consumed boundaries and pruning removed tables. Reactor builds, scoped Spotless checks, and Maven verification also passed. Tests were run separately from skip-tests verification. ### Scope Other databases were validated through compilation, offset tests, and controlled component tests, not live database recovery tests. The existing factory rejects restore state containing multiple incremental splits. Regrouping was therefore verified at component level, not claimed as a multi-incremental-split recovery E2E. Serialization checks cover the specified dev/PR class layouts, rather than every historical release. ### CI The current Build check is still failing. The only failed job reported in the fork run is `Run / unit-test (17, ubuntu-latest)`. I could not retrieve its complete failure log, so I cannot attribute that failure or dismiss it as unrelated. I support the implementation based on source review and local validation. Please resolve and rerun the failing CI before merging. -- 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]
