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

optimize: commit/rollback retry timeout event #4391

Merged
merged 16 commits into from
Feb 24, 2022

Conversation

Bughue
Copy link
Contributor

@Bughue Bughue commented Feb 18, 2022

Ⅰ. Describe what this PR did

全局事务commit/rollback在MAX_ROLLBACK_RETRY_TIMEOUT或者MAX_COMMIT_RETRY_TIMEOUT之后发出eventBus,新增状态CommitRetryTimeout和RollbackRetryTimeout

Ⅱ. Does this pull request fix one issue?

#4387

Ⅲ. Why don't you add test cases (unit test/integration test)?

Ⅳ. Describe how to verify it

Ⅴ. Special notes for reviews

@slievrly slievrly added the first-time contributor first-time contributor label Feb 18, 2022
@slievrly
Copy link
Member

@Bughue please add change log and add global transaction status Desc in https://seata.io/zh-cn/docs/user/appendix/global-transaction-status.html

@Bughue
Copy link
Contributor Author

Bughue commented Feb 18, 2022

@Bughue please add change log and add global transaction status Desc in https://seata.io/zh-cn/docs/user/appendix/global-transaction-status.html

ok apache/incubator-seata-website#425

@funky-eyes funky-eyes added the module/server server module label Feb 20, 2022
@funky-eyes funky-eyes added this to the 1.5.0 milestone Feb 20, 2022
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. 请在changes目录下的两份1.5.0.md中进行pr和作者登记
对于此pr后续扩展想法,进行changestatus为xxxxRetryTimeout,不要再有remove动作
异步补偿定时任务不应该补偿这类状态,未来交由控制台,或者用户手动接入后从数据库删除

@funky-eyes funky-eyes changed the title commit/rollback retry timeout event optimize: commit/rollback retry timeout event Feb 20, 2022
@codecov-commenter
Copy link

codecov-commenter commented Feb 20, 2022

Codecov Report

Merging #4391 (b4eb9ca) into develop (fccd58f) will increase coverage by 0.02%.
The diff coverage is 86.48%.

Impacted file tree graph

@@              Coverage Diff              @@
##             develop    #4391      +/-   ##
=============================================
+ Coverage      48.99%   49.02%   +0.02%     
- Complexity      3956     3961       +5     
=============================================
  Files            733      733              
  Lines          24988    24999      +11     
  Branches        3093     3093              
=============================================
+ Hits           12244    12256      +12     
+ Misses         11448    11446       -2     
- Partials        1296     1297       +1     
Impacted Files Coverage Δ
...ava/io/seata/server/metrics/MetricsSubscriber.java 63.29% <60.00%> (+11.11%) ⬆️
...o/seata/server/coordinator/DefaultCoordinator.java 51.47% <66.66%> (-1.09%) ⬇️
...rc/main/java/io/seata/core/model/GlobalStatus.java 100.00% <100.00%> (ø)
.../java/io/seata/server/coordinator/DefaultCore.java 48.12% <100.00%> (-4.99%) ⬇️
...in/java/io/seata/server/session/SessionHelper.java 72.41% <100.00%> (+7.19%) ⬆️
...in/java/io/seata/server/session/GlobalSession.java 78.65% <0.00%> (-0.40%) ⬇️

@objcoding objcoding self-requested a review February 21, 2022 13:31
Copy link
Contributor

@wangliang181230 wangliang181230 left a comment

Choose a reason for hiding this comment

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

LGTM

@wangliang181230 wangliang181230 merged commit 9ce9b53 into apache:develop Feb 24, 2022
eventBus.post(new GlobalTransactionEvent(globalSession.getTransactionId(), GlobalTransactionEvent.ROLE_TC,
globalSession.getTransactionName(), globalSession.getApplicationId(), globalSession.getTransactionServiceGroup(),
globalSession.getBeginTime(), null, globalSession.getStatus()));
SessionHelper.postTcSessionBeginEvent(globalSession);
Copy link
Member

Choose a reason for hiding this comment

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

postTcSessionBeginEvent is not appropriate here.

Copy link
Member

Choose a reason for hiding this comment

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

optimize the method naming.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

will optimize later

globalSession.getApplicationId(),
globalSession.getTransactionServiceGroup(), globalSession.getBeginTime(),
null, globalSession.getStatus()));
SessionHelper.postTcSessionBeginEvent(globalSession);
Copy link
Member

Choose a reason for hiding this comment

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

postTcSessionBeginEvent is not appropriate here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

will optimize later

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
first-time contributor first-time contributor module/server server module
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants