xinnyuli commented on issue #12624: URL: https://github.com/apache/seatunnel/issues/12624#issuecomment-5988483379
> Thanks for the detailed analysis and for the deterministic SQLite tests, that makes both paths easy to follow. > > The root cause in both cases looks the same: in manual-commit mode, everything flushed since the last commit lives in one open transaction, so losing or replacing the connection before commit silently discards earlier batches while the checkpoint still succeeds. > > A few thoughts on the fix direction: > > 1. Flush retry: when a flush fails with a connection or transaction-level error (08xxx/40xxx) in manual-commit mode, retrying only the latest batch is not safe. It seems better to fail the task so the engine restores from the last checkpoint, rather than reconnecting and re-sending only the current batch. > 2. Pooled connection replaced before commit: `commitIfNeeded()` should not transparently commit on a freshly swapped-in connection. If the connection that held the open transaction has been replaced, that should surface as an error so the checkpoint fails and the data is replayed. > > Since you mentioned you are willing to submit a PR, that would be very welcome. It would be great if the PR includes the two `JdbcMultiTableReconnectTest` cases you described so the fix is covered by tests. Please also note in the PR whether the Oracle path (which always forces manual commit) is handled, since Oracle users would hit this with default settings. Thanks @SEZ9! I've already opened #12625 for this, and it follows the same direction: Flush retry: in manual-commit mode, if earlier batches are still uncommitted, or the error is SQLState class 40 (the whole transaction was rolled back), the flush now fails the task instead of retrying. One small difference: a connection error (08xxx) on the first batch after a commit still retries, because there's nothing earlier to lose, so re-sending that batch is complete. Replaced connection: commitIfNeeded() now calls checkUncommittedBatchesOn(connection) first. If the connection that held the flushed batches has been replaced, it throws, so the checkpoint fails and the data is replayed. Both JdbcMultiTableReconnectTest cases are in the PR, plus unit tests in JdbcOutputFormatReconnectTest. Oracle: the check reads the connection's actual auto-commit state (getAutoCommit()), not the auto_commit option. Since resolveSinkAutoCommit() always returns false for Oracle, Oracle connections go through the same path. There's no Oracle-specific test though; the tests use SQLite in manual-commit mode. I'll add this note to the PR description too. -- 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]
