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

dubbo zookeeper registry too slow #4880

Merged
merged 4 commits into from
Sep 5, 2019

Conversation

qixiaobo
Copy link
Contributor

ref to #4828
I just add concurrentHashSet to avoid checking persistent node exist

I find a branch missing delete the persist node
@qixiaobo
Copy link
Contributor Author

@tswstarplanet You are right, I find the metaDataReport delete the persist node

I find a branch missing delete the persist node
@codecov-io
Copy link

Codecov Report

Merging #4880 into master will decrease coverage by 0.03%.
The diff coverage is 57.14%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #4880      +/-   ##
============================================
- Coverage     63.97%   63.93%   -0.04%     
- Complexity      452      453       +1     
============================================
  Files           769      769              
  Lines         33157    33168      +11     
  Branches       5228     5232       +4     
============================================
- Hits          21211    21206       -5     
- Misses         9525     9545      +20     
+ Partials       2421     2417       -4
Impacted Files Coverage Δ Complexity Δ
...ing/zookeeper/support/AbstractZookeeperClient.java 67.04% <57.14%> (-3.33%) 0 <0> (ø)
.../apache/dubbo/qos/protocol/QosProtocolWrapper.java 65.85% <0%> (-17.08%) 0% <0%> (ø)
...ache/dubbo/remoting/transport/AbstractChannel.java 75% <0%> (-12.5%) 0% <0%> (ø)
...o/rpc/cluster/loadbalance/AbstractLoadBalance.java 42.85% <0%> (-10.09%) 0% <0%> (ø)
...bbo/registry/support/ProviderConsumerRegTable.java 80.48% <0%> (-4.88%) 0% <0%> (ø)
.../dubbo/monitor/support/AbstractMonitorFactory.java 78.37% <0%> (-2.71%) 0% <0%> (ø)
.../rpc/protocol/dubbo/LazyConnectExchangeClient.java 56.47% <0%> (-2.36%) 0% <0%> (ø)
.../apache/dubbo/remoting/transport/AbstractPeer.java 60.86% <0%> (-2.18%) 0% <0%> (ø)
...g/apache/dubbo/registry/consul/ConsulRegistry.java 61.87% <0%> (-0.63%) 29% <0%> (ø)
...a/org/apache/dubbo/monitor/dubbo/DubboMonitor.java 90.65% <0%> (+0.93%) 17% <0%> (+1%) ⬆️
... and 4 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 00653c3...8c92462. Read the comment docs.

Copy link
Member

@beiwei30 beiwei30 left a comment

Choose a reason for hiding this comment

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

the change looks good except for explicitly using guava, pls. change it. @qixiaobo

change to dubbo concurrenthashset
@cvictory cvictory requested a review from beiwei30 September 4, 2019 03:37
@cvictory cvictory added this to the 2.7.4 milestone Sep 4, 2019
Copy link
Member

@beiwei30 beiwei30 left a comment

Choose a reason for hiding this comment

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

LGTM

@beiwei30 beiwei30 merged commit 101c5ec into apache:master Sep 5, 2019
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.

5 participants