fix: implement XA mode commitOnXA() and LockQuery() by CAICAIIs · Pull Request #1061 · apache/incubator-seata-go

CAICAIIs · 2026-03-10T06:32:43Z

Summary

Fix XA mode implementation - implements commitOnXA() and LockQuery() as reported in Issue #1060.

Problem

The XA mode in seata-go had two critical unimplemented methods:

commitOnXA() - Empty implementation returning nil, causing XA transactions to not execute properly
LockQuery() - Always returned (false, nil) without checking global locks
This caused XA mode to be unusable in production environments.

Solution

1. `pkg/datasource/sql/tx_xa.go`

Implements commitOnXA() to:

Check if global transaction is open
Execute XA END + XA PREPARE via xaConn.Commit()
Report branch status to TC (Phase One)

2. `pkg/datasource/sql/xa_resource_manager.go`

Fixes LockQuery() to delegate to rmRemoting.LockQuery() for actual global lock checking against TC

3. `pkg/datasource/sql/tx.go`

Adds xaConn field to Tx struct to support XA transaction coordination

4. `pkg/datasource/sql/conn_xa.go`

Sets xaConn reference in BeginTx() to enable transaction coordination

How It Works

Seata XA uses two-phase commit:
Phase 1 (Application):

XAConn.BeginTx() → XA START + register branch
Execute business SQL
XATx.Commit() → commitOnXA() → XA END + XA PREPARE + report to TC

Phase 2 (TC):
5. TC decides to commit → calls BranchCommit()
6. BranchCommit() → XA COMMIT (final step)
This PR implements Phase 1. Phase 2 was already implemented in XAResourceManager.BranchCommit().

Testing

Build passes: go build ./...
Vet passes: go vet ./pkg/datasource/sql/...
Tests pass: go test ./pkg/datasource/sql/...

Related Issues

Fixes [BUG] 项目部分 TODO 或 implement me 汇总 #1060

fix: implement XA mode commitOnXA() and LockQuery()

- tx_xa.go: implement commitOnXA() to execute XA END + XA PREPARE and report status to TC - xa_resource_manager.go: LockQuery() delegates to rmRemoting for global lock checking - tx.go: add xaConn field to support XA transactions - conn_xa.go: set xaConn reference in BeginTx

codecov-commenter · 2026-03-10T09:35:29Z

Codecov Report

❌ Patch coverage is 65.00000% with 28 lines in your changes missing coverage. Please review.
✅ Project coverage is 57.52%. Comparing base (39063bb) to head (3227b5f).

Files with missing lines	Patch %	Lines
pkg/datasource/sql/conn_xa.go	46.66%	13 Missing and 3 partials ⚠️
pkg/datasource/sql/tx_xa.go	70.00%	8 Missing and 4 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1061      +/-   ##
==========================================
+ Coverage   57.34%   57.52%   +0.17%     
==========================================
  Files         267      267              
  Lines       17597    17656      +59     
==========================================
+ Hits        10091    10156      +65     
+ Misses       6681     6672       -9     
- Partials      825      828       +3

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

thunguo · 2026-03-10T10:10:17Z

Do we need to add some test cases? It seems the unit test coverage has decreased somewhat.

This commit addresses PR apache#1061 feedback and Oracle architectural review findings: **Reviewer Feedback (thunguo):** - Extract anonymous xaConn interface into named XAConnection type for better readability **Code Quality Improvements:** - Add comprehensive logging to commitOnXA() following existing patterns (log.Infof/Errorf) - Add detailed documentation comments explaining XA two-phase commit protocol flow - Improve test coverage with 11 new unit tests for XATx functionality **Critical Bug Fixes:** 1. Fix BranchReport error handling - errors are now properly checked and logged 2. Complete XA Rollback lifecycle - now executes XA END(TMFAIL) + XA ROLLBACK + TC reporting 3. Fix SQL error reporting - createNewTxOnExecIfNeed() now reports PhaseoneFailed to TC 4. Fix nil pointer safety - set tx.target after real transaction is created in XAConn.BeginTx() 5. Update prepareTime after successful XA PREPARE for correct connection hold/timeout behavior **Files Changed:** - pkg/datasource/sql/tx.go: Extract XAConnection interface, add xaConn field - pkg/datasource/sql/tx_xa.go: Implement commitOnXA() with logging and error handling, complete Rollback() lifecycle - pkg/datasource/sql/conn_xa.go: Fix nil pointer issue, add SQL error reporting, update prepareTime - pkg/datasource/sql/tx_xa_test.go: Add comprehensive unit tests (11 tests covering all scenarios) **Test Results:** - All existing tests pass - 11 new tests added and passing - Build successful with no errors Co-authored-by: Oracle Review <architectural-review>

