[Dubbo-5871][Dubbo-5885][Dubbo-5899] Fix nacos registry not work bug since 2.7.6 by laddcn · Pull Request #5902 · apache/dubbo

laddcn · 2020-03-22T15:35:02Z

What is the purpose of the change

Fix nacos registry not work bug mentioned with #5871
, #5885 and #5899

Brief changelog

Verifying this change

Official Demo works out.

Follow this checklist to help us incorporate your contribution quickly and easily:

Make sure there is a GITHUB_issue field for the change (usually before you start working on it). Trivial changes like typos do not require a GITHUB issue. Your pull request should address just this issue, without pulling in other changes - one PR resolves one issue.
Format the pull request title like [Dubbo-XXX] Fix UnknownException when host config not exist #XXX. Each commit in the pull request should have a meaningful subject line and body.
Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
Write necessary unit-test to verify your logic correction, more mock a little better when cross module dependency exist. If the new feature or significant change is committed, please remember to add sample in dubbo samples project.
Run mvn clean install -DskipTests=false & mvn clean test-compile failsafe:integration-test to make sure unit-test and integration-test pass.
If this contribution is large, please follow the Software Donation Guide.

Sync with official master branch

CodingSinger · 2020-03-23T13:30:33Z

...gistry/dubbo-registry-nacos/src/main/java/org/apache/dubbo/registry/nacos/NacosRegistry.java

                List<Instance> instances = namingService.getAllInstances(serviceName);
