add zookeeper service discovery support(#3557) #4119

CalebZYC · 2021-11-27T07:41:05Z

What's the purpose of this PR

新增zookeeper服务发现支持

使用方式：

修改build.sh/build.bat，将config-service和admin-service的maven编译命令更改为

mvn clean package -Pgithub,zookeeper-discovery -DskipTests -pl apollo-configservice,apollo-adminservice -am -Dapollo_profile=github,zookeeper-discovery -Dspring_datasource_url=$apollo_config_db_url -Dspring_datasource_username=$apollo_config_db_username -Dspring_datasource_password=$apollo_config_db_password

分别修改apollo-configservice和apollo-adminservice安装包中config目录下的application-github.properties，配置zookeeper服务器地址

spring.cloud.zookeeper.discovery.instance-host=127.0.0.1
spring.cloud.zookeeper.discovery.instance-port=2181

3.本地调试

修改mavenprofiles项,需要在profiles中将zookeeper-discovery项勾选
分别修改apollo-configservice和apollo-adminservice服务对应的VM options,激活zookeeper-discovery的profile

-Dapollo_profile=github,zookeeper-discovery

替换成zookeeper服务注册效果如下:

Which issue(s) this PR fixes:

Fixes #3557

Brief changelog

XXXXX

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

Read the Contributing Guide before making this pull request.
Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
Write necessary unit tests to verify the code.
Run mvn clean test to make sure this pull request doesn't break anything.
Update the CHANGES log.

nobodyiam · 2021-11-30T00:41:55Z

@klboke would you please help to review this pr?

nobodyiam · 2021-12-05T10:59:37Z

pom.xml

+				<apache.zookeeper.version>3.4.14</apache.zookeeper.version>
+				<zookeeper.discovery.version>3.0.4</zookeeper.discovery.version>
+			</properties>
+			<dependencyManagement>


It looks like this dependencyManagement section is not necessary as the zookeeper related dependencies are managed in spring-cloud-zookeeper-dependencies which is imported in spring-cloud-dependencies.

spring-cloud-zookeeper-dependencies用到了curator的依赖，这里引入curator和zookeeper的依赖是考虑到zookeeper版本的兼容性
ZooKeeper Version 3.4.x Compatibility

Thanks for the clarification. Is there any particular reason that we should stick to the zookeeper 3.4.x version?

I have the same problem as nobodyiam about introducing zookeeper related dependencies. Also, I found that the current code running on jdk17, zk-client cannot connect to the local zk-server. the exception is as follows.

2021-12-08 17:39:56.700 WARN 61926 --- [localhost:2181)] org.apache.zookeeper.ClientCnxn : Session 0x0 for server localhost/<unresolved>:2181, unexpected error, closing socket connection and attempting reconnect java.lang.IllegalArgumentException: Unable to canonicalize address localhost/<unresolved>:2181 because it's not resolvable at org.apache.zookeeper.SaslServerPrincipal.getServerPrincipal(SaslServerPrincipal.java:65) at org.apache.zookeeper.SaslServerPrincipal.getServerPrincipal(SaslServerPrincipal.java:41) at org.apache.zookeeper.ClientCnxn$SendThread.startConnect(ClientCnxn.java:1001) at org.apache.zookeeper.ClientCnxn$SendThread.run(ClientCnxn.java:1060)

When I remove the following dependencies.

<profile> <id>zookeeper-discovery</id> <properties> <apache.curator.version>4.2.0</apache.curator.version> <apache.zookeeper.version>3.4.14</apache.zookeeper.version> </properties> <dependencyManagement> <dependencies>  <dependency> <groupId>org.apache.curator</groupId> <artifactId>curator-x-discovery</artifactId> <version>${apache.curator.version}</version> </dependency> <dependency> <groupId>org.apache.curator</groupId> <artifactId>curator-recipes</artifactId> <version>${apache.curator.version}</version> </dependency> <dependency> <groupId>org.apache.curator</groupId> <artifactId>curator-framework</artifactId> <version>${apache.curator.version}</version> </dependency>   <dependency> <groupId>org.apache.zookeeper</groupId> <artifactId>zookeeper</artifactId> <version>${apache.zookeeper.version}</version> <exclusions> <exclusion> <groupId>log4j</groupId> <artifactId>log4j</artifactId> </exclusion> <exclusion> <groupId>org.slf4j</groupId> <artifactId>slf4j-log4j12</artifactId> </exclusion> </exclusions> </dependency>  </dependencies> </dependencyManagement> </profile>

Works well on jdk17

docs/zh/deployment/distributed-deployment-guide.md

apollo-adminservice/src/main/resources/application-zookeeper-discovery.properties

nobodyiam · 2021-12-07T00:54:17Z

pom.xml

+				<apache.zookeeper.version>3.4.14</apache.zookeeper.version>
+				<zookeeper.discovery.version>3.0.4</zookeeper.discovery.version>
+			</properties>
+			<dependencyManagement>


Thanks for the clarification. Is there any particular reason that we should stick to the zookeeper 3.4.x version?

klboke · 2021-12-08T08:56:01Z

pom.xml

+				<apache.zookeeper.version>3.4.14</apache.zookeeper.version>
+				<zookeeper.discovery.version>3.0.4</zookeeper.discovery.version>
+			</properties>
+			<dependencyManagement>


I have the same problem as nobodyiam about introducing zookeeper related dependencies. Also, I found that the current code running on jdk17, zk-client cannot connect to the local zk-server. the exception is as follows.

2021-12-08 17:39:56.700 WARN 61926 --- [localhost:2181)] org.apache.zookeeper.ClientCnxn : Session 0x0 for server localhost/<unresolved>:2181, unexpected error, closing socket connection and attempting reconnect java.lang.IllegalArgumentException: Unable to canonicalize address localhost/<unresolved>:2181 because it's not resolvable at org.apache.zookeeper.SaslServerPrincipal.getServerPrincipal(SaslServerPrincipal.java:65) at org.apache.zookeeper.SaslServerPrincipal.getServerPrincipal(SaslServerPrincipal.java:41) at org.apache.zookeeper.ClientCnxn$SendThread.startConnect(ClientCnxn.java:1001) at org.apache.zookeeper.ClientCnxn$SendThread.run(ClientCnxn.java:1060)

When I remove the following dependencies.

<profile> <id>zookeeper-discovery</id> <properties> <apache.curator.version>4.2.0</apache.curator.version> <apache.zookeeper.version>3.4.14</apache.zookeeper.version> </properties> <dependencyManagement> <dependencies>  <dependency> <groupId>org.apache.curator</groupId> <artifactId>curator-x-discovery</artifactId> <version>${apache.curator.version}</version> </dependency> <dependency> <groupId>org.apache.curator</groupId> <artifactId>curator-recipes</artifactId> <version>${apache.curator.version}</version> </dependency> <dependency> <groupId>org.apache.curator</groupId> <artifactId>curator-framework</artifactId> <version>${apache.curator.version}</version> </dependency>   <dependency> <groupId>org.apache.zookeeper</groupId> <artifactId>zookeeper</artifactId> <version>${apache.zookeeper.version}</version> <exclusions> <exclusion> <groupId>log4j</groupId> <artifactId>log4j</artifactId> </exclusion> <exclusion> <groupId>org.slf4j</groupId> <artifactId>slf4j-log4j12</artifactId> </exclusion> </exclusions> </dependency>  </dependencies> </dependencyManagement> </profile>

Works well on jdk17

apollo-adminservice/pom.xml

docs/zh/deployment/distributed-deployment-guide.md

codecov-commenter · 2021-12-11T07:59:32Z

Codecov Report

Merging #4119 (d963c1e) into master (b2c101a) will decrease coverage by 0.00%.
The diff coverage is 100.00%.

@@             Coverage Diff              @@
##             master    #4119      +/-   ##
============================================
- Coverage     52.60%   52.59%   -0.01%     
+ Complexity     2615     2614       -1     
============================================
  Files           484      484              
  Lines         15190    15190              
  Branches       1572     1572              
============================================
- Hits           7990     7989       -1     
  Misses         6644     6644              
- Partials        556      557       +1