Copilot

Pull request overview

This PR fixes key gaps in Seata-Go XA mode by implementing phase-1 XA commit handling (commitOnXA()), wiring XA-capable connections into the transaction object, and delegating XA LockQuery() to the RM remoting layer so global lock checks are actually performed.

Changes:

Implement XA phase-1 commit (XA END + XA PREPARE) and reporting to TC; improve XA rollback flow and reporting.
Fix XA LockQuery() by delegating to rmRemoting.LockQuery.
Introduce XAConnection on Tx and wire XAConn into BeginTx, plus add/adjust tests for XA behavior.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
pkg/datasource/sql/xa_resource_manager.go	Makes XA `LockQuery()` actually query TC via `rmRemoting`.
pkg/datasource/sql/tx_xa.go	Implements XA phase-1 commit and improves rollback/reporting behavior.
pkg/datasource/sql/tx.go	Adds `XAConnection` plumbing and includes `BranchType` in branch reporting.
pkg/datasource/sql/conn_xa.go	Wires XAConn into Tx creation and adjusts auto-commit execution flow.
pkg/datasource/sql/tx_xa_test.go	Adds unit tests for `commitOnXA`, rollback/reporting, and option wiring.
pkg/datasource/sql/conn_xa_test.go	Adds test helpers and new tests covering auto-commit phase-1 reporting and rollback behavior.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

CAICAIIs · 2026-03-26T02:00:14Z

新更改：已修复 XA 分支生命周期：移除了 BeginTx 中错误的本地物理事务启动（代码先调用了数据库驱动的 Begin/BeginTx，这会在连接上发出普通事务），改为仅由 XA 流程驱动（XA 应该只走 XA 生命周期，不该先开一层普通事务，否则就会变成“两套事务状态混在一起”，容易出现状态不一致），并在 Rollback 缺失 xaConn 时显式报错以避免 RM/TC 状态不一致。

AlexStocks

[P0] conn_xa.go:54-64 xaBranchTx 空实现违反 XA 语义，Commit/Rollback 返回 nil 但不执行任何操作，可能导致数据不一致。建议返回明确错误或记录警告日志。

[P1] conn_xa.go:143 newTx 参数传递 branchTx 作为 originTx，branchTx 是空实现，后续调用 tx.target.Rollback() 会无操作。建议明确注释说明这是有意设计。

[P2] tx_xa.go:20-23 Import 顺序违反 Go 规范，标准库和第三方库应分组。

CAICAIIs · 2026-03-29T13:34:49Z

新更改：把 XA 的那个“占位事务”改成了误用就直接报错，避免表面成功、实际没执行的问题；顺手把 LockQuery 的测试补齐了，确保远程返回什么这里就按什么走。
另外还修了测试清理，防止测试里注册的全局 mock 残留到后面的用例。

thunguo

LGTM

github-actions Bot added bug Something isn't working enhancement New feature or request module/xa coding and removed bug Something isn't working enhancement New feature or request module/xa labels Mar 10, 2026

thunguo reviewed Mar 10, 2026

View reviewed changes

Comment thread pkg/datasource/sql/tx.go Outdated

github-actions Bot added bug Something isn't working enhancement New feature or request module/xa labels Mar 10, 2026

CAICAIIs force-pushed the fix/xa-mode-implementation branch from 158ed24 to e010686 Compare March 10, 2026 11:46

CAICAIIs force-pushed the fix/xa-mode-implementation branch from e010686 to 0aa178b Compare March 10, 2026 11:52

