optimize: fix eureka always refreshCluster error #4235

lcmvs · 2021-12-27T10:09:39Z

I have registered the PR changes.

Ⅰ. Describe what this PR did

new InetSocketAddress的时候，会去检查ip和端口，某些特殊情况，传到eureka上面的端口号可能为-1，开发环境偶尔遇到过，然后会一直refreshCluster失败，实际上可能只是某个服务实例传到eureka上面的端口号不正确，导致全部实例全部都无法刷新了。所以在这里catch一下这个检查参数的异常。

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

codecov-commenter · 2021-12-27T10:18:04Z

Codecov Report

Merging #4235 (422477d) into develop (7c9a875) will not change coverage.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             develop    #4235   +/-   ##
==========================================
  Coverage      48.84%   48.84%           
  Complexity      3802     3802           
==========================================
  Files            724      724           
  Lines          24238    24238           
  Branches        2972     2971    -1     
==========================================
  Hits           11839    11839           
  Misses         11159    11159           
  Partials        1240     1240

funky-eyes · 2021-12-27T11:50:03Z

这个pr并没有修复bug吧？只不过把异常打印出来了，如果port是-1，不应该去解决为什么是-1的问题吗？

lcmvs · 2021-12-28T01:20:13Z

这个pr并没有修复bug吧？只不过把异常打印出来了，如果port是-1，不应该去解决为什么是-1的问题吗？

端口号为-1的问题，seata也解决不了啊，这个是从eureka拿到的数据，服务注册到eureka的数据本身就不合法，但是不应该因为一个实例的数据不合法，让所有的实例都不能刷新啊。

catch的好处是，不会因为某一个实例注册的eureka的，ip端口数据不合法，而导致其他没问题的实例refresh也失败。

funky-eyes · 2021-12-28T01:34:38Z

为何不通过register来防止server注册错误的port，以及在server上寻找为什么是-1的port呢？

lcmvs · 2021-12-28T02:00:41Z

为何不通过register来防止server注册错误的port，以及在server上寻找为什么是-1的port呢？

不是很明白你的意思啊。
你的意思是说我们提前检查一下eureka实例信息的ip端口，而不是等它抛异常catch？

funky-eyes · 2021-12-28T05:50:29Z

为何不通过register来防止server注册错误的port，以及在server上寻找为什么是-1的port呢？

不是很明白你的意思啊。你的意思是说我们提前检查一下eureka实例信息的ip端口，而不是等它抛异常catch？

在tc也就是seata-server进行registry的时候进行注册，client这个检查可以保留，以防有新老版本server共存的情况
我们要治标，就得在server端的registry时检查为什么port变成了-1

lcmvs · 2021-12-28T06:21:11Z

我大概明白你的意思了，你应该是以为tc注册到eureka的ip端口不合法是吧？

List<Application> applications = getEurekaClient(false).getApplications().getRegisteredApplications();

但是她这里实际上不是只拿了tc注册的信息，而是eureka上面全部服务的注册信息。我这里出现ip端口不合法也不是因为某个tc不合法，而是其他注册到eureka上面的业务服务导致的。

其实我也不是很明白为啥注册的eureka上的所有服务信息都要保存起来。按道理不是应该只需要保存一下tc的实例信息吗？

而且每10s更新这一大堆实例信息，也会对客户端性能有点影响吧？

funky-eyes · 2021-12-28T06:39:13Z

我大概明白你的意思了，你应该是以为tc注册到eureka的ip端口不合法是吧？
List<Application> applications = getEurekaClient(false).getApplications().getRegisteredApplications();
但是她这里实际上不是只拿了tc注册的信息，而是eureka上面全部服务的注册信息。我这里出现ip端口不合法也不是因为某个tc不合法，而是其他注册到eureka上面的业务服务导致的。

其实我也不是很明白为啥注册的eureka上的所有服务信息都要保存起来。按道理不是应该只需要保存一下tc的实例信息吗？

而且每10s更新这一大堆实例信息，也会对客户端性能有点影响吧？

这里做的有点问题，application= clusterName的哪些才需要被遍历，你帮忙一起处理下？

lcmvs · 2021-12-28T06:40:15Z

我大概明白你的意思了，你应该是以为tc注册到eureka的ip端口不合法是吧？
List<Application> applications = getEurekaClient(false).getApplications().getRegisteredApplications();
但是她这里实际上不是只拿了tc注册的信息，而是eureka上面全部服务的注册信息。我这里出现ip端口不合法也不是因为某个tc不合法，而是其他注册到eureka上面的业务服务导致的。
其实我也不是很明白为啥注册的eureka上的所有服务信息都要保存起来。按道理不是应该只需要保存一下tc的实例信息吗？
而且每10s更新这一大堆实例信息，也会对客户端性能有点影响吧？
这里做的有点问题，application= clusterName的哪些才需要被遍历，你帮忙一起处理下？

行

…x_eureka_refresh_fail

funky-eyes · 2021-12-29T13:03:19Z

...overy-eureka/src/main/java/io/seata/discovery/registry/eureka/EurekaRegistryServiceImpl.java

