Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

bugfix: modify XA mode pre commit transaction from commit phase to before close phase #7102

Merged
merged 20 commits into from
Feb 15, 2025
Merged

bugfix: modify XA mode pre commit transaction from commit phase to before close phase #7102

merged 20 commits into from
Feb 15, 2025

Conversation

xiaoxiangyeyu0
Copy link
Contributor

@xiaoxiangyeyu0 xiaoxiangyeyu0 commented Jan 11, 2025 •
edited by funky-eyes
Loading

  • I have registered the PR changes.

Ⅰ. Describe what this PR did

Modify XA mode pre commit transaction from commit phase to before close phase

Ⅱ. Does this pull request fix one issue?

fixes #7100

Ⅲ. Why don't you add test cases (unit test/integration test)?

Ⅳ. Describe how to verify it

link #7100

Ⅴ. Special notes for reviews

@codecov Codecov
Copy link

codecov bot commented Jan 12, 2025 •
edited
Loading

Codecov Report

Attention: Patch coverage is 12.50000% with 21 lines in your changes missing coverage. Please review.

Project coverage is 52.20%. Comparing base (6b25b21) to head (02fc25e).
Report is 1 commits behind head on 2.x.

Files with missing lines Patch % Lines
...ache/seata/rm/datasource/xa/ConnectionProxyXA.java 12.50% 18 Missing and 3 partials ⚠️
Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff              @@
##                2.x    #7102      +/-   ##
============================================
- Coverage     52.24%   52.20%   -0.04%     
+ Complexity     6814     6812       -2     
============================================
  Files          1154     1154              
  Lines         41057    41058       +1     
  Branches       4811     4813       +2     
============================================
- Hits          21451    21436      -15     
- Misses        17569    17586      +17     
+ Partials       2037     2036       -1
Files with missing lines Coverage Δ
...ache/seata/rm/datasource/xa/ConnectionProxyXA.java 50.27% <12.50%> (-7.51%) ⬇️

... and 3 files with indirect coverage changes

@xingfudeshi
Copy link
Member

I suggest making the title more specific to better describe the bug that has been fixed.

@xiaoxiangyeyu0
Copy link
Contributor Author

Description added

@xingfudeshi xingfudeshi changed the title Fix bug in https://github.com/apache/incubator-seata/issues/7100 bugfix:modify XA mode pre commit transaction from commit phase to before close phase Jan 13, 2025
@funky-eyes funky-eyes changed the title bugfix:modify XA mode pre commit transaction from commit phase to before close phase bugfix: modify XA mode pre commit transaction from commit phase to before close phase Jan 13, 2025
@funky-eyes funky-eyes added type: bug Category issues or prs related to bug. mode: XA XA transaction mode module/rm-datasource rm-datasource module labels Jan 13, 2025
@funky-eyes funky-eyes added this to the 2.4.0 milestone Jan 13, 2025
funky-eyes
funky-eyes reviewed Jan 15, 2025
View reviewed changes
rm-datasource/src/main/java/org/apache/seata/rm/datasource/xa/ConnectionProxyXA.java Outdated
@@ -117,6 +117,10 @@ private void xaEnd(XAXid xaXid, int flags) throws XAException {
if (!xaEnded) {
xaResource.end(xaXid, flags);
xaEnded = true;
} else {
if (flags == XAResource.TMSUCCESS) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

为什么只有TMSUCCESS才需要end？join进来的不再end了吗？
假设事务1 commit，事务2继续用这个connection commit（等于先join）走到这个xaend，就不执行xaend了？还有rollback的时候会走到这个xaend方法，此时rollback不直接end，直接进行rollback吗？

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

xaEnded为false的时候会xaResource.end，这里flags == XAResource.TMSUCCESS是为了事务2继续用这个connection commit时能够xaResource.end，因为第一次commit把xaEnded置为true了

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@xiaoxiangyeyu0 我看到只在close的时候至为false

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

xaEnded默认是false，第一次调用xaEnd方法会把xaEnded置为true

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

connection没有close时直接复用，xaended就是true了，如何再次end？ @xiaoxiangyeyu0

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

传入的flags值为XAResource.TMSUCCESS，走else的分支会执行end

@funky-eyes funky-eyes requested a review from slievrly January 15, 2025 15:05
funky-eyes
funky-eyes approved these changes Feb 14, 2025
View reviewed changes
Copy link
Contributor

@funky-eyes funky-eyes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@xiaoxiangyeyu0
Copy link
Contributor Author

pr已登记

@funky-eyes funky-eyes merged commit 56a2f14 into apache:2.x Feb 15, 2025
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@funky-eyes funky-eyes funky-eyes approved these changes

@slievrly slievrly Awaiting requested review from slievrly

Assignees
No one assigned
Labels
mode: XA XA transaction mode module/rm-datasource rm-datasource module type: bug Category issues or prs related to bug.
Projects
None yet
Milestone
2.4.0
Development

Successfully merging this pull request may close these issues.

同一个PreparedStatement执行多次execute会报ShouldNeverHappenException
3 participants
@xiaoxiangyeyu0 @xingfudeshi @funky-eyes