CAICAIIs and others added 5 commits March 10, 2026 19:59

Merge branch 'master' into fix/xa-mode-implementation

9b2776f

Merge branch 'master' into fix/xa-mode-implementation

1674dd2

fix: tighten xa branch reporting lifecycle

fb6f02b

Merge branch 'master' into fix/xa-mode-implementation

df06570

Merge branch 'master' into fix/xa-mode-implementation

2ce6043

thunguo requested a review from Copilot March 25, 2026 09:02

Copilot started reviewing on behalf of thunguo March 25, 2026 09:02 View session

Copilot AI reviewed Mar 25, 2026

View reviewed changes

Comment thread pkg/datasource/sql/conn_xa.go Outdated

Comment thread pkg/datasource/sql/tx_xa.go Outdated

Comment thread pkg/datasource/sql/tx_xa_test.go Outdated

Comment thread pkg/datasource/sql/tx_xa_test.go Outdated

Comment thread pkg/datasource/sql/conn_xa_test.go Outdated

fix: align xa branch tx lifecycle

b91c08d

AlexStocks reviewed Mar 26, 2026

View reviewed changes

Comment thread pkg/datasource/sql/xa_resource_manager.go

AlexStocks reviewed Mar 28, 2026

View reviewed changes

Comment thread pkg/datasource/sql/conn_xa.go

AlexStocks reviewed Mar 28, 2026

View reviewed changes

Comment thread pkg/datasource/sql/conn_xa.go

CAICAIIs added 2 commits March 28, 2026 14:08

Merge branch 'master' into fix/xa-mode-implementation

89ed886

test: tighten xa review follow-ups

c288409

github-actions Bot added the rm label Mar 29, 2026

Merge branch 'master' into fix/xa-mode-implementation

1ff764f

CAICAIIs and others added 5 commits April 4, 2026 20:42

Merge branch 'master' into fix/xa-mode-implementation

0b4be66

Merge branch 'master' into fix/xa-mode-implementation

11fdd43

Merge branch 'master' into fix/xa-mode-implementation

529425c

Merge branch 'master' into fix/xa-mode-implementation

f2c2131

Merge branch 'master' into fix/xa-mode-implementation

3227b5f

thunguo approved these changes Apr 22, 2026

View reviewed changes

thunguo merged commit 976e7b9 into apache:master Apr 22, 2026
12 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: implement XA mode commitOnXA() and LockQuery()#1061

fix: implement XA mode commitOnXA() and LockQuery()#1061
thunguo merged 16 commits intoapache:masterfrom
CAICAIIs:fix/xa-mode-implementation

CAICAIIs commented Mar 10, 2026 •

edited

Loading

Uh oh!

Uh oh!

codecov-commenter commented Mar 10, 2026 •

edited

Loading

Uh oh!

thunguo commented Mar 10, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

CAICAIIs commented Mar 26, 2026 •

edited

Loading

Uh oh!

Uh oh!

AlexStocks left a comment

Uh oh!

Uh oh!

Uh oh!

CAICAIIs commented Mar 29, 2026

Uh oh!

thunguo left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Conversation

CAICAIIs commented Mar 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Problem

Solution

1. pkg/datasource/sql/tx_xa.go

2. pkg/datasource/sql/xa_resource_manager.go

3. pkg/datasource/sql/tx.go

4. pkg/datasource/sql/conn_xa.go

How It Works

Testing

Related Issues

Uh oh!

Uh oh!

codecov-commenter commented Mar 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

thunguo commented Mar 10, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

CAICAIIs commented Mar 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

AlexStocks left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

CAICAIIs commented Mar 29, 2026

Uh oh!

thunguo left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

CAICAIIs commented Mar 10, 2026 •

edited

Loading

1. `pkg/datasource/sql/tx_xa.go`

2. `pkg/datasource/sql/xa_resource_manager.go`

3. `pkg/datasource/sql/tx.go`

4. `pkg/datasource/sql/conn_xa.go`

codecov-commenter commented Mar 10, 2026 •

edited

Loading

CAICAIIs commented Mar 26, 2026 •

edited

Loading