SEZ9 commented on PR #11569: URL: https://github.com/apache/seatunnel/pull/11569#issuecomment-5565034464
Thanks @DanielLeens for tracing the `b56f423a1` build — agreed, the `FlussSinkIT` timeout is unrelated to this diff, which only touches `connector-jdbc`'s XA path and `XaGroupOpsImplIT`. Retriggering just that job sounds right. While CI reruns, the earlier review points are still open. Could you please address (or push back on) each of these: 1. **Empty recovery scan in `restoreCommit()`**: an all-absent XA RECOVER result is currently treated as "already resolved", so a truncated or incomplete scan would silently drop prepared data. Please distinguish "scan returned nothing" from "each xid was positively confirmed resolved", or fail loudly in the former case. 2. **Re-enabled commit failure propagation**: the new reconciliation only lives in `restoreCommit()`, but engines such as Flink/Spark replay restored commit infos through `commit()`. Please either apply the same reconciliation on the `commit()` path or explain how restore works on those engines. 3. **Prefix-skip logic**: "absent from XA RECOVER" is equated with "committed", so an externally rolled-back transaction is accepted as success. Please clarify how a rollback is detected, or document the limitation. 4. **Fixed 1s backoff × retry budget on the committer thread**: this can block the checkpoint/commit thread long enough to trip checkpoint timeouts, the recovery scan is re-run per batch, and the synchronous sleep needs explicit interruption handling. Please make the backoff configurable/bounded, reuse the scan across a batch where possible, and handle interruption properly. 5. **`TRANSIENT_ERR_CODES` reclassification** (XA_RBTRANSIENT → permanent, XA_RETRY → retryable) plus the unwrapped `TransientXaException`: this applies to every XA verb going through the facade, not just commit, and changes the connector error-code contract on surfaced failures. Please scope it to commit or justify the global change, and keep surfaced failures wrapped in the connector exception type. 6. **Docs**: since the error-code reclassification is user-facing, please add an entry to the incompatible-changes docs. Once those are addressed and the retriggered job finishes, I'll take another pass. <!-- streview-comment:872 --> -- 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]
