DanielLeens commented on PR #12254:
URL: https://github.com/apache/seatunnel/pull/12254#issuecomment-5664492943
Response to the 2026-09-14 review, Issue 1 (undisclosed
`connector-cdc-postgres` checkpoint-commit change).
That change is not part of this PR. `LsnOffset#getLsnCommit()`, the
`LAST_COMMIT_LSN_KEY` write, the `commitCurrentOffset` switch to
`getLsnCommit()`, the `volatile lastCommitLsn`, the producer-throwable
surfacing in `PostgresWalFetchTask#execute()` and `LsnOffsetTest` all come from
#11864 ("Fix Postgres-CDC committed-offset recovery skipping rows after
savepoint"), merged to `dev` on 2026-09-11 as `5af8d789`. Evidence:
- `git log -S getLsnCommit` on this branch resolves to a single commit,
`5af8d789` by Arvin, which is the #11864 merge commit on `dev`; none of this
PR's own commits (`dff9bc6d`, `b21de7cf`, or any other) touch `LsnOffset.java`
or `PostgresWalFetchTask.java`.
- Those changes reached this branch only through `8b79be19`, the "Merge
branch 'dev' into gaussdb-cdc-connector" commit pushed this morning; the
merge-base with `dev` is now `35b2716c`, which already contains #11864.
- GitHub's Files changed for this PR (three-dot diff against that
merge-base) lists exactly one file under `connector-cdc-postgres`:
`PostgresSourceFetchTaskContext.java`, which is the additive constructor
described in the PR description's Compatibility section and in the 2026-09-12
comment. The existing constructor delegates to it with the same value converter
builder as before.
So the Compatibility section is accurate for this PR's diff, and the
regression-test and `incompatible-changes.md` questions about the commit-LSN
semantics belong to #11864, not here. No code change is needed for this
finding. Two smaller corrections to the review's context: the PR has been ready
for review since 2026-09-13, and the previously flagged three Medium findings
are confirmed fixed as the review itself notes.
CI on `8b79be19` (fork run 34797859522) has been queued since 02:02 UTC
behind other runs on the fork and has not started; the last completed run on
the pre-merge head `b9158912` passed every job this PR owns, with only the
MinIO image failures (now fixed on `dev` by #12287, which this merge brings in)
and one unrelated Fluss hang.
--
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]
