You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@seata.apache.org by "lhozy (via GitHub)" <gi...@apache.org> on 2026/06/17 11:07:53 UTC
[PR] bugfix: fix global lock batch acquire false-failure on Dameng(DM) [incubator-seata]
lhozy opened a new pull request, #8145:
URL: https://github.com/apache/incubator-seata/pull/8145
# PR: bugfix: fix global lock batch acquire false-failure on Dameng(DM)
## Ⅰ. Describe what this PR does
Fix `LockStoreDataBaseDAO#doAcquireLocks` so that batch global-lock acquisition no longer produces a **false "lock acquire failed"** on databases (e.g. **Dameng / DM**) whose JDBC driver returns an `executeBatch()` array whose length differs from the number of batched statements.
Fixes #8144.
## Ⅱ. Why make this change
`doAcquireLocks` judged batch success with:
```java
return ps.executeBatch().length == lockDOs.size();
```
The JDBC spec does **not** guarantee `executeBatch()` returns exactly one element per batched statement (elements may be counts, `SUCCESS_NO_INFO`, or `EXECUTE_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 == size` check returns `false`, so the caller rolls back the inserted locks and the branch register is reported as a lock conflict. Single-row branches (non-batch `doAcquireLock`) 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.java`
```diff
for (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
- Existing behavior preserved on databases that return one element per statement (MySQL, Oracle, PostgreSQL…): no `EXECUTE_FAILED` → returns `true`; genuine duplicate-`row_key` conflicts still throw `SQLIntegrityConstraintViolationException` → handled → `false`.
- On DM8 (store.mode=db): a branch acquiring multiple row locks now succeeds; a cross-resource global transaction with multi-row branches both commits and rolls back correctly, where before every multi-row branch register failed with `Global lock batch acquire failed`.
## Ⅴ. Checklist
- [ ] Add/keep unit tests (consider a test asserting `doAcquireLocks` returns true when `executeBatch()` returns an aggregated/short array).
- [ ] Changelog entry.
--
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: notifications-unsubscribe@seata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For additional commands, e-mail: notifications-help@seata.apache.org
Re: [PR] bugfix: fix global lock batch acquire false-failure on Dameng(DM) [incubator-seata]
Posted by "funky-eyes (via GitHub)" <gi...@apache.org>.
funky-eyes merged PR #8145:
URL: https://github.com/apache/incubator-seata/pull/8145
--
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: notifications-unsubscribe@seata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For additional commands, e-mail: notifications-help@seata.apache.org
Re: [PR] bugfix: fix global lock batch acquire false-failure on Dameng(DM) [incubator-seata]
Posted by "funky-eyes (via GitHub)" <gi...@apache.org>.
funky-eyes commented on PR #8145:
URL: https://github.com/apache/incubator-seata/pull/8145#issuecomment-4766588693
If you're using the DingTalk app, please send your DingTalk account to my email at [jianbin@apache.org](mailto:jianbin@apache.org), and I will invite you to join the Seata developer group
--
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: notifications-unsubscribe@seata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For additional commands, e-mail: notifications-help@seata.apache.org
Re: [PR] bugfix: fix global lock batch acquire false-failure on Dameng(DM) [incubator-seata]
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #8145:
URL: https://github.com/apache/incubator-seata/pull/8145#issuecomment-4767034452
## [Codecov](https://app.codecov.io/gh/apache/incubator-seata/pull/8145?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:white_check_mark: All modified and coverable lines are covered by tests.
:white_check_mark: Project coverage is 72.72%. Comparing base ([`bba87e7`](https://app.codecov.io/gh/apache/incubator-seata/commit/bba87e7cbe69779ebb07cfbab0a354d645f8b7b0?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`6f2722a`](https://app.codecov.io/gh/apache/incubator-seata/commit/6f2722a0f9fa33ab8b47229454afb421020e52b1?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
:warning: Report is 1 commits behind head on 2.x.
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## 2.x #8145 +/- ##
=========================================
Coverage 72.71% 72.72%
- Complexity 879 880 +1
=========================================
Files 1327 1327
Lines 50848 50852 +4
Branches 6070 6071 +1
=========================================
+ Hits 36976 36980 +4
- Misses 10877 10882 +5
+ Partials 2995 2990 -5
```
| [Files with missing lines](https://app.codecov.io/gh/apache/incubator-seata/pull/8145?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [...a/server/storage/db/lock/LockStoreDataBaseDAO.java](https://app.codecov.io/gh/apache/incubator-seata/pull/8145?src=pr&el=tree&filepath=server%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fseata%2Fserver%2Fstorage%2Fdb%2Flock%2FLockStoreDataBaseDAO.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c2VydmVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9zZWF0YS9zZXJ2ZXIvc3RvcmFnZS9kYi9sb2NrL0xvY2tTdG9yZURhdGFCYXNlREFPLmphdmE=) | `55.31% <100.00%> (+1.20%)` | :arrow_up: |
... and [14 files with indirect coverage changes](https://app.codecov.io/gh/apache/incubator-seata/pull/8145/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
[](https://app.codecov.io/gh/apache/incubator-seata/pull/8145?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
</details>
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
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: notifications-unsubscribe@seata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For additional commands, e-mail: notifications-help@seata.apache.org
Re: [PR] bugfix: fix global lock batch acquire false-failure on Dameng(DM) [incubator-seata]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #8145:
URL: https://github.com/apache/incubator-seata/pull/8145#discussion_r3440418126
##########
server/src/main/java/org/apache/seata/server/storage/db/lock/LockStoreDataBaseDAO.java:
##########
@@ -390,7 +390,18 @@ protected boolean doAcquireLocks(Connection conn, List<LockDO> lockDOs) throws S
ps.setInt(8, lockDO.getStatus());
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;
+ }
+ }
Review Comment:
`executeBatch()` can legally return update counts of `0` for statements that affected no rows. Since this batch is inserting lock rows, treating `0` as success could allow the method to return `true` even if some lock rows were not inserted (i.e., the lock was not actually acquired). Align this with `doAcquireLock()` which checks `executeUpdate() > 0` by also treating `0` as a failure result (in addition to `EXECUTE_FAILED`).
##########
server/src/main/java/org/apache/seata/server/storage/db/lock/LockStoreDataBaseDAO.java:
##########
@@ -390,7 +390,18 @@ protected boolean doAcquireLocks(Connection conn, List<LockDO> lockDOs) throws S
ps.setInt(8, lockDO.getStatus());
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();
Review Comment:
This change fixes a driver-compatibility bug, but existing lock-store tests use H2 and won't cover the aggregated/short `executeBatch()` result scenario (e.g., DM). Consider adding a unit test (e.g., with a mocked `PreparedStatement`/`Connection` or a small test subclass) that verifies `doAcquireLocks()` returns `true` when `executeBatch()` returns a shorter array with no `EXECUTE_FAILED` values.
--
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: notifications-unsubscribe@seata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For additional commands, e-mail: notifications-help@seata.apache.org