-
Notifications
You must be signed in to change notification settings - Fork 8.8k
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 wrong exception information when rollback fails due to dirty data #2333
Conversation
Codecov Report
@@ Coverage Diff @@
## develop #2333 +/- ##
============================================
- Coverage 51.77% 51.6% -0.18%
+ Complexity 2698 2695 -3
============================================
Files 517 517
Lines 16752 16740 -12
Branches 2033 1995 -38
============================================
- Hits 8674 8639 -35
+ Misses 7270 7268 -2
- Partials 808 833 +25
|
@@ -143,7 +144,9 @@ private void rollbackTransaction(GlobalTransaction tx, Throwable ex) throws Tran | |||
tx.rollback(); | |||
triggerAfterRollback(); | |||
// 3.1 Successfully rolled back | |||
throw new TransactionalExecutor.ExecutionException(tx, TransactionalExecutor.Code.RollbackDone, ex); | |||
throw new TransactionalExecutor.ExecutionException(tx, |
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 feel that the code you changed is not correct. I think it is OK to throw RollbackDone directly here.
@a364176773 check the code, no problem. However, there are a few minor problems. Tx.getStatus () is a remote call, but in fact status is already available when rollback. The second is that the exception thrown by tx.rollback () must be the rpc exception of the framework and the business rollback fails Both are defined with RollbackFailure, without distinction. |
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, pr title should be written according to the specification.
@a364176773 Please solve the problem of getStatus to get the remote status, in fact status has been returned in rollback. |
@slievrly i will submit in additional |
optimize:remove redundant configuration (apache#2348)
@zjinlei @xingfudeshi PTAL |
@@ -67,6 +67,12 @@ public void onRollbackFailure(GlobalTransaction tx, Throwable cause) { | |||
timer.newTimeout(new CheckTimerTask(tx, GlobalStatus.Rollbacked), SCHEDULE_INTERVAL_SECONDS, TimeUnit.SECONDS); | |||
} | |||
|
|||
@Override | |||
public void onRollbackRetrying(GlobalTransaction tx, Throwable cause) { | |||
LOGGER.warn("Failed to rollback transaction[" + tx.getXid() + "]", cause); |
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.
Please use {}
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.
@xingfudeshi PTAL
@xingfudeshi @zjinlei PTAL |
optimize: adjust the processing logic for unsupported listeners (apache#2354)
@Override | ||
public void onRollbackRetrying(GlobalTransaction tx, Throwable cause) { | ||
StackTraceLogger.info(LOGGER, cause, "Retrying to rollback transaction[{}]", new String[] {tx.getXid()}, null, | ||
null); |
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.
StackTraceLogger.info(LOGGER, cause, "Retrying to rollback transaction[{}]", new String[] {tx.getXid()}, "Retrying to rollback transaction[{}]", new String[] {tx.getXid()});
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.
StackTraceLogger.info(LOGGER, cause, "Retrying to rollback transaction[{}]", new String[] {tx.getXid()}, "Retrying to rollback transaction[{}]", new String[] {tx.getXid()});
PTAL @zjinlei
optimize: StackTraceLogger
…nto seata_develop
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
Ⅰ. Describe what this PR did
fix wrong exception information when rollback fails due to dirty data
Ⅱ. Does this pull request fix one issue?
#2325