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

fix the bug can't subscribe the interface #5085 #5086

Closed
wants to merge 1 commit into from

Conversation

CodingSinger
Copy link
Member

fix the bug #5085

@codecov-io
Copy link

Codecov Report

Merging #5086 into master will increase coverage by 0.02%.
The diff coverage is 66.66%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #5086      +/-   ##
============================================
+ Coverage        64%   64.02%   +0.02%     
- Complexity      451      452       +1     
============================================
  Files           769      769              
  Lines         33251    33293      +42     
  Branches       5247     5251       +4     
============================================
+ Hits          21282    21316      +34     
- Misses         9541     9543       +2     
- Partials       2428     2434       +6
Impacted Files Coverage Δ Complexity Δ
...he/dubbo/registry/zookeeper/ZookeeperRegistry.java 67.36% <66.66%> (+0.69%) 0 <0> (ø) ⬇️
.../apache/dubbo/remoting/transport/AbstractPeer.java 58.69% <0%> (-4.35%) 0% <0%> (ø)
...c/main/java/org/apache/dubbo/rpc/RpcException.java 80% <0%> (-3.34%) 0% <0%> (ø)
...he/dubbo/registry/multicast/MulticastRegistry.java 67.87% <0%> (-1.81%) 0% <0%> (ø)
.../dubbo/remoting/transport/netty4/NettyChannel.java 63.63% <0%> (-1.14%) 0% <0%> (ø)
...rg/apache/dubbo/common/timer/HashedWheelTimer.java 62.41% <0%> (-0.35%) 0% <0%> (ø)
...g/apache/dubbo/registry/consul/ConsulRegistry.java 62.5% <0%> (+0.62%) 29% <0%> (ø) ⬇️
...a/org/apache/dubbo/monitor/dubbo/DubboMonitor.java 90.65% <0%> (+0.93%) 17% <0%> (+1%) ⬆️
...ing/zookeeper/support/AbstractZookeeperClient.java 69.66% <0%> (+3.37%) 0% <0%> (ø) ⬇️
... and 1 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 a3c89e2...052cdbb. Read the comment docs.

@CLAassistant
Copy link

CLAassistant commented Sep 26, 2019

CLA assistant check
All committers have signed the CLA.

@Patrick0308
Copy link

这么做的话,zookeeper上短时间内删除和增加相同的节点会导致所有机器重新subcribe这些节点,会不会网络波动有点大?

@CodingSinger
Copy link
Member Author

这么做的话,zookeeper上短时间内删除和增加相同的节点会导致所有机器重新subcribe这些节点,会不会网络波动有点大?

谢谢反馈,但是没明白你的意思,这个改动只是新增了在节点被删除后对内部已订阅节点的缓存的清理,我觉得这是符合逻辑的,至于你说的上下线导致的网络流量过大这个问题,是zookeeper作为注册中心一直有的问题,和pr应该没啥直接联系。

@@ -138,14 +139,18 @@ public void doSubscribe(final URL url, final NotifyListener listener) {
ChildListener zkListener = listeners.get(listener);
if (zkListener == null) {
listeners.putIfAbsent(listener, (parentPath, currentChilds) -> {
List<String> copy = new LinkedList<>(anyServices);

Choose a reason for hiding this comment

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

Refactor: using iterators to remove child is better ?

@AlbumenJ
Copy link
Member

@CodingSinger Hi, thanks for your contributions.

Please merge the latest master branch to resolve confilcting files.

@AlbumenJ AlbumenJ added the status/waiting-for-feedback Need reporters to triage label Apr 11, 2021
@AlbumenJ
Copy link
Member

Close for long time no response.

Please feel free to reopen if you have any question.

If you think these changes still useful in the latest master branch, please submit a new pull request.

@AlbumenJ AlbumenJ closed this May 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status/waiting-for-feedback Need reporters to triage
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants