Skip to content

nacos_registry_bugfix#5212

Merged
mercyblitz merged 2 commits intoapache:masterfrom
WZD-MI:feature/nacos_registry_bugfix
Feb 19, 2020
Merged

nacos_registry_bugfix#5212
mercyblitz merged 2 commits intoapache:masterfrom
WZD-MI:feature/nacos_registry_bugfix

Conversation

@goodjava
Copy link
Copy Markdown
Contributor

What is the purpose of the change

两个consumer同时通知接收provider变更信息的时候,在如下场景存在bug

  @Reference(check = false,timeout = 2000)
  private TestService testService;

  @Reference(check = false)
  private TestService testService;
  在dubbo看来是两个consumer,但nacos注册中心的代码如下:
 private void subscribeEventListener(String serviceName, final URL url, final NotifyListener listener)
            throws NacosException {
        if (!nacosListeners.containsKey(serviceName)) {
            EventListener eventListener = event -> {
                if (event instanceof NamingEvent) {
                    NamingEvent e = (NamingEvent) event;
                    notifySubscriber(url, listener, e.getInstances());
                }
            };
            namingService.subscribe(serviceName, eventListener);
            nacosListeners.put(serviceName, eventListener);
        }
    }

有一个验证操作.思路应当是serviceName能构成一个唯一的consumer,但这里是有问题的.
对于dubbo那边是把一个url看做一个key,而这里的key仅仅是serviceName+group+version.
这两边逻辑不匹配.

bug复现步骤
1.同一进程启动两个consumer(两个timeout设置不同,或者其他参数不同)
2.启动一个provider
3.分别调用两个consumer(都可调通)
4.provider停机,手动修改端口号(或者ip)
5.这时候从新启动provider,你会发现一个consumer不能调用了(底层没有通知到)
频发场景
1.provider提供方是docker的情况
2.provider端口号是随机的情况,并且是混合部署

@codecov-io
Copy link
Copy Markdown

codecov-io commented Nov 1, 2019

Codecov Report

Merging #5212 into master will increase coverage by 0.04%.
The diff coverage is 0%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #5212      +/-   ##
============================================
+ Coverage     61.17%   61.21%   +0.04%     
+ Complexity      424      423       -1     
============================================
  Files           919      919              
  Lines         37487    37488       +1     
  Branches       5461     5461              
============================================
+ Hits          22933    22949      +16     
+ Misses        12049    12032      -17     
- Partials       2505     2507       +2
Impacted Files Coverage Δ Complexity Δ
...org/apache/dubbo/registry/nacos/NacosRegistry.java 0% <0%> (ø) 0 <0> (ø) ⬇️
...pache/dubbo/remoting/transport/AbstractServer.java 53.75% <0%> (-3.75%) 0% <0%> (ø)
...he/dubbo/remoting/transport/netty/NettyServer.java 70.17% <0%> (-3.51%) 8% <0%> (-1%)
...c/main/java/org/apache/dubbo/rpc/RpcException.java 80% <0%> (-3.34%) 0% <0%> (ø)
...g/apache/dubbo/rpc/protocol/rest/RestProtocol.java 68.21% <0%> (-3.11%) 0% <0%> (ø)
...a/org/apache/dubbo/monitor/dubbo/DubboMonitor.java 89.71% <0%> (-0.94%) 16% <0%> (-1%)
...rg/apache/dubbo/common/timer/HashedWheelTimer.java 63.44% <0%> (+0.34%) 0% <0%> (ø) ⬇️
...exchange/support/header/HeaderExchangeHandler.java 64.6% <0%> (+2.65%) 0% <0%> (ø) ⬇️
...pache/dubbo/registry/support/AbstractRegistry.java 80.52% <0%> (+2.99%) 0% <0%> (ø) ⬇️
...e/dubbo/remoting/transport/netty/NettyChannel.java 55.68% <0%> (+3.4%) 20% <0%> (+1%) ⬆️
... and 5 more

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 a9746a0...592df91. Read the comment docs.

@chickenlj chickenlj added this to the 2.7.6 milestone Dec 30, 2019
@mercyblitz mercyblitz merged commit 0e30d76 into apache:master Feb 19, 2020
seedeed pushed a commit to seedeed/dubbo that referenced this pull request Feb 20, 2020
Co-authored-by: Xin Wang <xin.victorwang@gmail.com>
@codecov-commenter
Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 0% with 9 lines in your changes missing coverage. Please review.
✅ Project coverage is 61.21%. Comparing base (a9746a0) to head (592df91).

Files with missing lines Patch % Lines
...org/apache/dubbo/registry/nacos/NacosRegistry.java 0.00% 9 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##             master    #5212      +/-   ##
============================================
- Coverage     61.30%   61.21%   -0.09%     
+ Complexity      427      423       -4     
============================================
  Files           919      919              
  Lines         37487    37488       +1     
  Branches       5461     5461              
============================================
- Hits          22980    22949      -31     
- Misses        12003    12032      +29     
- Partials       2504     2507       +3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants