narendly commented on a change in pull request #846: Make ZKHelixAdmin and 
ZKHelixManager Realm-aware
URL: https://github.com/apache/helix/pull/846#discussion_r391402054
 
 

 ##########
 File path: 
helix-core/src/main/java/org/apache/helix/manager/zk/ZKHelixManager.java
 ##########
 @@ -1285,4 +1266,60 @@ public void handleSessionEstablishmentError(Throwable 
error) throws Exception {
   public Long getSessionStartTime() {
     return _sessionStartTime;
   }
+
+  private RealmAwareZkClient createSingleRealmZkClient() {
+    final String shardingKey = buildShardingKey();
+    PathBasedZkSerializer zkSerializer =
+        ChainedPathZkSerializer.builder(new ZNRecordSerializer()).build();
+
+    RealmAwareZkClient.RealmAwareZkConnectionConfig connectionConfig =
+        new RealmAwareZkClient.RealmAwareZkConnectionConfig.Builder()
+            .setRealmMode(RealmAwareZkClient.RealmMode.SINGLE_REALM)
+            .setZkRealmShardingKey(shardingKey)
+            .setSessionTimeout(_sessionTimeout).build();
+
+    RealmAwareZkClient.RealmAwareZkClientConfig clientConfig =
+        new RealmAwareZkClient.RealmAwareZkClientConfig();
+
+    clientConfig.setZkSerializer(zkSerializer)
+        .setConnectInitTimeout(_connectionInitTimeout)
+        .setMonitorType(_instanceType.name())
+        .setMonitorKey(_clusterName)
+        .setMonitorInstanceName(_instanceName)
+        .setMonitorRootPathOnly(isMonitorRootPathOnly());
+
+    if (_instanceType == InstanceType.ADMINISTRATOR) {
+      return buildSelectiveZkClient(SharedZkClientFactory.getInstance(), 
connectionConfig,
+          clientConfig);
+    }
+
+    return buildSelectiveZkClient(DedicatedZkClientFactory.getInstance(), 
connectionConfig,
+        clientConfig);
+  }
+
+  private RealmAwareZkClient buildSelectiveZkClient(HelixZkClientFactory 
zkClientFactory,
 
 Review comment:
   My suggestion is
   
   - JavaDoc:
   
   Resolves what type of ZkClient this HelixManager should use based on whether 
MULTI_ZK_ENABLED System config is set or not. Two types of ZkClients are 
available: 1) If MULTI_ZK_ENABLED is set to true, we create a dedicated 
RealmAwareZkClient that provides full ZkClient functionalities and connects to 
the correct ZK by querying MetadataStoreDirectoryService. 2) Otherwise, we 
create a dedicated HelixZkClient which plainly connects to the ZK address given.
   
   - Rename the method to `resolveZkClient()`
   
   I think this will make our intention more clear.

----------------------------------------------------------------
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]

Reply via email to