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: optimize the conditions for executing unlocking #3236

Merged
merged 19 commits into from
Feb 8, 2021

Conversation

wangliang181230
Copy link
Contributor

@wangliang181230 wangliang181230 commented Oct 29, 2020

Ⅰ. Describe what this PR did

optimize: opt the conditions for executing unlocking.
优化:优化执行解锁的条件,避免不必要的unlock操作。
顺便修复了一个BUG:当TC的server.rollbackRetryTimeoutUnlockEnable=true时,如果unlock出现异常,全局事务却删除成功时锁数据会遗留下来。

当前PR优化了以下五种情况:

  1. 当没有AT分支时,就不清理锁数据:包含4种情况。
    1.1. 全局提交刚开始,执行closeAndClean()时,没有AT分支就不需要clean().
    1.2. 全局提交或全局回滚完成后,执行SessionHelper.endXxxxed()方法时,因为已经没有分支数据了,不需要clean()
    1.3. 当分支返回PhaseTwo_CommitFailed_UnretryablePhaseTwo_RollbackFailed_Unretryable时,当剩余分支没有AT分支时,不需要clean().
    1.4. SAGA事务,不需要clean().
  2. 在全局事务状态为Committing, CommitRetrying, AsyncCommitting时,删除分支前不需要unlock.

Ⅱ. 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

Copy link
Contributor

@caohdgege caohdgege left a comment

Choose a reason for hiding this comment

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

GlobalSession.end 里面的那个clean方法也应该要改,那个地方99%的情况下都是不需要clean的。

@wangliang181230 wangliang181230 changed the title optimize: do not clean when has no AT branch at global commit optimize: do not clean when has no AT branch Oct 29, 2020
@wangliang181230
Copy link
Contributor Author

GlobalSession.end 里面的那个clean方法也应该要改,那个地方99%的情况下都是不需要clean的。

是的,你是对的,我已经将添加的判断条件移到clean方法中了。

@codecov-io
Copy link

codecov-io commented Oct 29, 2020

Codecov Report

Merging #3236 (36580c2) into develop (23751d6) will decrease coverage by 0.45%.
The diff coverage is 58.33%.

Impacted file tree graph

@@              Coverage Diff              @@
##             develop    #3236      +/-   ##
=============================================
- Coverage      51.57%   51.12%   -0.46%     
+ Complexity      3379     3303      -76     
=============================================
  Files            619      614       -5     
  Lines          20468    20133     -335     
  Branches        2563     2516      -47     
=============================================
- Hits           10556    10292     -264     
+ Misses          8852     8807      -45     
+ Partials        1060     1034      -26     
Impacted Files Coverage Δ Complexity Δ
...in/java/io/seata/server/session/GlobalSession.java 82.89% <54.54%> (-0.75%) 75.00 <7.00> (+4.00) ⬇️
...c/main/java/io/seata/config/FileConfiguration.java 43.29% <100.00%> (+0.18%) 11.00 <0.00> (-1.00) ⬆️
.../java/io/seata/config/ConfigurationChangeType.java 0.00% <0.00%> (-100.00%) 0.00% <0.00%> (-1.00%)
tm/src/main/java/io/seata/tm/TMClient.java 0.00% <0.00%> (-83.34%) 0.00% <0.00%> (-2.00%)
...n/src/main/java/io/seata/common/util/IdWorker.java 0.00% <0.00%> (-83.34%) 0.00% <0.00%> (-12.00%)
...in/java/io/seata/config/AbstractConfiguration.java 28.57% <0.00%> (-42.86%) 7.00% <0.00%> (-8.00%)
...java/io/seata/config/ConfigurationChangeEvent.java 38.46% <0.00%> (-38.47%) 6.00% <0.00%> (-3.00%)
.../main/java/io/seata/rm/datasource/AsyncWorker.java 15.00% <0.00%> (-32.30%) 5.00% <0.00%> (-4.00%)
.../main/java/io/seata/config/ConfigurationCache.java 45.00% <0.00%> (-21.67%) 10.00% <0.00%> (-1.00%)
...seata/core/exception/TransactionExceptionCode.java 82.75% <0.00%> (-17.25%) 2.00% <0.00%> (-1.00%)
... and 76 more

Copy link
Contributor

@caohdgege caohdgege left a comment

Choose a reason for hiding this comment

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

LGMT

@wangliang181230 wangliang181230 added this to the 1.5.0 milestone Oct 29, 2020
@caohdgege
Copy link
Contributor

caohdgege commented Oct 29, 2020

如果globalSession的status是Committing,AsyncCommitting,CommitRetrying的时候,那么GlobalSession#removeBranch的时候,其实也不需要unlock了,因为他在前面的closeAndClean的时候已经把lock都清除了,王良哥看一下要不要在这个pr里面一起处理一下。

@wangliang181230 wangliang181230 changed the title optimize: do not clean when has no AT branch optimize: do not clean when has no AT branch or global status in Committing,AsyncCommitting,CommitRetrying before remove branch Oct 29, 2020
@wangliang181230 wangliang181230 changed the title optimize: do not clean when has no AT branch or global status in Committing,AsyncCommitting,CommitRetrying before remove branch optimize: opt the logic of unlock Oct 29, 2020
@wangliang181230 wangliang181230 changed the title optimize: opt the logic of unlock optimize: opt the conditions for executing unlocking Oct 29, 2020
@funky-eyes funky-eyes added the module/server server module label Nov 30, 2020
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下2个md文件的pr登记进行补充下

@wangliang181230
Copy link
Contributor Author

LGTM changes下2个md文件的pr登记进行补充下

done

Copy link
Contributor

@l81893521 l81893521 left a comment

Choose a reason for hiding this comment

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

LGTM

…-tcc-global-commit

# Conflicts:
#	changes/1.5.0.md
#	changes/en-us/1.5.0.md
@funky-eyes funky-eyes merged commit 8dc7b22 into apache:develop Feb 8, 2021
@wangliang181230 wangliang181230 deleted the optimize-tcc-global-commit branch February 8, 2021 07:32
Copy link
Member

@slievrly slievrly 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 slievrly changed the title optimize: opt the conditions for executing unlocking optimize: optimize the conditions for executing unlocking Apr 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
module/server server module
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants