Bugfix: Failure to Enter TimeoutRollbackRetrying State #7480
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## 2.x #7480 +/- ##
============================================
+ Coverage 60.49% 60.73% +0.24%
- Complexity 656 658 +2
============================================
Files 1298 1298
Lines 49081 49079 -2
Branches 5771 5769 -2
============================================
+ Hits 29691 29809 +118
+ Misses 16757 16631 -126
- Partials 2633 2639 +6
... and 16 files with indirect coverage changes 🚀 New features to boost your workflow:
|
| ex.getMessage() | ||
| }); | ||
| if (!retrying) { | ||
| if (shouldQueueToRetryRollback(retrying, globalSession)) { |
There was a problem hiding this comment.
Why not directly remove this retrying check? Then handle the transaction state transition in the queueToRetryRollback method instead?
There was a problem hiding this comment.
Why not directly remove this retrying check? Then handle the transaction state transition in the queueToRetryRollback method instead?
Does it mean moving the logic of shouldQueueToRetryRollback(retrying, globalSession) into globalSession.queueToRetryRollback()?
There was a problem hiding this comment.
Why not directly remove this retrying check? Then handle the transaction state transition in the queueToRetryRollback method instead?
Does it mean moving the logic of shouldQueueToRetryRollback(retrying, globalSession) into globalSession.queueToRetryRollback()?
Yes, because regardless of whether it is in a retry state or not, the transaction always transitions from state A to a substate of A, such as Committing. If the transaction in this state fails, the state will change to CommitRetry.
There was a problem hiding this comment.
Why not directly remove this retrying check? Then handle the transaction state transition in the queueToRetryRollback method instead?
Does it mean moving the logic of shouldQueueToRetryRollback(retrying, globalSession) into globalSession.queueToRetryRollback()?
Yes, because regardless of whether it is in a retry state or not, the transaction always transitions from state A to a substate of A, such as Committing. If the transaction in this state fails, the state will change to CommitRetry.
Ok,is it like this?
There was a problem hiding this comment.
@funky-eyes 这里辛苦再帮忙看看
There was a problem hiding this comment.
Why not directly remove this retrying check? Then handle the transaction state transition in the queueToRetryRollback method instead?
Does it mean moving the logic of shouldQueueToRetryRollback(retrying, globalSession) into globalSession.queueToRetryRollback()?
Yes, because regardless of whether it is in a retry state or not, the transaction always transitions from state A to a substate of A, such as Committing. If the transaction in this state fails, the state will change to CommitRetry.
Ok,is it like this?
yes
There was a problem hiding this comment.
Why not directly remove this retrying check? Then handle the transaction state transition in the queueToRetryRollback method instead?
Does it mean moving the logic of shouldQueueToRetryRollback(retrying, globalSession) into globalSession.queueToRetryRollback()?
Yes, because regardless of whether it is in a retry state or not, the transaction always transitions from state A to a substate of A, such as Committing. If the transaction in this state fails, the state will change to CommitRetry.
Ok,is it like this?
yes
done
# Conflicts: # changes/en-us/2.x.md # changes/zh-cn/2.x.md
…k-retrying-status # Conflicts: # changes/en-us/2.x.md # changes/zh-cn/2.x.md
…k-retrying-status # Conflicts: # changes/en-us/2.x.md # changes/zh-cn/2.x.md
|
This branch has conflicts that must be resolved changes/en-us/2.x.md |
Ⅰ. Describe what this PR did
1.Added a check for the GlobalStatus.TimeoutRollbacking state to decide whether to enter the retry queue during rollback.
Ⅱ. Does this pull request fix one issue?
fixes #7470
Ⅲ. Why don't you add test cases (unit test/integration test)?
Ⅳ. Describe how to verify it
Ⅴ. Special notes for reviews