-
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
feature: support propagation.never, propagation.mandatory, transaction suspend and resume api #2359
Conversation
Codecov Report
@@ Coverage Diff @@
## develop #2359 +/- ##
=============================================
- Coverage 51.75% 51.73% -0.02%
- Complexity 2697 2702 +5
=============================================
Files 517 518 +1
Lines 16788 16819 +31
Branches 2030 2035 +5
=============================================
+ Hits 8688 8701 +13
- Misses 7289 7306 +17
- Partials 811 812 +1
|
@CharmingRabbit @zjinlei PTAL |
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
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
I created a new class SuspendedResourcesHolder,Can be used to temporarily save transaction state for later recovery,when we suspend global transaction ,or change the branchType, Or handle other nesting situations later. |
@zjinlei @CharmingRabbit @a364176773 PTAL,THS |
@zjinlei @CharmingRabbit @a364176773 PTAL,THS |
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.
LTGM
return business.execute(); | ||
} | ||
break; | ||
case REQUIRED: | ||
break; | ||
case NEVER: | ||
if (existingTransaction()) { | ||
throw new TransactionException("Existing transaction found for transaction marked with propagation 'never'"); |
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.
how about print xid here?
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.
ok,i will add now
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.
finished
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
1.add propagation about:never,mandatory;
2.optimize the transationTemplate code;
Ⅱ. 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