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: xa resource suspension #4228

Merged
merged 45 commits into from
Apr 8, 2022

Conversation

lcmvs
Copy link
Contributor

@lcmvs lcmvs commented Dec 23, 2021

  • I have registered the PR changes.

Ⅰ. Describe what this PR did

修复xa模式获取channel导致的资源悬挂问题

  1. 增加一个特殊的二阶段重试状态:PhaseTwo_RollbackFailed_XAER_NOTA_Retryable和PhaseTwo_CommitFailed_XAER_NOTA_Retryable
  2. rm出现XAException.XAER_NOTA,返回PhaseTwo_RollbackFailed_XAER_NOTA_Retryable或PhaseTwo_CommitFailed_XAER_NOTA_Retryable
  3. tc回滚如果返回PhaseTwo_RollbackFailed_XAER_NOTA_Retryable或PhaseTwo_CommitFailed_XAER_NOTA_Retryable,重试60s,超时直接当作PhaseTwo_Rollbacked
  4. xa模式一阶段执行超过60s,直接回滚,抛异常。
  5. xa模式检查hold住的连接,如果一阶段完成,等待二阶段超过10s,直接将物理连接关闭,一方面防止资源悬挂,还能防止连接hold住时间过长。

Ⅱ. Does this pull request fix one issue?

fixed #4138

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

Ⅳ. Describe how to verify it

Ⅴ. Special notes for reviews

lcmvs and others added 12 commits December 13, 2021 14:07
# Conflicts:
#	common/src/main/java/io/seata/common/DefaultValues.java
#	core/src/main/java/io/seata/core/constants/ConfigurationKeys.java
@funky-eyes funky-eyes changed the title Fix xa resource suspension bugfix: xa resource suspension Dec 23, 2021
@funky-eyes funky-eyes added this to the 1.5.0 milestone Dec 23, 2021
@funky-eyes funky-eyes added module/rm-datasource rm-datasource module module/server server module type: bug Category issues or prs related to bug. labels Dec 23, 2021
@codecov-commenter
Copy link

codecov-commenter commented Dec 24, 2021

Codecov Report

Merging #4228 (01482c4) into develop (abe196f) will decrease coverage by 0.01%.
The diff coverage is 32.18%.

❗ Current head 01482c4 differs from pull request most recent head a34010f. Consider uploading reports for the commit a34010f to get more accurate results

Impacted file tree graph

@@              Coverage Diff              @@
##             develop    #4228      +/-   ##
=============================================
- Coverage      49.27%   49.26%   -0.02%     
- Complexity      4068     4071       +3     
=============================================
  Files            731      731              
  Lines          25370    25462      +92     
  Branches        3146     3156      +10     
=============================================
+ Hits           12501    12543      +42     
- Misses         11539    11583      +44     
- Partials        1330     1336       +6     
Impacted Files Coverage Δ
...c/main/java/io/seata/common/ConfigurationKeys.java 0.00% <ø> (ø)
...n/src/main/java/io/seata/common/DefaultValues.java 0.00% <ø> (ø)
...c/main/java/io/seata/core/context/RootContext.java 46.77% <0.00%> (-2.38%) ⬇️
...e/protocol/transaction/BranchRegisterResponse.java 86.66% <ø> (ø)
...ta/core/rpc/netty/AbstractNettyRemotingClient.java 18.62% <ø> (ø)
...a/io/seata/rm/datasource/xa/ResourceManagerXA.java 14.54% <12.50%> (+0.25%) ⬆️
.../java/io/seata/server/coordinator/DefaultCore.java 53.61% <16.66%> (-2.88%) ⬇️
.../autoconfigure/properties/client/RmProperties.java 39.62% <25.00%> (-2.60%) ⬇️
...oconfigure/properties/server/ServerProperties.java 28.57% <25.00%> (-0.60%) ⬇️
...a/io/seata/rm/datasource/xa/ConnectionProxyXA.java 54.96% <48.27%> (-1.49%) ⬇️
... and 8 more

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

lcmvs and others added 2 commits January 29, 2022 10:16
# Conflicts:
#	changes/1.5.0.md
#	changes/en-us/1.5.0.md
#	common/src/main/java/io/seata/common/DefaultValues.java
#	core/src/main/java/io/seata/core/constants/ConfigurationKeys.java
#	seata-spring-autoconfigure/seata-spring-autoconfigure-client/src/main/java/io/seata/spring/boot/autoconfigure/properties/client/RmProperties.java
@funky-eyes
Copy link
Contributor

@slievrly PTAL

# Conflicts:
#	changes/1.5.0.md
#	changes/en-us/1.5.0.md
#	core/src/main/java/io/seata/core/constants/ConfigurationKeys.java
@@ -185,6 +204,12 @@ public synchronized void commit() throws SQLException {
try {
end(XAResource.TMSUCCESS);
xaResource.prepare(xaBranchXid);
long now = System.currentTimeMillis();
Copy link
Member

Choose a reason for hiding this comment

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

The timeout calculation should not include the time when the database executes XA prepare. Xa prepare is a time-consuming operation. if the branch timeout can be executed directly xa rollback without the need for xa prepare.


超时计算不应该包含数据库prepare的时间,xa prepare 是一个耗时代价比较高的操作,如果分支超时可以直接xa rollback 而不需要 xa prepare.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@slievrly
Copy link
Member

slievrly commented Apr 4, 2022

@@ -49,7 +57,13 @@
private volatile boolean kept = false;

private volatile boolean rollBacked = false;


private volatile Long branchRegisterTime = null;
Copy link
Member

Choose a reason for hiding this comment

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

why do these transaction contexts not need to be cleaned up when close?

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 merged commit 4a69bde into apache:develop Apr 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
module/rm-datasource rm-datasource module 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.

xa模式获取channel导致的资源悬挂问题
4 participants