-    private void refreshCluster() {
-        List<Application> applications = getEurekaClient(false).getApplications().getRegisteredApplications();
+    private void refreshCluster(String clusterName) {
+        Application application = getEurekaClient(false).getApplications().getRegisteredApplications(clusterName);


直接getApplication(clusterName) 是否可以使用? 而不是拿所有的applications里的clustername

试了一下，也可以，不过好像区别不大。
@OverRide
public Application getApplication(String appName) {
return getApplications().getRegisteredApplications(appName);
}

...overy-eureka/src/main/java/io/seata/discovery/registry/eureka/EurekaRegistryServiceImpl.java

funky-eyes · 2022-01-01T14:56:24Z

...overy-eureka/src/main/java/io/seata/discovery/registry/eureka/EurekaRegistryServiceImpl.java

        }

-        clusterAddressMap = collect;
+        clusterAddressMap.put(clusterName, list);


不要put,还是按之前的走,否则在集群切换的时候,之前的集群地址会残留在内存里没法清除

funky-eyes

LGTM 做一下pr登记

xingfudeshi

LGTM.

…x_eureka_refresh_fail # Conflicts: # changes/1.5.0.md # changes/en-us/1.5.0.md

slievrly

LGTM

fix

04d38d3

lcmvs added 3 commits December 28, 2021 11:30

fix

d9328ce

fix

2571b25

Merge branch 'develop' into fix_eureka_refresh_fail

2eadb8a

lcmvs added 2 commits December 28, 2021 15:01

fix

7ba54c0

Merge remote-tracking branch 'origin/fix_eureka_refresh_fail' into fi…

a6925bc

…x_eureka_refresh_fail

funky-eyes added this to the 1.5.0 milestone Dec 28, 2021

funky-eyes added module/discovery discovery module type: bug Category issues or prs related to bug. labels Dec 28, 2021

funky-eyes reviewed Dec 29, 2021

View reviewed changes

fix

dd8e008

funky-eyes reviewed Dec 29, 2021

View reviewed changes

...overy-eureka/src/main/java/io/seata/discovery/registry/eureka/EurekaRegistryServiceImpl.java Outdated Show resolved Hide resolved

fix

5131388

funky-eyes reviewed Jan 1, 2022

View reviewed changes

lcmvs added 2 commits January 4, 2022 14:27

fix

a18d1c5

fix

dbf652d

funky-eyes approved these changes Jan 5, 2022

View reviewed changes

funky-eyes requested review from slievrly and xingfudeshi January 5, 2022 11:10

Merge branch 'develop' into fix_eureka_refresh_fail

50dd708

xingfudeshi approved these changes Jan 5, 2022

View reviewed changes

lcmvs changed the title ~~bugfix: fix eureka always refreshCluster error~~ optimize: fix eureka always refreshCluster error Jan 7, 2022

lcmvs added 3 commits January 7, 2022 13:36

fix

4fe1a7b

Merge branch 'seata:develop' into fix_eureka_refresh_fail

4cfd63e

Merge remote-tracking branch 'origin/fix_eureka_refresh_fail' into fi…

422477d

…x_eureka_refresh_fail # Conflicts: # changes/1.5.0.md # changes/en-us/1.5.0.md

funky-eyes merged commit 6fa8e91 into apache:develop Jan 7, 2022

slievrly reviewed Feb 8, 2022

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

optimize: fix eureka always refreshCluster error #4235

optimize: fix eureka always refreshCluster error #4235

lcmvs commented Dec 27, 2021

codecov-commenter commented Dec 27, 2021 •

edited

funky-eyes commented Dec 27, 2021

lcmvs commented Dec 28, 2021

funky-eyes commented Dec 28, 2021

lcmvs commented Dec 28, 2021

funky-eyes commented Dec 28, 2021

lcmvs commented Dec 28, 2021 •

edited

funky-eyes commented Dec 28, 2021

lcmvs commented Dec 28, 2021

funky-eyes Dec 29, 2021

lcmvs Dec 29, 2021 •

edited

funky-eyes Jan 1, 2022 •

edited

lcmvs Jan 4, 2022

funky-eyes left a comment

xingfudeshi left a comment

slievrly left a comment

optimize: fix eureka always refreshCluster error #4235

optimize: fix eureka always refreshCluster error #4235

Conversation

lcmvs commented Dec 27, 2021

Ⅰ. Describe what this PR did

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

codecov-commenter commented Dec 27, 2021 • edited

Codecov Report

funky-eyes commented Dec 27, 2021

lcmvs commented Dec 28, 2021

funky-eyes commented Dec 28, 2021

lcmvs commented Dec 28, 2021

funky-eyes commented Dec 28, 2021

lcmvs commented Dec 28, 2021 • edited

funky-eyes commented Dec 28, 2021

lcmvs commented Dec 28, 2021

funky-eyes Dec 29, 2021

Choose a reason for hiding this comment

lcmvs Dec 29, 2021 • edited

Choose a reason for hiding this comment

funky-eyes Jan 1, 2022 • edited

Choose a reason for hiding this comment

lcmvs Jan 4, 2022

Choose a reason for hiding this comment

funky-eyes left a comment

Choose a reason for hiding this comment

xingfudeshi left a comment

Choose a reason for hiding this comment

slievrly left a comment

Choose a reason for hiding this comment

codecov-commenter commented Dec 27, 2021 •

edited

lcmvs commented Dec 28, 2021 •

edited

lcmvs Dec 29, 2021 •

edited

funky-eyes Jan 1, 2022 •

edited