+
+                //TO FIX bug with https://github.com/apache/dubbo/issues/5885
+                if (CollectionUtils.isEmpty(instances) && isServiceNamesWithCompatibleMode(url)) {


这里有个问题，目前你的更改应该只能解决instances为空的问题，但是没解决不为空的情况下，其中一个serviceName触发实例变动导致的覆盖问题。

是的，现在的解决方案存在这个问题。这两个serviceName表示同一个接口，只是因为版本问题产生的两个不同的名字，两个serviceName的instances应该放在一起考虑。我来改下代码

我增加一个实例管理工具类，实现了相关serviceName的实例一起考虑，避免了覆盖问题，同时对instances为空的情况依然能正常处理，测试功能正常，之前的bug已经解决。

CodingSinger · 2020-03-24T07:57:39Z

...gistry-nacos/src/main/java/org/apache/dubbo/registry/nacos/util/NacosInstanceManageUtil.java

+    /**
+     * serviceName -> corresponding serviceName list
+     */
+    private static final Map<String, Set<String>> correspondingServiceNamesMap = Maps.newHashMap();


这里是不是应该考虑并发问题，用ConcurrentHashMap比较合适？

CodingSinger · 2020-03-24T07:57:59Z

...gistry-nacos/src/main/java/org/apache/dubbo/registry/nacos/util/NacosInstanceManageUtil.java

+    /**
+     * serviceName -> refreshed instance list
+     */
+    private static final Map<String, List<Instance>> serviceInstanceListMap = Maps.newHashMap();


CodingSinger · 2020-03-24T08:01:38Z

有个冲突顺便解决下

Sycn with master

laddcn · 2020-03-24T10:47:49Z

有个冲突顺便解决下

已经解决

codecov-io · 2020-03-24T11:28:44Z

Codecov Report

Merging #5902 into master will decrease coverage by 0.07%.
The diff coverage is 0.00%.

@@             Coverage Diff              @@
##             master    #5902      +/-   ##
============================================
- Coverage     61.31%   61.23%   -0.08%     
- Complexity      498      499       +1     
============================================
  Files           981      982       +1     
  Lines         39106    39147      +41     
  Branches       5620     5628       +8     
============================================
- Hits          23976    23971       -5     
- Misses        12515    12561      +46     
  Partials       2615     2615

Impacted Files	Coverage Δ	Complexity Δ
...org/apache/dubbo/registry/nacos/NacosRegistry.java	`0.00% <0.00%> (ø)`	`0.00 <0.00> (ø)`
...o/registry/nacos/util/NacosInstanceManageUtil.java	`0.00% <0.00%> (ø)`	`0.00 <0.00> (?)`
...in/java/org/apache/dubbo/common/utils/JVMUtil.java	`81.13% <0.00%> (-11.33%)`	`0.00% <0.00%> (ø%)`
...mmon/threadpool/support/AbortPolicyWithReport.java	`83.78% <0.00%> (-5.41%)`	`0.00% <0.00%> (ø%)`
...pache/dubbo/remoting/transport/AbstractServer.java	`53.75% <0.00%> (-3.75%)`	`0.00% <0.00%> (ø%)`
...ng/transport/dispatcher/all/AllChannelHandler.java	`68.96% <0.00%> (ø)`	`0.00% <0.00%> (ø%)`
.../apache/dubbo/remoting/transport/AbstractPeer.java	`67.39% <0.00%> (+4.34%)`	`0.00% <0.00%> (ø%)`
...e/dubbo/remoting/transport/netty/NettyChannel.java	`60.22% <0.00%> (+4.54%)`	`21.00% <0.00%> (+1.00%)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 332e85f...398a94e. Read the comment docs.

CodingSinger · 2020-03-24T11:31:43Z

LGTM

laddcn · 2020-03-24T12:09:53Z

@htynkn Please help me to merge this PR

...gistry/dubbo-registry-nacos/src/main/java/org/apache/dubbo/registry/nacos/NacosRegistry.java

laddcn · 2020-03-26T12:09:33Z

#5871 seems to occur for the same reason with #5885 and #5889

…since 2.7.6 (apache#5902) * apache#5902 * TO FIX bug with apache#5885 * Avoid instances overwriting problem with different service name * Avoid duplicated service name which will lead duplicated subscribe * Use ConcurrentHashMap instead of HashMap * Format codes (cherry picked from commit cb12928)

horizonzy · 2021-03-08T10:29:34Z

...gistry/dubbo-registry-nacos/src/main/java/org/apache/dubbo/registry/nacos/NacosRegistry.java

+     * @return
+     */
+    private boolean isServiceNamesWithCompatibleMode(final URL url) {
+        if (!isAdminProtocol(url) && createServiceName(url).isConcrete()) {


你好，最近看这块代码觉得有点困惑。为什么判断服务是否需要兼容的时候，要通过createServiceName(url).isConcrete()去判断呢？

laddcn and others added 2 commits February 23, 2020 13:34

Merge pull request #1 from apache/master

c52ca7d

Sync with official master branch

TO FIX bug with apache#5885

ca4f9fb

laddcn mentioned this pull request Mar 22, 2020

nacos v1.1.4, dubbo v2.7.6.SNAPSHOT "No provider available from registry xxx" #5885

Closed

2 tasks

htynkn added type/bug Bugs to being fixed type/need-triage Need maintainers to triage labels Mar 23, 2020

CodingSinger reviewed Mar 23, 2020

View reviewed changes

laddcn added 4 commits March 24, 2020 00:18

Avoid instances overwriting problem with different service name

8a4bc9b

Avoid duplicated service name which will lead duplicated subscribe

7121bfa

Resolve Conflicts

bca6b6c

Resolve conflicts

91060c2

CodingSinger reviewed Mar 24, 2020

View reviewed changes

laddcn and others added 4 commits March 24, 2020 18:33

Use ConcurrentHashMap instead of HashMap

9e12335

Resolve conflicts

9406634

Merge pull request #2 from apache/master

08101d9

Sycn with master

Resolve conflicts

a6749aa

laddcn added 2 commits March 24, 2020 18:50

Resolve conflicts

c1a51d4

Format codes

398a94e

tswstarplanet requested changes Mar 26, 2020

View reviewed changes

...gistry/dubbo-registry-nacos/src/main/java/org/apache/dubbo/registry/nacos/NacosRegistry.java Show resolved Hide resolved

laddcn changed the title ~~[Dubbo-5885][Dubbo-5899] Fix nacos registry not work bug since 2.7.6~~ [Dubbo-5871][Dubbo-5885][Dubbo-5899] Fix nacos registry not work bug since 2.7.6 Mar 26, 2020

laddcn requested a review from tswstarplanet March 28, 2020 08:49

mercyblitz merged commit cb12928 into apache:master Mar 31, 2020

vergilyn mentioned this pull request Mar 31, 2020

sync repo appache/dubbo/master vergilyn/dubbo-fork#5

Merged

mercyblitz mentioned this pull request Apr 1, 2020

[Solution] How to integrate Apache Dubbo with Nacos authentication #5956

Closed

This was referenced Apr 14, 2020

nacos1.2.1 +dubbo 2.7.6.1-SNAPSHOT #6013

Closed

upgrade to 2.7.6 version, consumer is abnormal #5983

Closed

vergilyn mentioned this pull request May 28, 2020

nacos + dubbo consumer 重启后无法获取服务提供者 #6230

Closed

horizonzy reviewed Mar 8, 2021

View reviewed changes

AlbumenJ mentioned this pull request Nov 17, 2022

Remove NacosInstanceManageUtil #10949

Merged

8 tasks

Conversation

laddcn commented Mar 22, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What is the purpose of the change

Brief changelog

Verifying this change

Uh oh!

CodingSinger Mar 23, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

laddcn Mar 23, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

laddcn Mar 23, 2020

Choose a reason for hiding this comment

Uh oh!

CodingSinger Mar 24, 2020

Choose a reason for hiding this comment

Uh oh!

CodingSinger Mar 24, 2020

Choose a reason for hiding this comment

Uh oh!

laddcn Mar 24, 2020

Choose a reason for hiding this comment

Uh oh!

CodingSinger commented Mar 24, 2020

Uh oh!

laddcn commented Mar 24, 2020

Uh oh!

codecov-io commented Mar 24, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

CodingSinger commented Mar 24, 2020

Uh oh!

laddcn commented Mar 24, 2020

Uh oh!

Uh oh!

laddcn commented Mar 26, 2020

Uh oh!

horizonzy Mar 8, 2021

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

laddcn commented Mar 22, 2020 •

edited

Loading

CodingSinger Mar 23, 2020 •

edited

Loading

laddcn Mar 23, 2020 •

edited

Loading

codecov-io commented Mar 24, 2020 •

edited

Loading