Skip to content
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: global session is always begin in saga mode #5145

Merged
merged 16 commits into from
Mar 22, 2023
Merged

bugfix: global session is always begin in saga mode #5145

merged 16 commits into from
Mar 22, 2023

Conversation

tuwenlin
Copy link
Contributor

fixes #5045
#5050 can not fix this bug,because in saga mode,when a global transactional end , TM report the global session status to TC rather than do commit or do rollback

@codecov-commenter
Copy link

codecov-commenter commented Dec 12, 2022

Codecov Report

Merging #5145 (b0df085) into develop (357f8e8) will decrease coverage by 0.11%.
The diff coverage is 42.85%.

❗ Current head b0df085 differs from pull request most recent head 544f5aa. Consider uploading reports for the commit 544f5aa to get more accurate results

📣 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    #5145      +/-   ##
=============================================
- Coverage      48.79%   48.69%   -0.11%     
- Complexity      4176     4189      +13     
=============================================
  Files            743      743              
  Lines          26617    26652      +35     
  Branches        3327     3330       +3     
=============================================
- Hits           12989    12977      -12     
- Misses         12225    12280      +55     
+ Partials        1403     1395       -8     
Impacted Files Coverage Δ
...seata/discovery/registry/MultiRegistryFactory.java 0.00% <0.00%> (ø)
...a/io/seata/discovery/registry/RegistryFactory.java 0.00% <0.00%> (ø)
...in/java/io/seata/server/session/SessionHelper.java 60.34% <0.00%> (-3.30%) ⬇️
...ava/io/seata/server/transaction/saga/SagaCore.java 2.75% <0.00%> (ø)
...in/java/io/seata/tm/api/TransactionalTemplate.java 58.90% <22.22%> (-1.97%) ⬇️
...o/seata/integration/http/AbstractHttpExecutor.java 55.55% <50.00%> (-8.60%) ⬇️
.../src/main/java/io/seata/core/protocol/Version.java 47.61% <80.00%> (+40.11%) ⬆️
...ain/java/io/seata/config/ConfigurationFactory.java 57.35% <100.00%> (+0.63%) ⬆️
...o/seata/server/coordinator/DefaultCoordinator.java 47.17% <100.00%> (+1.02%) ⬆️
...ava/io/seata/server/lock/LockerManagerFactory.java 61.11% <100.00%> (+4.86%) ⬆️
... and 1 more

... and 20 files with indirect coverage changes

@CLAassistant
Copy link

CLAassistant commented Dec 12, 2022

CLA assistant check
All committers have signed the CLA.

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 @slievrly

@funky-eyes funky-eyes modified the milestones: 1.7.0, 2.0.0 Jan 31, 2023
@funky-eyes funky-eyes added type: bug Category issues or prs related to bug. module/server server module labels Jan 31, 2023
@slievrly
Copy link
Member

@tuwenlin Pls add change.

@@ -194,12 +194,12 @@ public boolean doGlobalRollback(GlobalSession globalSession, boolean retrying) t
public void doGlobalReport(GlobalSession globalSession, String xid, GlobalStatus globalStatus) throws TransactionException {
if (GlobalStatus.Committed.equals(globalStatus)) {
SessionHelper.removeAllBranch(globalSession, false);
SessionHelper.endCommitted(globalSession,false);
SessionHelper.endSagaGlobalSession(globalSession, GlobalStatus.Committing);
Copy link
Member

Choose a reason for hiding this comment

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

why Committed -> Committing -> Committed ?

LOGGER.info("Global[{}] committed", globalSession.getXid());
} else if (GlobalStatus.Rollbacked.equals(globalStatus)
|| GlobalStatus.Finished.equals(globalStatus)) {
SessionHelper.removeAllBranch(globalSession, false);
SessionHelper.endRollbacked(globalSession, false);
SessionHelper.endSagaGlobalSession(globalSession, GlobalStatus.Rollbacking);
Copy link
Member

Choose a reason for hiding this comment

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

same above.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

same above.

I decide to end the GlobalSession when tc report Committed or Rollbacked

@tuwenlin
Copy link
Contributor Author

@tuwenlin Pls add change.

done

@@ -201,6 +205,10 @@ public static void endRollbacked(GlobalSession globalSession, boolean retryGloba
MetricsPublisher.postSessionDoneEvent(globalSession, IdConstants.STATUS_VALUE_AFTER_ROLLBACKED_KEY, true,
beginTime, retryBranch);
} else {
if (globalSession.isSaga()) {
Copy link
Member

Choose a reason for hiding this comment

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

LGTM. This PR satisfies the functional requirements, but the common method to future designs should be decoupled from the transaction mode.

@wt-better
Copy link
Contributor

Saga mode can consider switch globalReport to commit/rollback,makes it consistent, reduce the judgment like isSaga .

@tuwenlin tuwenlin closed this by deleting the head repository Mar 22, 2023
@tuwenlin tuwenlin reopened this Mar 22, 2023
@slievrly slievrly merged commit 09bfc42 into apache:develop Mar 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
module/server server module type: bug Category issues or prs related to bug.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

store.mode=db 事务结束后globalsession的状态未正确修改为commited
6 participants