kaisun2000 commented on a change in pull request #770: WIP: Add SharedZkClient
and update SharedZkClientFactory
URL: https://github.com/apache/helix/pull/770#discussion_r380989661
##########
File path:
zookeeper-api/src/main/java/org/apache/helix/zookeeper/impl/factory/SharedZkClientFactory.java
##########
@@ -82,13 +94,8 @@ public HelixZkClient
buildZkClient(HelixZkClient.ZkConnectionConfig connectionCo
throw new ZkClientException("Failed to create a connection manager in
the pool to share.");
}
LOG.info("Sharing ZkConnection {} to a new SharedZkClient.",
connectionConfig.toString());
- return new SharedZkClient(zkConnectionManager, clientConfig,
- new SharedZkClient.OnCloseCallback() {
- @Override
- public void onClose() {
- cleanupConnectionManager(zkConnectionManager);
- }
- });
+ return new InnerSharedZkClient(zkConnectionManager, clientConfig,
Review comment:
nit: the intention is to keeper old sharedZkClient the same, but use new
name innerSharedZkClient. let us don't use the new Java feature to rewrite the
same logic. Just leave it as what it were. It would be easier to people to look
at the history to understand the intention.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
