fix #3179. Make registry, configcenter, matadata-report share a zooke… #3182

cvictory · 2019-01-10T07:28:45Z

In dubbo 2.7, we introduce config-center, metadata-report and we define SPI for them. There are zookeeper extension for config-center and medata-report.
So far, there are 3 connections that are used for registry, config-center, metadata-report .
We should use one connection that can be shared with registry, config-center, metadata-report .

Add group field into MetadataReportConfig.

When get ZookeeperClient, must make it is connected.

… zookeeper connection

…re a zookeeper connection

codecov-io · 2019-01-10T11:59:20Z

Codecov Report

Merging #3182 into 2.7.0-release will increase coverage by 0.1%.
The diff coverage is 81.13%.

@@                Coverage Diff                @@
##             2.7.0-release   #3182     +/-   ##
=================================================
+ Coverage            63.79%   63.9%   +0.1%     
  Complexity              75      75             
=================================================
  Files                  651     652      +1     
  Lines                28311   28355     +44     
  Branches              4792    4800      +8     
=================================================
+ Hits                 18061   18120     +59     
+ Misses                7980    7964     -16     
- Partials              2270    2271      +1

Impacted Files	Coverage Δ	Complexity Δ
...ng/zookeeper/zkclient/ZkclientZookeeperClient.java	`55% <ø> (ø)`	`0 <0> (ø)`	⬇️
...c/main/java/org/apache/dubbo/common/Constants.java	`92.85% <ø> (ø)`	`0 <0> (ø)`	⬇️
...etadata/support/AbstractMetadataReportFactory.java	`87.5% <ø> (-0.74%)`	`0 <0> (ø)`
...o/remoting/zookeeper/zkclient/ZkClientWrapper.java	`83.63% <ø> (ø)`	`0 <0> (ø)`	⬇️
...g/apache/dubbo/config/AbstractInterfaceConfig.java	`72.18% <ø> (+0.21%)`	`0 <0> (ø)`	⬇️
...ting/zookeeper/curator/CuratorZookeeperClient.java	`67.7% <0%> (+1.04%)`	`0 <0> (ø)`	⬇️
...dubbo/metadata/support/AbstractMetadataReport.java	`69.51% <100%> (+0.16%)`	`0 <0> (ø)`	⬇️
...zookeeper/curator/CuratorZookeeperTransporter.java	`100% <100%> (ø)`	`0 <0> (ø)`	⬇️
...okeeper/zkclient/ZkclientZookeeperTransporter.java	`100% <100%> (ø)`	`0 <0> (ø)`	⬇️
.../org/apache/dubbo/config/MetadataReportConfig.java	`41.02% <33.33%> (-0.65%)`	`0 <0> (ø)`
... and 18 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 347f154...4a10a81. Read the comment docs.

...eeper/src/main/java/org/apache/dubbo/remoting/zookeeper/support/AbstractZookeeperClient.java

...keeper/src/main/java/org/apache/dubbo/remoting/zookeeper/curator/CuratorZookeeperClient.java

.../src/main/java/org/apache/dubbo/remoting/zookeeper/support/AbstractZookeeperTransporter.java

…me code

.../src/main/java/org/apache/dubbo/remoting/zookeeper/support/AbstractZookeeperTransporter.java

…o issue_3179

.../src/main/java/org/apache/dubbo/remoting/zookeeper/support/AbstractZookeeperTransporter.java

...adata-report-api/src/main/java/org/apache/dubbo/metadata/support/AbstractMetadataReport.java

...eport-api/src/main/java/org/apache/dubbo/metadata/support/AbstractMetadataReportFactory.java

dubbo-common/src/main/java/org/apache/dubbo/common/URL.java

...adata-report-api/src/main/java/org/apache/dubbo/metadata/support/AbstractMetadataReport.java

...keeper/src/main/java/org/apache/dubbo/remoting/zookeeper/curator/CuratorZookeeperClient.java

.../src/main/java/org/apache/dubbo/remoting/zookeeper/support/AbstractZookeeperTransporter.java

beiwei30 · 2019-01-13T15:16:33Z

...eper/src/main/java/org/apache/dubbo/remoting/zookeeper/zkclient/ZkclientZookeeperClient.java

@@ -16,18 +16,17 @@
 */
 package org.apache.dubbo.remoting.zookeeper.zkclient;

+import org.I0Itec.zkclient.IZkChildListener;


double check all import sequence. I believe this will break CI. We have import rule configured, 'org.apache.dubbo' should go first.

…o issue_3179

… sharing zookeeper connection, it should judge zookeeperClient.isConnected()

.../src/main/java/org/apache/dubbo/remoting/zookeeper/support/AbstractZookeeperTransporter.java

beiwei30 · 2019-01-14T09:43:30Z

.../src/main/java/org/apache/dubbo/remoting/zookeeper/support/AbstractZookeeperTransporter.java

+        // avoid creating too many connections， so add lock
+        synchronized (zookeeperClientMap) {
+            if ((zookeeperClient = fetchAndUpdateZookeeperClientCache(addressList)) != null && zookeeperClient.isConnected()) {
+                logger.info("Get result from map for the second time when invoking zookeeperTransporter.connnect .");


"find valid zookeeper client from the cache for address: " + addressList.toString()

beiwei30 · 2019-01-14T09:44:19Z

.../src/main/java/org/apache/dubbo/remoting/zookeeper/support/AbstractZookeeperTransporter.java

+            }
+
+            zookeeperClient = createZookeeperClient(createServerURL(url));
+            logger.info("Get result by creating new connection when invoking zookeeperTransporter.connnect .");


"no valid zookeeper client found from cache, therefore create a new client for url " + url.

beiwei30 · 2019-01-14T09:49:15Z

.../src/main/java/org/apache/dubbo/remoting/zookeeper/support/AbstractZookeeperTransporter.java

+     * @param url
+     * @return
+     */
+    URL createServerURL(URL url) {


need a better method name, say: toClientUrl()

beiwei30 · 2019-01-14T09:50:02Z

...ng-zookeeper/src/main/java/org/apache/dubbo/remoting/zookeeper/zkclient/ZkClientWrapper.java

 import org.I0Itec.zkclient.IZkChildListener;
 import org.I0Itec.zkclient.IZkStateListener;
 import org.I0Itec.zkclient.ZkClient;
+import org.apache.dubbo.common.logger.Logger;


import order, check style.

beiwei30 · 2019-01-14T09:51:13Z

this pull request has too many comments already. Pls. submit a separated pull request for the pending comments.

fix apache#3179. Make registry, configcenter, matadata-report share a…

385aab2

… zookeeper connection

cvictory requested review from chickenlj and beiwei30 January 10, 2019 07:28

cvictory added 2 commits January 10, 2019 19:08

fix apache#3179. optimize registry, configcenter, matadata-report sha…

5aa31fa

…re a zookeeper connection

fix apache#3179. add licence

45832a7

khanimteyaz reviewed Jan 10, 2019

View reviewed changes

...eeper/src/main/java/org/apache/dubbo/remoting/zookeeper/support/AbstractZookeeperClient.java Outdated Show resolved Hide resolved

khanimteyaz reviewed Jan 10, 2019

View reviewed changes

...keeper/src/main/java/org/apache/dubbo/remoting/zookeeper/curator/CuratorZookeeperClient.java Outdated Show resolved Hide resolved

khanimteyaz reviewed Jan 10, 2019

View reviewed changes

...keeper/src/main/java/org/apache/dubbo/remoting/zookeeper/curator/CuratorZookeeperClient.java Outdated Show resolved Hide resolved

khanimteyaz reviewed Jan 10, 2019

View reviewed changes

.../src/main/java/org/apache/dubbo/remoting/zookeeper/support/AbstractZookeeperTransporter.java Outdated Show resolved Hide resolved

fix some review issue: just optimize constuctor order and refactor so…

2cfa820

…me code

khanimteyaz reviewed Jan 11, 2019

View reviewed changes

.../src/main/java/org/apache/dubbo/remoting/zookeeper/support/AbstractZookeeperTransporter.java Outdated Show resolved Hide resolved

khanimteyaz reviewed Jan 11, 2019

View reviewed changes

.../src/main/java/org/apache/dubbo/remoting/zookeeper/support/AbstractZookeeperTransporter.java Outdated Show resolved Hide resolved

khanimteyaz reviewed Jan 11, 2019

View reviewed changes

.../src/main/java/org/apache/dubbo/remoting/zookeeper/support/AbstractZookeeperTransporter.java Show resolved Hide resolved

khanimteyaz reviewed Jan 11, 2019

View reviewed changes

.../src/main/java/org/apache/dubbo/remoting/zookeeper/support/AbstractZookeeperTransporter.java Outdated Show resolved Hide resolved

cvictory added 5 commits January 11, 2019 16:30

modify HashMap to ConcurrentHashMap

25f70e7

add some comments

ef9cd30

remove ZookeeperClientData and originalURLs

c8cb545

Merge branch '2.7.0-release' of github.com:apache/incubator-dubbo int…

bf3e6c3

…o issue_3179

remove ZookeeperClientData and originalURLs

7a15b54