Skip to content

bugfix: fix when TC returns RollbackFailed FailureHandler not executed#5321

Merged
slievrly merged 7 commits intoapache:developfrom
ZhangShiYeChina:develop
Feb 17, 2023
Merged

bugfix: fix when TC returns RollbackFailed FailureHandler not executed#5321
slievrly merged 7 commits intoapache:developfrom
ZhangShiYeChina:develop

Conversation

@ZhangShiYeChina
Copy link
Contributor

  • I have registered the PR changes.

Ⅰ. Describe what this PR did

fixes :#5231

Ⅱ. Does this pull request fix one issue?

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

Ⅳ. Describe how to verify it

Ⅴ. Special notes for reviews

# Conflicts:
#	tm/src/main/java/io/seata/tm/api/TransactionalTemplate.java
@funky-eyes funky-eyes added type: bug Category issues or prs related to bug. module/tm tm module labels Feb 10, 2023
@funky-eyes funky-eyes added this to the 1.7.0 milestone Feb 10, 2023
case TimeoutRollbacking:
case TimeoutRollbackRetrying:
case TimeoutRollbacked:
case RollbackRetryTimeout:
Copy link
Contributor

Choose a reason for hiding this comment

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

RollbackRetryTimeout 应该放在RollbackFailure那块,因为他是一个失败状态,且不会进行重试了

Copy link
Contributor Author

@ZhangShiYeChina ZhangShiYeChina Feb 10, 2023

Choose a reason for hiding this comment

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

好的,是不是按照下图这样修改
image

@codecov-commenter
Copy link

codecov-commenter commented Feb 10, 2023

Codecov Report

Merging #5321 (8f211fe) into develop (b4f6fb7) will increase coverage by 0.03%.
The diff coverage is 22.22%.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

Impacted file tree graph

@@              Coverage Diff              @@
##             develop    #5321      +/-   ##
=============================================
+ Coverage      48.76%   48.79%   +0.03%     
- Complexity      4173     4176       +3     
=============================================
  Files            743      743              
  Lines          26626    26634       +8     
  Branches        3327     3327              
=============================================
+ Hits           12984    12996      +12     
+ Misses         12241    12235       -6     
- Partials        1401     1403       +2     
Impacted Files Coverage Δ
...in/java/io/seata/tm/api/TransactionalTemplate.java 58.90% <22.22%> (-1.97%) ⬇️
...erver/storage/file/session/FileSessionManager.java 48.40% <0.00%> (+0.63%) ⬆️
.../java/io/seata/server/coordinator/DefaultCore.java 50.58% <0.00%> (+2.35%) ⬆️
...o/seata/server/session/AbstractSessionManager.java 64.17% <0.00%> (+2.98%) ⬆️
...n/src/main/java/io/seata/common/util/IdWorker.java 83.33% <0.00%> (+6.25%) ⬆️

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

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.

LGTM

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.

onTimeoutRollback has a design flaw, so change it after merged.

@slievrly slievrly changed the title bugfix: When the rollback logic on the TC side returns RollbackFailed, the custom FailureHandler is not executed bugfix: fix when TC returns RollbackFailed FailureHandler not executed Feb 17, 2023
@slievrly slievrly merged commit 97bad4b into apache:develop Feb 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

module/tm tm 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