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: tcc phase two response timeout exception(#3484) #3497

Merged
merged 7 commits into from
Dec 2, 2021

Conversation

zch0214
Copy link
Contributor

@zch0214 zch0214 commented Jan 28, 2021

Ⅰ. Describe what this PR did

TCC模式下,global commit的响应依赖于branch commit的响应。而global commit与branch commit的处理线程用的同一线程池,当并发量较大时,若线程池里都是global commit的线程,此时无法创建新线程执行branch commit的响应处理,导致超时。

Ⅱ. Does this pull request fix one issue?

fixes #3484
fixes #3711

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

Ⅳ. Describe how to verify it

Ⅴ. Special notes for reviews

@xingfudeshi xingfudeshi changed the title bugfix: tcc phrase two response timeout exception(#3484) bugfix: tcc phase two response timeout exception(#3484) Feb 16, 2021
@xyz327
Copy link
Contributor

xyz327 commented Apr 30, 2021

我本地将seata服务核心线程池(ServerHanlderThread)设置为1 然后再使用 TCC 模式就会出现超时问题
这个是我复现的项目demo 以及问题分析 seata 版本 1.4.2 https://github.com/xyz327/seata-tcc-timeout-demo

Copy link
Contributor

@xyz327 xyz327 left a comment

Choose a reason for hiding this comment

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

如果是单线程必然出现,那默认的50个线程在高并发情况下也会出现这个问题

@codecov-commenter
Copy link

codecov-commenter commented May 9, 2021

Codecov Report

Merging #3497 (eae37b6) into develop (c60cb5e) will decrease coverage by 0.03%.
The diff coverage is 0.00%.

Impacted file tree graph

@@              Coverage Diff              @@
##             develop    #3497      +/-   ##
=============================================
- Coverage      49.43%   49.40%   -0.04%     
+ Complexity      3761     3759       -2     
=============================================
  Files            700      700              
  Lines          23626    23637      +11     
  Branches        2918     2918              
=============================================
- Hits           11679    11677       -2     
- Misses         10756    10767      +11     
- Partials        1191     1193       +2     
Impacted Files Coverage Δ
...ava/io/seata/core/constants/ConfigurationKeys.java 0.00% <ø> (ø)
...a/io/seata/core/rpc/netty/NettyRemotingServer.java 0.00% <0.00%> (ø)
...ava/io/seata/core/rpc/netty/NettyServerConfig.java 0.00% <0.00%> (ø)
...o/seata/server/coordinator/DefaultCoordinator.java 47.84% <0.00%> (-0.54%) ⬇️
...in/java/io/seata/server/session/GlobalSession.java 79.56% <0.00%> (-0.44%) ⬇️

@Pinocchio2018
Copy link
Contributor

请问,这个优化会被合并吗?我想了解下这个超时问题的最新进展。

will this change be merged? I want to know the latest progress on this problem.

@Pinocchio2018
Copy link
Contributor

我想提个问题

public class RmBranchCommitProcessor implements RemotingProcessor {

    private void handleBranchCommit(RpcMessage request, String serverAddress, BranchCommitRequest branchCommitRequest) {
        BranchCommitResponse resultMessage = (BranchCommitResponse)this.handler.onRequest(branchCommitRequest, (RpcContext)null);
        if (LOGGER.isDebugEnabled()) {
            LOGGER.debug("branch commit result:" + resultMessage);
        }

        try {
            this.remotingClient.sendAsyncResponse(serverAddress, request, resultMessage);
        } catch (Throwable var6) {
            LOGGER.error("branch commit error: {}", var6.getMessage(), var6);
        }

    }
}

请问,this.remotingClient.sendAsyncResponse(serverAddress, request, resultMessage);这行代码,是在向tc发送二阶段成功的响应吗?这里是在向tc发送一个新的request吗?

如果是在向tc发req的话,我不理解,为什么rm收到tc的调用后,不是return结果,而是新发送一个req给tc呢?

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

@l81893521
Copy link
Contributor

This change is important, please @slievrly review it.

@l81893521
Copy link
Contributor

We talk about your solution lately, and we agree about that. Can you change the 1.5.0.md?

@zch0214
Copy link
Contributor Author

zch0214 commented Oct 21, 2021

We talk about your solution lately, and we agree about that. Can you change the 1.5.0.md?

Sure.Thanks for merging.

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

@funky-eyes funky-eyes added the module/core core module label Dec 2, 2021
@funky-eyes funky-eyes added this to the 1.5.0 milestone Dec 2, 2021
@funky-eyes funky-eyes merged commit c86f7ef into apache:develop Dec 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
7 participants