Impacted Files	Coverage Δ
...llo/metaservice/controller/HomePageController.java	`100.00% <ø> (ø)`
...o/metaservice/service/DefaultDiscoveryService.java	`100.00% <ø> (ø)`
...vice/service/SpringCloudInnerDiscoveryService.java	`100.00% <100.00%> (ø)`
...ework/apollo/internals/RemoteConfigRepository.java	`87.73% <0.00%> (-0.62%)`	⬇️

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 b2c101a...d963c1e. Read the comment docs.

nobodyiam

It seems the latest code doesn't work in my environment, see the error messages below.
Java version: 1.8
ZooKeeper server version: 3.4.13

2021-12-13 08:50:46.571  WARN 46365 --- [127.0.0.1:2181)] org.apache.zookeeper.ClientCnxn          : Session 0x10002b21b600002 for sever localhost/127.0.0.1:2181, Closing socket connection. Attempting reconnect except it is a SessionExpiredException.

org.apache.zookeeper.ClientCnxn$EndOfStreamException: Unable to read additional data from server sessionid 0x10002b21b600002, likely server has closed socket
	at org.apache.zookeeper.ClientCnxnSocketNIO.doIO(ClientCnxnSocketNIO.java:75)
	at org.apache.zookeeper.ClientCnxnSocketNIO.doTransport(ClientCnxnSocketNIO.java:348)
	at org.apache.zookeeper.ClientCnxn$SendThread.run(ClientCnxn.java:1262)

2021-12-13 08:50:46.572 ERROR 46365 --- [nStateManager-0] o.a.c.x.d.details.ServiceDiscoveryImpl   : Could not re-register instances after reconnection

org.apache.zookeeper.KeeperException$UnimplementedException: KeeperErrorCode = Unimplemented for /services/apollo-configservice/xx.xx.xx.xx:8080
	at org.apache.zookeeper.KeeperException.create(KeeperException.java:106)
	at org.apache.zookeeper.KeeperException.create(KeeperException.java:54)
	at org.apache.zookeeper.ZooKeeper.create(ZooKeeper.java:1836)
	at org.apache.curator.framework.imps.CreateBuilderImpl$16.call(CreateBuilderImpl.java:1131)
	at org.apache.curator.framework.imps.CreateBuilderImpl$16.call(CreateBuilderImpl.java:1113)
	at org.apache.curator.RetryLoop.callWithRetry(RetryLoop.java:93)
	at org.apache.curator.framework.imps.CreateBuilderImpl.pathInForeground(CreateBuilderImpl.java:1110)
	at org.apache.curator.framework.imps.CreateBuilderImpl.protectedPathInForeground(CreateBuilderImpl.java:593)
	at org.apache.curator.framework.imps.CreateBuilderImpl.forPath(CreateBuilderImpl.java:583)
	at org.apache.curator.framework.imps.CreateBuilderImpl.forPath(CreateBuilderImpl.java:48)
	at org.apache.curator.x.discovery.details.ServiceDiscoveryImpl.internalRegisterService(ServiceDiscoveryImpl.java:237)
	at org.apache.curator.x.discovery.details.ServiceDiscoveryImpl.reRegisterServices(ServiceDiscoveryImpl.java:456)
	at org.apache.curator.x.discovery.details.ServiceDiscoveryImpl.access$100(ServiceDiscoveryImpl.java:59)
	at org.apache.curator.x.discovery.details.ServiceDiscoveryImpl$1.stateChanged(ServiceDiscoveryImpl.java:79)
	at org.apache.curator.framework.state.ConnectionStateManager.lambda$processEvents$0(ConnectionStateManager.java:279)
	at org.apache.curator.framework.listen.MappingListenerManager.lambda$forEach$0(MappingListenerManager.java:92)
	at org.apache.curator.framework.listen.MappingListenerManager.forEach(MappingListenerManager.java:89)
	at org.apache.curator.framework.listen.StandardListenerManager.forEach(StandardListenerManager.java:89)
	at org.apache.curator.framework.state.ConnectionStateManager.processEvents(ConnectionStateManager.java:279)
	at org.apache.curator.framework.state.ConnectionStateManager.access$000(ConnectionStateManager.java:43)
	at org.apache.curator.framework.state.ConnectionStateManager$1.call(ConnectionStateManager.java:132)
	at java.util.concurrent.FutureTask.run(FutureTask.java:266)
	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
	at java.lang.Thread.run(Thread.java:748)

