kaisun2000 commented on a change in pull request #770: WIP: Add SharedZkClient 
and update SharedZkClientFactory
URL: https://github.com/apache/helix/pull/770#discussion_r380961009
 
 

 ##########
 File path: 
zookeeper-api/src/main/java/org/apache/helix/zookeeper/impl/client/SharedZkClient.java
 ##########
 @@ -37,79 +50,541 @@
  * HelixZkClient that uses shared ZkConnection.
  * A SharedZkClient won't manipulate the shared ZkConnection directly.
  */
-public class SharedZkClient extends ZkClient implements HelixZkClient {
+public class SharedZkClient implements RealmAwareZkClient {
   private static Logger LOG = LoggerFactory.getLogger(SharedZkClient.class);
-  /*
-   * Since we cannot really disconnect the ZkConnection, we need a dummy 
ZkConnection placeholder.
-   * This is to ensure connection field is never null even the shared 
RealmAwareZkClient instance is closed so as to avoid NPE.
-   */
-  private final static ZkConnection IDLE_CONNECTION = new 
ZkConnection("Dummy_ZkServers");
-  private final OnCloseCallback _onCloseCallback;
-  private final ZkConnectionManager _connectionManager;
-
-  public interface OnCloseCallback {
-    /**
-     * Triggered after the RealmAwareZkClient is closed.
-     */
-    void onClose();
-  }
-
-  /**
-   * Construct a shared RealmAwareZkClient that uses a shared ZkConnection.
-   *
-   * @param connectionManager     The manager of the shared ZkConnection.
-   * @param clientConfig          ZkClientConfig details to create the shared 
RealmAwareZkClient.
-   * @param callback              Clean up logic when the shared 
RealmAwareZkClient is closed.
-   */
-  public SharedZkClient(ZkConnectionManager connectionManager, ZkClientConfig 
clientConfig,
-      OnCloseCallback callback) {
-    super(connectionManager.getConnection(), 0, 
clientConfig.getOperationRetryTimeout(),
-        clientConfig.getZkSerializer(), clientConfig.getMonitorType(), 
clientConfig.getMonitorKey(),
-        clientConfig.getMonitorInstanceName(), 
clientConfig.isMonitorRootPathOnly());
-    _connectionManager = connectionManager;
-    // Register to the base dedicated RealmAwareZkClient
-    _connectionManager.registerWatcher(this);
-    _onCloseCallback = callback;
+
+  private final HelixZkClient _innerSharedZkClient;
+  private final Map<String, String> _routingDataCache; // TODO: replace with 
RoutingDataCache
+  private final String _zkRealmShardingKey;
+
+  public SharedZkClient(RealmAwareZkClient.RealmAwareZkConnectionConfig 
connectionConfig,
+      RealmAwareZkClient.RealmAwareZkClientConfig clientConfig,
+      Map<String, String> routingDataCache) {
+
+    if (connectionConfig == null) {
+      throw new IllegalArgumentException("RealmAwareZkConnectionConfig cannot 
be null!");
+    }
+    _zkRealmShardingKey = connectionConfig.getZkRealmShardingKey();
+
+    // TODO: Replace this Map with a real RoutingDataCache
+    if (routingDataCache == null) {
+      throw new IllegalArgumentException("RoutingDataCache cannot be null!");
+    }
+    _routingDataCache = routingDataCache;
+
+    // Get the ZkRealm address based on the ZK path sharding key
+    String zkRealmAddress = _routingDataCache.get(_zkRealmShardingKey);
+
+    // Create an InnerSharedZkClient to actually serve ZK requests
+    // TODO: Rename HelixZkClient in the future or remove it entirely - this 
will be a backward-compatibility breaking change because HelixZkClient is being 
used by Helix users.
+    HelixZkClient.ZkConnectionConfig zkConnectionConfig =
+        new HelixZkClient.ZkConnectionConfig(zkRealmAddress)
+            .setSessionTimeout(connectionConfig.getSessionTimeout());
+    _innerSharedZkClient = SharedZkClientFactory.getInstance()
 
 Review comment:
   This actually implies the ZKRealmAware SharedZKClient are mixed with the 
original SharedZKCLient (now called innerSharedZKClient). We should note this 
behavior. 
   
   Put it another way, underlying the same ZkConnection manager, there can be 
innersharedZkClient which do not check path validity and new SharedZkClient 
which does check path validity. 

----------------------------------------------------------------
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:
us...@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscr...@helix.apache.org
For additional commands, e-mail: reviews-h...@helix.apache.org

Reply via email to