DanielLeens commented on PR #12014:
URL: https://github.com/apache/seatunnel/pull/12014#issuecomment-5805501305
Thanks for continuing the trace, SEZ9. First, a live-status note: the PR has
moved again since we were discussing `b9f6da93` - the current head is now
`a36c6f440068` ("Merge branch 'dev' into feature-8259-mysql-cdc-truncate",
pushed 2026-09-23T01:31:36Z). I diffed `b9f6da93..a36c6f440068` directly: 85
files, and none of them are this PR's own classes or the
`JdbcOutputFormat.java` / `AbstractJdbcSinkWriter.java` pair we were just
discussing - it is again entirely unrelated `dev` content (cdc-oceanbase,
fluss, mqtt, redis, rocketmq, tablestore, engine health-monitor, imap-storage,
transforms, translation-flink). So the trace below, done against the actual
current source, still applies unchanged at `a36c6f440068`.
Here is the completed trace you asked for:
`applyTableOperation()` (`AbstractJdbcSinkWriter.java:88-98`) calls
`this.prepareCommit()` at line 89 **before** it ever reaches
`dialect.applyTableOperation(...)` (the TRUNCATE call) at line 93.
`prepareCommit()` resolves to `JdbcSinkWriter.prepareCommitInternal()`. On both
branches of that method - the row-error-collector path
(`JdbcSinkWriter.java:384-401`) and the normal path (`:404-422`) -
`outputFormat.checkFlushException()` is called (lines 387 and 406 respectively)
**before** `outputFormat.flush()` (lines 390 and 407). `checkFlushException()`
itself (`JdbcOutputFormat.java:114-121`) is untouched by the new
`commitOnFlush`/latch machinery: if `flushException != null` it unconditionally
throws a `JdbcConnectorException` right there, regardless of what the new
`flush()` short-circuit (lines 183-191) would otherwise do. So a stale flush
failure is surfaced and thrown out of `prepareCommit()` at the
`checkFlushException()` call, meaning `applyTableOperation()` never reaches lin
e 93 at all - the TRUNCATE statement is never issued once a prior flush has
failed. This holds on every path through `prepareCommitInternal()`, so the F1
atomicity invariant is intact after this merge too.
On your second point - TRUNCATE being DDL with an implicit commit, potential
FK failures, and undefined behavior in exactly-once/XA mode - that's
independent of the latch and was already covered in my original review: XA
exactly-once with TRUNCATE is explicitly rejected at config-validation time by
`1a4f2d32d6fe` ("Reject JDBC XA exactly-once for TRUNCATE table-operations"),
and the FK/implicit-commit caveats are the connector's documented limitation,
not new territory opened by this merge. I don't see this needing a code change
beyond what's already there.
For F2-F8: since neither this new merge nor the previous one
(`913cc45e..b9f6da93`) touches any file this PR owns, and the patch-id for the
PR's own diff is unchanged from my 09-15 full review, those items remain in the
state that review recorded - all resolved with the evidence cited there. Happy
to point to specific line numbers again for any of them if useful.
--
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]
