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: ensure that the register message is sent after RM client initialization #2968

Merged
merged 3 commits into from
Aug 4, 2020

Conversation

guang384
Copy link
Contributor

@guang384 guang384 commented Aug 4, 2020

Ⅰ. Describe what this PR did

【情况说明】
当资源注册(ResourceManager.registerResource)先于RM初始化(RMClient.init)时会打印错误日志,不过由于RMClient初始化后会有定时连接检测,所以对业务的影响只是在连接检测被触发之前资源不会被注册到TC,连接检测启动延迟目前为1分钟。(AbstractNettyRemotingClient.SCHEDULE_DELAY_MILLS = 60 * 1000L)。

错误日志:
Failed to get available servers: endpoint format should like ip:port
image

【原因分析】:

由于资源注册时RM还没有初始化,所以此时transactionServiceGroup为空!

【解决方案】:
RmNettyRemotingClient.registerResource 加上transactionServiceGroup是否为空的检查
延迟注册,注册资源时如果RM尚未初始化则先不连接,待RM初始化时检查有已注册的RM则连接后将资源一并注册到TC

Ⅱ. Does this pull request fix one issue?

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

Ⅳ. Describe how to verify it

【复现方法】:
1、样例代码 直接将POM中seata版本切换为1.3运行
日志中会找到错误:Failed to get available servers: endpoint format should like ip:port
2、简化代码后发现,只要当资源注册先于RM初始化就会复现问题。可以将样例代码中的RM初始化部分简化为如下形式,可复现问题。同时可见,一分钟后连接检测时会自动修复问题。

DataSourceUtil.getDataSource("account"); // DataSourceProxy初始化的时候会触发registerResource
RMClient.init(applicationId, txServiceGroup);

image

3、进一步分析发现,如果初始化先于资源注册,则无此问题。

RMClient.init(applicationId, txServiceGroup);
DataSourceUtil.getDataSource("account"); 

image

【验证方式】:
修正后,无论先注册资源还是先初始化RM,都不会报错,且当资源注册先于RM初始化时,会在RM初始化后立即将资源注册到TC

Ⅴ. Special notes for reviews

@codecov-commenter
Copy link

codecov-commenter commented Aug 4, 2020

Codecov Report

Merging #2968 into develop will decrease coverage by 0.01%.
The diff coverage is 0.00%.

Impacted file tree graph

@@              Coverage Diff              @@
##             develop    #2968      +/-   ##
=============================================
- Coverage      50.22%   50.21%   -0.02%     
+ Complexity      3054     3053       -1     
=============================================
  Files            599      599              
  Lines          19460    19466       +6     
  Branches        2392     2396       +4     
=============================================
  Hits            9774     9774              
- Misses          8702     8707       +5     
- Partials         984      985       +1     
Impacted Files Coverage Δ Complexity Δ
...io/seata/core/rpc/netty/RmNettyRemotingClient.java 39.13% <0.00%> (-2.16%) 10.00 <0.00> (ø)

@funky-eyes funky-eyes added the module/rm rm module label Aug 4, 2020
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

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 merged commit 0d16fb8 into apache:develop Aug 4, 2020
@l81893521 l81893521 added this to the 1.4.0 milestone Aug 5, 2020
l81893521 pushed a commit to l81893521/seata that referenced this pull request Oct 22, 2020
hicf pushed a commit to hicf/seata that referenced this pull request Nov 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
module/rm rm module
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants