risdenk commented on code in PR #760:
URL: https://github.com/apache/solr/pull/760#discussion_r1346588451


##########
solr/solrj/src/java/org/apache/solr/common/cloud/SolrZkClient.java:
##########
@@ -147,125 +154,93 @@ public SolrZkClient(
       String zkServerAddress,
       int zkClientTimeout,
       int clientConnectTimeout,
-      ZkClientConnectionStrategy strat,
+      ZkCredentialsProvider zkCredentialsProvider,
       final OnReconnect onReconnect,
-      BeforeReconnect beforeReconnect) {
+      OnDisconnect onDisconnect) {
     this(
         zkServerAddress,
         zkClientTimeout,
         clientConnectTimeout,
-        strat,
-        onReconnect,
-        beforeReconnect,
+        zkCredentialsProvider,
         null,
+        onReconnect,
+        onDisconnect,
         null);
   }
 
   public SolrZkClient(
       String zkServerAddress,
       int zkClientTimeout,
       int clientConnectTimeout,
-      ZkClientConnectionStrategy strat,
+      ZkCredentialsProvider zkCredentialsProvider,
+      ACLProvider aclProvider,
       final OnReconnect onReconnect,
-      BeforeReconnect beforeReconnect,
-      ZkACLProvider zkACLProvider,
+      OnDisconnect onDisconnect,
       IsClosed higherLevelIsClosed) {
     this.zkServerAddress = zkServerAddress;
-    this.higherLevelIsClosed = higherLevelIsClosed;
-    if (strat == null) {
-      strat = new DefaultConnectionStrategy();
+    String chroot, zkHost;
+    int chrootIndex = zkServerAddress.indexOf('/');
+    if (chrootIndex == -1) {
+      zkHost = zkServerAddress;
+      chroot = null;
+    } else if (chrootIndex == zkServerAddress.length() - 1) {
+      zkHost = zkServerAddress.substring(0, zkServerAddress.length() - 1);
+      chroot = null;
+    } else {
+      zkHost = zkServerAddress.substring(0, chrootIndex);
+      chroot = zkServerAddress.substring(chrootIndex + 1);
     }
-    this.zkClientConnectionStrategy = strat;
+    this.higherLevelIsClosed = higherLevelIsClosed;
 
-    if (!strat.hasZkCredentialsToAddAutomatically()) {
-      ZkCredentialsProvider zkCredentialsToAddAutomatically =
-          createZkCredentialsToAddAutomatically();
-      
strat.setZkCredentialsToAddAutomatically(zkCredentialsToAddAutomatically);
+    if (zkCredentialsProvider == null) {
+      zkCredentialsProvider = createZkCredentialsToAddAutomatically();
+    }
+    if (aclProvider == null) {
+      aclProvider = createACLProvider();
+    }
+    if (chroot != null && aclProvider instanceof SecurityAwareZkACLProvider) {
+      this.aclProvider = 
((SecurityAwareZkACLProvider)aclProvider).withChroot(chroot);
+    } else {
+      this.aclProvider = aclProvider;
     }
 
     this.zkClientTimeout = zkClientTimeout;
-    // we must retry at least as long as the session timeout
-    zkCmdExecutor =
-        new ZkCmdExecutor(
-            zkClientTimeout,
-            new IsClosed() {
-
-              @Override
-              public boolean isClosed() {
-                return SolrZkClient.this.isClosed();
-              }
-            });
-    connManager =
-        new ConnectionManager(
-            "ZooKeeperConnection Watcher:" + zkServerAddress,
-            this,
-            zkServerAddress,
-            strat,
-            onReconnect,
-            beforeReconnect,
-            new IsClosed() {
-
-              @Override
-              public boolean isClosed() {
-                return SolrZkClient.this.isClosed();
-              }
-            });
 
-    try {
-      strat.connect(
-          zkServerAddress,
-          zkClientTimeout,
-          wrapWatcher(connManager),
-          zooKeeper -> {
-            SolrZooKeeper oldKeeper = keeper;
-            keeper = zooKeeper;
-            try {
-              closeKeeper(oldKeeper);
-            } finally {
-              if (isClosed) {
-                // we may have been closed
-                closeKeeper(SolrZkClient.this.keeper);
-              }
-            }
-          });
-    } catch (Exception e) {
-      connManager.close();
-      if (keeper != null) {
-        try {
-          keeper.close();
-        } catch (InterruptedException e1) {
-          Thread.currentThread().interrupt();
-        }
-      }
-      throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, e);
-    }
+    RetryPolicy retryPolicy = new ExponentialBackoffRetry(1000, 3);
+    var clientBuilder = CuratorFrameworkFactory.builder()
+        .ensembleProvider(new FixedEnsembleProvider(zkHost))
+        .namespace(chroot)
+        .sessionTimeoutMs(zkClientTimeout)
+        .connectionTimeoutMs(clientConnectTimeout)
+        .aclProvider(this.aclProvider)
+        .authorization(zkCredentialsProvider.getCredentials())
+        .retryPolicy(retryPolicy);
 
+    client = clientBuilder.build();

Review Comment:
   FWIW https://github.com/apache/solr/pull/1743 used this runSafeService stuff 
for the hadoop-auth changes. I think it might only come into play when there is 
an external zkClient provided but good to know either way.



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

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to