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.

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

bugfix: fix TC retry rollback wrongly, after the XA transaction fail and rollback #5833

Merged
merged 9 commits into from
Oct 8, 2023

Conversation

capthua
Copy link
Contributor

@capthua capthua commented Sep 8, 2023

  • I have registered the PR changes.

Ⅰ. Describe what this PR did

修复bug #5832 。当XA分支事务回滚后,报告给TC,避免TC不知道重试回滚

Ⅱ. Does this pull request fix one issue?

fixes #5832

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

Ⅳ. Describe how to verify it

Ⅴ. Special notes for reviews

@codecov
Copy link

codecov bot commented Sep 8, 2023

Codecov Report

Merging #5833 (34eb2db) into develop (351b108) will increase coverage by 0.07%.
The diff coverage is 50.00%.

Additional details and impacted files

Impacted file tree graph

@@              Coverage Diff              @@
##             develop    #5833      +/-   ##
=============================================
+ Coverage      48.80%   48.88%   +0.07%     
- Complexity      4165     4172       +7     
=============================================
  Files            793      793              
  Lines          28003    28003              
  Branches        3417     3417              
=============================================
+ Hits           13668    13690      +22     
+ Misses         12911    12889      -22     
  Partials        1424     1424              
Files Coverage Δ
...a/io/seata/rm/datasource/xa/ConnectionProxyXA.java 56.49% <50.00%> (+1.94%) ⬆️

... and 6 files with indirect coverage 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 补充下changlog

@funky-eyes funky-eyes added this to the 1.8.0 milestone Sep 14, 2023
@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 Sep 14, 2023
@capthua
Copy link
Contributor Author

capthua commented Sep 15, 2023

LGTM 补充下changlog

已补充

DefaultResourceManager.get().branchReport(BranchType.XA, xid, xaBranchXid.getBranchId(),
status, null);
} catch (TransactionException te) {
LOGGER.warn("Failed to report XA branch " + status + " on " + xid + "-" + xaBranchXid.getBranchId()
Copy link
Member

@jsbxyyx jsbxyyx Sep 25, 2023

Choose a reason for hiding this comment

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

use log placeholder {}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

use log placeholder {}

already modified

Copy link
Member

@jsbxyyx jsbxyyx left a comment

Choose a reason for hiding this comment

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

LGTM

@funky-eyes funky-eyes merged commit 866ef33 into apache:develop Oct 8, 2023
7 checks passed
Copy link
Member

@slievrly slievrly left a comment

Choose a reason for hiding this comment

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

Is it reasonable to evaluate PhaseTwo_CommitFailed_XAER_NOTA_Retryable and PhaseTwo_RollbackFailed_XAER_NOTA_Retryable in other ways? Whether it is possible to change the status to NoRetryable.

@capthua
Copy link
Contributor Author

capthua commented Oct 8, 2023

Is it reasonable to evaluate PhaseTwo_CommitFailed_XAER_NOTA_Retryable and PhaseTwo_RollbackFailed_XAER_NOTA_Retryable in other ways? Whether it is possible to change the status to NoRetryable.

There are other situations that can also cause XAException, so RM needs to return Retryable to TC.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
mode: XA XA transaction mode module/rm-datasource rm-datasource module type: bug Category issues or prs related to bug.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants