bugfix: fix global lock batch acquire false-failure on Dameng(DM)#8145
Open
lhozy wants to merge 1 commit into
Open
bugfix: fix global lock batch acquire false-failure on Dameng(DM)#8145lhozy wants to merge 1 commit into
lhozy wants to merge 1 commit into
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes a JDBC-driver compatibility issue in Seata Server’s DB lock store where batch global-lock acquisition could incorrectly report failure on databases (e.g., Dameng/DM) whose executeBatch() result array length doesn’t match the number of batched statements, even when inserts succeeded.
Changes:
- Replace
executeBatch().length == lockDOs.size()with logic that detects batch failure viaStatement.EXECUTE_FAILED. - Add explanatory comments documenting the JDBC-spec nuance and DM driver behavior.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+399
to
+403
| for (int updated : result) { | ||
| if (updated == java.sql.Statement.EXECUTE_FAILED) { | ||
| return false; | ||
| } | ||
| } |
| // aggregate the per-statement results, returning an array of a different length. | ||
| // Detect failure via EXECUTE_FAILED instead; real conflicts (duplicate row_key) still | ||
| // throw SQLIntegrityConstraintViolationException and are handled by the catch block below. | ||
| int[] result = ps.executeBatch(); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PR: bugfix: fix global lock batch acquire false-failure on Dameng(DM)
Ⅰ. Describe what this PR does
Fix
LockStoreDataBaseDAO#doAcquireLocksso that batch global-lock acquisition no longer produces a false "lock acquire failed" on databases (e.g. Dameng / DM) whose JDBC driver returns anexecuteBatch()array whose length differs from the number of batched statements.Fixes #8144.
Ⅱ. Why make this change
doAcquireLocksjudged batch success with:The JDBC spec does not guarantee
executeBatch()returns exactly one element per batched statement (elements may be counts,SUCCESS_NO_INFO, orEXECUTE_FAILED, and some drivers aggregate). The DM driver aggregates the per-row results (DmdbPreparedStatement.executeBatchByRow→ExecuteRetInfo.union), so the returned array length ≠ statement count.Consequence on DM: when a branch locks ≥ 2 rows, the lock rows are inserted successfully but the
length == sizecheck returnsfalse, so the caller rolls back the inserted locks and the branch register is reported as a lock conflict. Single-row branches (non-batchdoAcquireLock) are unaffected, so the bug only manifests for multi-row branches — making AT mode unusable on DM for typical multi-row business transactions.Ⅲ. The change
server/src/main/java/org/apache/seata/server/storage/db/lock/LockStoreDataBaseDAO.javafor (LockDO lockDO : lockDOs) { ... ps.addBatch(); } - return ps.executeBatch().length == lockDOs.size(); + // Do not rely on executeBatch().length == size: per the JDBC spec the + // length is not guaranteed to equal the number of statements, and some + // drivers (e.g. Dameng/DM) aggregate the per-statement results, returning + // an array of a different length. Detect failure via EXECUTE_FAILED instead; + // real conflicts (duplicate row_key) still throw SQLIntegrityConstraintViolationException + // and are handled by the catch block below. + int[] result = ps.executeBatch(); + for (int updated : result) { + if (updated == java.sql.Statement.EXECUTE_FAILED) { + return false; + } + } + return true; } catch (SQLIntegrityConstraintViolationException e) { LOGGER.error("Global lock batch acquire error: {}", e.getMessage(), e); // return false,let the caller go to conn.rollback() return false;Ⅳ. How tested
EXECUTE_FAILED→ returnstrue; genuine duplicate-row_keyconflicts still throwSQLIntegrityConstraintViolationException→ handled →false.Global lock batch acquire failed.Ⅴ. Checklist
doAcquireLocksreturns true whenexecuteBatch()returns an aggregated/short array).