-
Notifications
You must be signed in to change notification settings - Fork 8.7k
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 repeated rollback caused by asynchronous rollback thread #2059
Conversation
Codecov Report
@@ Coverage Diff @@
## develop #2059 +/- ##
=============================================
- Coverage 55.32% 55.31% -0.02%
+ Complexity 2524 2523 -1
=============================================
Files 446 446
Lines 14910 14913 +3
Branches 1765 1766 +1
=============================================
Hits 8249 8249
- Misses 5899 5900 +1
- Partials 762 764 +2
|
@@ -381,6 +381,10 @@ protected void handleRetryRollbacking() { | |||
long now = System.currentTimeMillis(); | |||
for (GlobalSession rollbackingSession : rollbackingSessions) { | |||
try { | |||
//prevent repeated rollback | |||
if (rollbackingSession.getStatus().equals(GlobalStatus.Rollbacking) && !rollbackingSession.isTimeout()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if throw RunTimeException it will no longer rollback.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it will keep rollback
* @return if true force roll back | ||
*/ | ||
public boolean isRollbackingDead() { | ||
return (System.currentTimeMillis() - beginTime) > (3 * 6000); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(System.currentTimeMillis() - beginTime) > (3 * 6000 + timeout);
Suggested overload isTimeout()
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My mistake, you are right!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM,temporary solution to solve concurrent rollback and retry concurrent operations and rollback after restart.
Ⅰ. Describe what this PR did
schedule retry rollback thread pool cause repeated rollback
Ⅱ. Does this pull request fix one issue?
#2058
Ⅲ. Why don't you add test cases (unit test/integration test)?
Ⅳ. Describe how to verify it
Ⅴ. Special notes for reviews