SEZ9 commented on PR #12555: URL: https://github.com/apache/seatunnel/pull/12555#issuecomment-5988323726
@ricky2129 thanks for the PR — the problem it targets (re-reading retained transactions of a lineage the checkpoint never tracked) is real and worth fixing. Here is where I'd like to land on the open points: **1. Previous_gtids looked up by binlog filename (HIGH, blocking).** In GTID mode the GTID set is the server-stable identifier; the filename only means something on the server that wrote it. After a failover to a promoted/different server that has a same-named file, `previousGtidSet` returns that server's own lineages, `untrackedGtids` keeps them (they are not in the checkpoint), and they end up in the set passed to `client.setGtidSet` — so transactions the job never consumed are marked done and skipped. I'd like the claimed lineages to be derived from something that is robust to a host change (or the merge to be gated so it cannot run against a server other than the one the checkpoint came from). Could you describe the approach you intend to take before pushing code? **2. `SHOW BINLOG EVENTS` query (MEDIUM).** Please validate the restored filename (e.g. against the expected binlog-name shape) rather than interpolating it directly, and avoid the hard-coded `LIMIT 3` — read until the `Previous_gtids` event is found or bail out explicitly. **3. Extra round-trip and WARN on every restart (LOW).** Please only issue the query when there is actually an untracked lineage to resolve, and downgrade/shorten the log for accounts lacking the privilege so a normal restart does not emit a full stack trace. **4. Tests (MEDIUM/LOW).** `GtidUtilsTest` currently re-implements the merge in a private `merge` helper, so a divergence from `filterGtidSet` in `MySqlStreamingChangeEventSource` would go unnoticed. Please exercise `filterGtidSet` directly, cover `previousGtidSet` / the `SHOW BINLOG EVENTS` parsing path, and extend the existing MySQL-CDC E2E suite with the restore scenario this PR changes (untracked lineage present on the server), plus a case for the host-change situation from point 1 once you have a fix. **5. Docs (LOW).** Since restore semantics for GTID-mode jobs change on upgrade (extra server query, different starting GTID set), please add a note to the MySQL-CDC docs and `incompatible-changes.md`. Happy to review again once point 1 has a concrete design and the tests are in place. <!-- streview-comment:1532 --> -- 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]