docs/zh/deployment/distributed-deployment-guide.md

nobodyiam

LGTM

add zookeeper service discovery support(apolloconfig#3557)

bb2dc70

nobodyiam requested a review from klboke November 28, 2021 02:18

nobodyiam reviewed Dec 5, 2021

View reviewed changes

docs/zh/deployment/distributed-deployment-guide.md Outdated Show resolved Hide resolved

nobodyiam reviewed Dec 5, 2021

View reviewed changes

apollo-adminservice/src/main/resources/application-zookeeper-discovery.properties Outdated Show resolved Hide resolved

CalebZYC and others added 2 commits December 7, 2021 00:13

Merge branch 'master' into zookeeper-discovery

034f427

fix review

a56a166

nobodyiam reviewed Dec 7, 2021

View reviewed changes

klboke reviewed Dec 8, 2021

View reviewed changes

fix zk dependency

1429ec7

klboke reviewed Dec 9, 2021

View reviewed changes

docs/zh/deployment/distributed-deployment-guide.md Show resolved Hide resolved

CalebZYC and others added 2 commits December 11, 2021 15:47

Merge branch 'master' into zookeeper-discovery

6053499

fix spring cloud inner discovery starter compatibility

1ca9939

nobodyiam requested changes Dec 13, 2021

View reviewed changes

docs/zh/deployment/distributed-deployment-guide.md Outdated Show resolved Hide resolved

CalebZYC and others added 2 commits December 19, 2021 22:51

Merge branch 'master' into zookeeper-discovery

1a59aa2

add supported zk version description

d963c1e

nobodyiam reviewed Dec 20, 2021

View reviewed changes

docs/zh/deployment/distributed-deployment-guide.md Outdated Show resolved Hide resolved

remove local development instructions

d9175ba

nobodyiam approved these changes Dec 21, 2021

View reviewed changes

nobodyiam merged commit a62881d into apolloconfig:master Dec 21, 2021

github-actions bot locked and limited conversation to collaborators Dec 21, 2021

CalebZYC deleted the zookeeper-discovery branch December 21, 2021 12:02

nobodyiam added this to the 2.0.0 milestone Jan 1, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add zookeeper service discovery support(#3557) #4119

add zookeeper service discovery support(#3557) #4119

CalebZYC commented Nov 27, 2021 •

edited

Loading

nobodyiam commented Nov 30, 2021

nobodyiam Dec 5, 2021

CalebZYC Dec 6, 2021 •

edited

Loading

nobodyiam Dec 7, 2021

klboke Dec 8, 2021

nobodyiam Dec 7, 2021

klboke Dec 8, 2021

codecov-commenter commented Dec 11, 2021 •

edited

Loading

nobodyiam left a comment •

edited

Loading

nobodyiam left a comment

add zookeeper service discovery support(#3557) #4119

add zookeeper service discovery support(#3557) #4119

Conversation

CalebZYC commented Nov 27, 2021 • edited Loading

What's the purpose of this PR

Which issue(s) this PR fixes:

Brief changelog

nobodyiam commented Nov 30, 2021

nobodyiam Dec 5, 2021

Choose a reason for hiding this comment

CalebZYC Dec 6, 2021 • edited Loading

Choose a reason for hiding this comment

nobodyiam Dec 7, 2021

Choose a reason for hiding this comment

klboke Dec 8, 2021

Choose a reason for hiding this comment

nobodyiam Dec 7, 2021

Choose a reason for hiding this comment

klboke Dec 8, 2021

Choose a reason for hiding this comment

codecov-commenter commented Dec 11, 2021 • edited Loading

Codecov Report

nobodyiam left a comment • edited Loading

Choose a reason for hiding this comment

nobodyiam left a comment

Choose a reason for hiding this comment

CalebZYC commented Nov 27, 2021 •

edited

Loading

CalebZYC Dec 6, 2021 •

edited

Loading

codecov-commenter commented Dec 11, 2021 •

edited

Loading

nobodyiam left a comment •

edited

Loading