SOLR-14667 Make zkClientTimeout consistent and based on a system prop…

…erty
apache · Aug 11, 2023 · 4dc58d8 · 4dc58d8
1 parent 88990d6
commit 4dc58d8
Show file tree

Hide file tree

Showing 20 changed files with 161 additions and 94 deletions.
diff --git a/solr/core/src/java/org/apache/solr/cli/AuthTool.java b/solr/core/src/java/org/apache/solr/cli/AuthTool.java
@@ -34,6 +34,7 @@
 import org.apache.commons.cli.HelpFormatter;
 import org.apache.commons.cli.Option;
 import org.apache.lucene.util.Constants;
+import org.apache.solr.client.solrj.impl.SolrZkClientTimeout;
 import org.apache.solr.common.cloud.SolrZkClient;
 import org.apache.solr.common.util.StrUtils;
 import org.apache.solr.security.Sha256AuthenticationProvider;
@@ -187,7 +188,8 @@ private int handleKerberos(CommandLine cli) throws Exception {
             try (SolrZkClient zkClient =
                 new SolrZkClient.Builder()
                     .withUrl(zkHost)
-                    .withTimeout(10000, TimeUnit.MILLISECONDS)
+                    .withTimeout(
+                        SolrZkClientTimeout.DEFAULT_ZK_CLIENT_TIMEOUT, TimeUnit.MILLISECONDS)
                     .build()) {
               checkSecurityJsonExists(zkClient);
             } catch (Exception ex) {
@@ -206,7 +208,8 @@ private int handleKerberos(CommandLine cli) throws Exception {
             try (SolrZkClient zkClient =
                 new SolrZkClient.Builder()
                     .withUrl(zkHost)
-                    .withTimeout(10000, TimeUnit.MILLISECONDS)
+                    .withTimeout(
+                        SolrZkClientTimeout.DEFAULT_ZK_CLIENT_TIMEOUT, TimeUnit.MILLISECONDS)
                     .build()) {
               zkClient.setData(
                   "/security.json", securityJson.getBytes(StandardCharsets.UTF_8), true);
@@ -321,7 +324,7 @@ private int handleBasicAuth(CommandLine cli) throws Exception {
           try (SolrZkClient zkClient =
               new SolrZkClient.Builder()
                   .withUrl(zkHost)
-                  .withTimeout(10000, TimeUnit.MILLISECONDS)
+                  .withTimeout(SolrZkClientTimeout.DEFAULT_ZK_CLIENT_TIMEOUT, TimeUnit.MILLISECONDS)
                   .build()) {
             checkSecurityJsonExists(zkClient);
           }
@@ -384,7 +387,7 @@ private int handleBasicAuth(CommandLine cli) throws Exception {
           try (SolrZkClient zkClient =
               new SolrZkClient.Builder()
                   .withUrl(zkHost)
-                  .withTimeout(10000, TimeUnit.MILLISECONDS)
+                  .withTimeout(SolrZkClientTimeout.DEFAULT_ZK_CLIENT_TIMEOUT, TimeUnit.MILLISECONDS)
                   .build()) {
             zkClient.setData("/security.json", securityJson.getBytes(StandardCharsets.UTF_8), true);
           }
@@ -479,7 +482,7 @@ private void clearSecurityJson(CommandLine cli, boolean updateIncludeFileOnly) t
       try (SolrZkClient zkClient =
           new SolrZkClient.Builder()
               .withUrl(zkHost)
-              .withTimeout(10000, TimeUnit.MILLISECONDS)
+              .withTimeout(SolrZkClientTimeout.DEFAULT_ZK_CLIENT_TIMEOUT, TimeUnit.MILLISECONDS)
               .build()) {
         zkClient.setData("/security.json", "{}".getBytes(StandardCharsets.UTF_8), true);
       }

diff --git a/solr/core/src/java/org/apache/solr/cli/ConfigSetDownloadTool.java b/solr/core/src/java/org/apache/solr/cli/ConfigSetDownloadTool.java
@@ -25,6 +25,7 @@
 import java.util.concurrent.TimeUnit;
 import org.apache.commons.cli.CommandLine;
 import org.apache.commons.cli.Option;
+import org.apache.solr.client.solrj.impl.SolrZkClientTimeout;
 import org.apache.solr.common.cloud.SolrZkClient;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -79,7 +80,7 @@ public void runImpl(CommandLine cli) throws Exception {
     try (SolrZkClient zkClient =
         new SolrZkClient.Builder()
             .withUrl(zkHost)
-            .withTimeout(30000, TimeUnit.MILLISECONDS)
+            .withTimeout(SolrZkClientTimeout.DEFAULT_ZK_CLIENT_TIMEOUT, TimeUnit.MILLISECONDS)
             .build()) {
       echoIfVerbose("\nConnecting to ZooKeeper at " + zkHost + " ...", cli);
       String confName = cli.getOptionValue("confname");

diff --git a/solr/core/src/java/org/apache/solr/cli/ConfigSetUploadTool.java b/solr/core/src/java/org/apache/solr/cli/ConfigSetUploadTool.java
@@ -23,6 +23,7 @@
 import java.util.concurrent.TimeUnit;
 import org.apache.commons.cli.CommandLine;
 import org.apache.commons.cli.Option;
+import org.apache.solr.client.solrj.impl.SolrZkClientTimeout;
 import org.apache.solr.common.cloud.SolrZkClient;
 import org.apache.solr.common.cloud.ZkMaintenanceUtils;
 import org.apache.solr.core.ConfigSetService;
@@ -86,7 +87,7 @@ public void runImpl(CommandLine cli) throws Exception {
     try (SolrZkClient zkClient =
         new SolrZkClient.Builder()
             .withUrl(zkHost)
-            .withTimeout(30000, TimeUnit.MILLISECONDS)
+            .withTimeout(SolrZkClientTimeout.DEFAULT_ZK_CLIENT_TIMEOUT, TimeUnit.MILLISECONDS)
             .build()) {
       echoIfVerbose("\nConnecting to ZooKeeper at " + zkHost + " ...", cli);
       Path confPath =

diff --git a/solr/core/src/java/org/apache/solr/cli/ZkCpTool.java b/solr/core/src/java/org/apache/solr/cli/ZkCpTool.java
@@ -23,6 +23,7 @@
 import java.util.concurrent.TimeUnit;
 import org.apache.commons.cli.CommandLine;
 import org.apache.commons.cli.Option;
+import org.apache.solr.client.solrj.impl.SolrZkClientTimeout;
 import org.apache.solr.common.cloud.SolrZkClient;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -78,7 +79,7 @@ public void runImpl(CommandLine cli) throws Exception {
     try (SolrZkClient zkClient =
         new SolrZkClient.Builder()
             .withUrl(zkHost)
-            .withTimeout(30000, TimeUnit.MILLISECONDS)
+            .withTimeout(SolrZkClientTimeout.DEFAULT_ZK_CLIENT_TIMEOUT, TimeUnit.MILLISECONDS)
             .build()) {
       echoIfVerbose("\nConnecting to ZooKeeper at " + zkHost + " ...", cli);
       String src = cli.getOptionValue("src");

diff --git a/solr/core/src/java/org/apache/solr/cli/ZkLsTool.java b/solr/core/src/java/org/apache/solr/cli/ZkLsTool.java
@@ -22,6 +22,7 @@
 import java.util.concurrent.TimeUnit;
 import org.apache.commons.cli.CommandLine;
 import org.apache.commons.cli.Option;
+import org.apache.solr.client.solrj.impl.SolrZkClientTimeout;
 import org.apache.solr.common.cloud.SolrZkClient;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -72,7 +73,7 @@ public void runImpl(CommandLine cli) throws Exception {
     try (SolrZkClient zkClient =
         new SolrZkClient.Builder()
             .withUrl(zkHost)
-            .withTimeout(30000, TimeUnit.MILLISECONDS)
+            .withTimeout(SolrZkClientTimeout.DEFAULT_ZK_CLIENT_TIMEOUT, TimeUnit.MILLISECONDS)
             .build()) {
       echoIfVerbose("\nConnecting to ZooKeeper at " + zkHost + " ...", cli);
 

diff --git a/solr/core/src/java/org/apache/solr/cli/ZkMkrootTool.java b/solr/core/src/java/org/apache/solr/cli/ZkMkrootTool.java
@@ -22,6 +22,7 @@
 import java.util.concurrent.TimeUnit;
 import org.apache.commons.cli.CommandLine;
 import org.apache.commons.cli.Option;
+import org.apache.solr.client.solrj.impl.SolrZkClientTimeout;
 import org.apache.solr.common.cloud.SolrZkClient;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -71,7 +72,7 @@ public void runImpl(CommandLine cli) throws Exception {
     try (SolrZkClient zkClient =
         new SolrZkClient.Builder()
             .withUrl(zkHost)
-            .withTimeout(30000, TimeUnit.MILLISECONDS)
+            .withTimeout(SolrZkClientTimeout.DEFAULT_ZK_CLIENT_TIMEOUT, TimeUnit.MILLISECONDS)
             .build()) {
       echoIfVerbose("\nConnecting to ZooKeeper at " + zkHost + " ...", cli);
 

diff --git a/solr/core/src/java/org/apache/solr/cli/ZkMvTool.java b/solr/core/src/java/org/apache/solr/cli/ZkMvTool.java
@@ -24,6 +24,7 @@
 import org.apache.commons.cli.CommandLine;
 import org.apache.commons.cli.Option;
 import org.apache.solr.client.solrj.SolrServerException;
+import org.apache.solr.client.solrj.impl.SolrZkClientTimeout;
 import org.apache.solr.common.cloud.SolrZkClient;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -78,7 +79,7 @@ public void runImpl(CommandLine cli) throws Exception {
     try (SolrZkClient zkClient =
         new SolrZkClient.Builder()
             .withUrl(zkHost)
-            .withTimeout(30000, TimeUnit.MILLISECONDS)
+            .withTimeout(SolrZkClientTimeout.DEFAULT_ZK_CLIENT_TIMEOUT, TimeUnit.MILLISECONDS)
             .build()) {
       echoIfVerbose("\nConnecting to ZooKeeper at " + zkHost + " ...", cli);
       String src = cli.getOptionValue("src");

diff --git a/solr/core/src/java/org/apache/solr/cli/ZkRmTool.java b/solr/core/src/java/org/apache/solr/cli/ZkRmTool.java
@@ -24,6 +24,7 @@
 import org.apache.commons.cli.CommandLine;
 import org.apache.commons.cli.Option;
 import org.apache.solr.client.solrj.SolrServerException;
+import org.apache.solr.client.solrj.impl.SolrZkClientTimeout;
 import org.apache.solr.common.cloud.SolrZkClient;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -84,7 +85,7 @@ public void runImpl(CommandLine cli) throws Exception {
     try (SolrZkClient zkClient =
         new SolrZkClient.Builder()
             .withUrl(zkHost)
-            .withTimeout(30000, TimeUnit.MILLISECONDS)
+            .withTimeout(SolrZkClientTimeout.DEFAULT_ZK_CLIENT_TIMEOUT, TimeUnit.MILLISECONDS)
             .build()) {
       if (!recurse && zkClient.getChildren(znode, null, true).size() != 0) {
         throw new SolrServerException(

diff --git a/solr/core/src/java/org/apache/solr/cloud/ZkCLI.java b/solr/core/src/java/org/apache/solr/cloud/ZkCLI.java
@@ -42,6 +42,7 @@
 import org.apache.commons.cli.ParseException;
 import org.apache.commons.cli.PosixParser;
 import org.apache.solr.cli.CLIO;
+import org.apache.solr.client.solrj.impl.SolrZkClientTimeout;
 import org.apache.solr.common.SolrException;
 import org.apache.solr.common.cloud.ClusterProperties;
 import org.apache.solr.common.cloud.SolrZkClient;
@@ -299,8 +300,9 @@ public static void main(String[] args)
       try (SolrZkClient zkClient =
           new SolrZkClient.Builder()
               .withUrl(zkServerAddress)
-              .withTimeout(30000, TimeUnit.MILLISECONDS)
-              .withConnTimeOut(30000, TimeUnit.MILLISECONDS)
+              .withTimeout(SolrZkClientTimeout.DEFAULT_ZK_CLIENT_TIMEOUT, TimeUnit.MILLISECONDS)
+              .withConnTimeOut(
+                  SolrZkClientTimeout.DEFAULT_ZK_CONNECT_TIMEOUT, TimeUnit.MILLISECONDS)
               .withReconnectListener(() -> {})
               .withCompressor(compressor)
               .build()) {

diff --git a/solr/core/src/java/org/apache/solr/cloud/ZkController.java b/solr/core/src/java/org/apache/solr/cloud/ZkController.java
@@ -62,6 +62,7 @@
 import org.apache.solr.client.solrj.impl.CloudLegacySolrClient;
 import org.apache.solr.client.solrj.impl.HttpSolrClient.Builder;
 import org.apache.solr.client.solrj.impl.SolrClientCloudManager;
+import org.apache.solr.client.solrj.impl.SolrZkClientTimeout;
 import org.apache.solr.client.solrj.impl.ZkClientClusterStateProvider;
 import org.apache.solr.client.solrj.request.CoreAdminRequest.WaitForState;
 import org.apache.solr.cloud.overseer.ClusterStateMutator;
@@ -233,8 +234,6 @@ public String toString() {
 
   private boolean genericCoreNodeNames;
 
-  private int clientTimeout;
-
   private volatile boolean isClosed;
 
   private final ConcurrentHashMap<String, Throwable> replicasMetTragicEvent =
@@ -331,7 +330,7 @@ public ZkController(
     this.leaderVoteWait = cloudConfig.getLeaderVoteWait();
     this.leaderConflictResolveWait = cloudConfig.getLeaderConflictResolveWait();
 
-    this.clientTimeout = cloudConfig.getZkClientTimeout();
+    int clientTimeout = cloudConfig.getZkClientTimeout();
 
     String connectionStrategy = System.getProperty("solr.zookeeper.connectionStrategy");
     ZkClientConnectionStrategy strat =
@@ -1188,8 +1187,8 @@ public static boolean checkChrootPath(String zkHost, boolean create)
     SolrZkClient tmpClient =
         new SolrZkClient.Builder()
             .withUrl(zkHost.substring(0, zkHost.indexOf('/')))
-            .withTimeout(60000, TimeUnit.MILLISECONDS)
-            .withConnTimeOut(30000, TimeUnit.MILLISECONDS)
+            .withTimeout(SolrZkClientTimeout.DEFAULT_ZK_CLIENT_TIMEOUT, TimeUnit.MILLISECONDS)
+            .withConnTimeOut(SolrZkClientTimeout.DEFAULT_ZK_CONNECT_TIMEOUT, TimeUnit.MILLISECONDS)
             .build();
     boolean exists = tmpClient.exists(chrootPath, true);
     if (!exists && create) {
@@ -2339,10 +2338,6 @@ public boolean clearAsyncId(String asyncId) throws KeeperException {
     }
   }
 
-  public int getClientTimeout() {
-    return clientTimeout;
-  }
-
   public Overseer getOverseer() {
     return overseer;
   }

diff --git a/solr/core/src/java/org/apache/solr/core/CloudConfig.java b/solr/core/src/java/org/apache/solr/core/CloudConfig.java
@@ -16,6 +16,7 @@
  */
 package org.apache.solr.core;
 
+import org.apache.solr.client.solrj.impl.SolrZkClientTimeout;
 import org.apache.solr.common.SolrException;
 
 public class CloudConfig {
@@ -179,7 +180,6 @@ public String getStateCompressorClass() {
 
   public static class CloudConfigBuilder {
 
-    private static final int DEFAULT_ZK_CLIENT_TIMEOUT = 45000;
     private static final int DEFAULT_LEADER_VOTE_WAIT = 180000; // 3 minutes
     private static final int DEFAULT_LEADER_CONFLICT_RESOLVE_WAIT = 180000;
     private static final int DEFAULT_CREATE_COLLECTION_ACTIVE_WAIT = 45; // 45 seconds
@@ -188,7 +188,7 @@ public static class CloudConfigBuilder {
         -1; // By default compression for state is disabled
 
     private String zkHost;
-    private int zkClientTimeout = Integer.getInteger("zkClientTimeout", DEFAULT_ZK_CLIENT_TIMEOUT);
+    private int zkClientTimeout = SolrZkClientTimeout.DEFAULT_ZK_CLIENT_TIMEOUT;
     private final int hostPort;
     private final String hostName;
     private boolean useGenericCoreNames;

diff --git a/solr/core/src/java/org/apache/solr/core/ZkContainer.java b/solr/core/src/java/org/apache/solr/core/ZkContainer.java
@@ -29,6 +29,7 @@
 import java.util.function.Predicate;
 import java.util.function.Supplier;
 import java.util.stream.Collectors;
+import org.apache.solr.client.solrj.impl.SolrZkClientTimeout;
 import org.apache.solr.cloud.SolrZkServer;
 import org.apache.solr.cloud.ZkController;
 import org.apache.solr.common.AlreadyClosedException;
@@ -111,7 +112,7 @@ public void initZooKeeper(final CoreContainer cc, CloudConfig config) {
       }
     }
 
-    int zkClientConnectTimeout = 30000;
+    int zkClientConnectTimeout = SolrZkClientTimeout.DEFAULT_ZK_CONNECT_TIMEOUT;
 
     if (zookeeperHost != null) {
 

diff --git a/solr/core/src/java/org/apache/solr/packagemanager/PackageManager.java b/solr/core/src/java/org/apache/solr/packagemanager/PackageManager.java
@@ -42,6 +42,7 @@
 import org.apache.solr.client.solrj.SolrClient;
 import org.apache.solr.client.solrj.SolrRequest;
 import org.apache.solr.client.solrj.SolrServerException;
+import org.apache.solr.client.solrj.impl.SolrZkClientTimeout;
 import org.apache.solr.client.solrj.request.GenericSolrRequest;
 import org.apache.solr.client.solrj.request.V2Request;
 import org.apache.solr.client.solrj.request.beans.PackagePayload;
@@ -83,7 +84,7 @@ public PackageManager(SolrClient solrClient, String solrUrl, String zkHost) {
     this.zkClient =
         new SolrZkClient.Builder()
             .withUrl(zkHost)
-            .withTimeout(30000, TimeUnit.MILLISECONDS)
+            .withTimeout(SolrZkClientTimeout.DEFAULT_ZK_CLIENT_TIMEOUT, TimeUnit.MILLISECONDS)
             .build();
     log.info("Done initializing a zkClient instance...");
   }

diff --git a/solr/core/src/java/org/apache/solr/schema/ManagedIndexSchemaFactory.java b/solr/core/src/java/org/apache/solr/schema/ManagedIndexSchemaFactory.java
@@ -485,7 +485,7 @@ private void zkUgradeToManagedSchema() {
       // Rename the non-managed schema znode in ZooKeeper
       final String nonManagedSchemaPath = zkLoader.getConfigSetZkPath() + "/" + resourceName;
       try {
-        ZkCmdExecutor zkCmdExecutor = new ZkCmdExecutor(zkController.getClientTimeout());
+        ZkCmdExecutor zkCmdExecutor = new ZkCmdExecutor(zkClient.getZkClientTimeout());
         if (zkController.pathExists(nonManagedSchemaPath)) {
           // First, copy the non-managed schema znode content to the upgraded schema znode
           byte[] bytes = zkController.getZkClient().getData(nonManagedSchemaPath, null, null, true);

diff --git a/...rj-zookeeper/src/java/org/apache/solr/client/solrj/impl/ZkClientClusterStateProvider.java b/...rj-zookeeper/src/java/org/apache/solr/client/solrj/impl/ZkClientClusterStateProvider.java
@@ -46,8 +46,8 @@ public class ZkClientClusterStateProvider implements ClusterStateProvider {
   volatile ZkStateReader zkStateReader;
   private boolean closeZkStateReader = true;
   private final String zkHost;
-  private int zkConnectTimeout = 15000;
-  private int zkClientTimeout = 45000;
+  private int zkConnectTimeout = SolrZkClientTimeout.DEFAULT_ZK_CONNECT_TIMEOUT;
+  private int zkClientTimeout = SolrZkClientTimeout.DEFAULT_ZK_CLIENT_TIMEOUT;
 
   private volatile boolean isClosed = false;
 

diff --git a/solr/solrj-zookeeper/src/java/org/apache/solr/common/cloud/SolrZkClient.java b/solr/solrj-zookeeper/src/java/org/apache/solr/common/cloud/SolrZkClient.java
@@ -34,6 +34,7 @@
 import java.util.function.Predicate;
 import java.util.regex.Pattern;
 import org.apache.solr.client.solrj.SolrServerException;
+import org.apache.solr.client.solrj.impl.SolrZkClientTimeout;
 import org.apache.solr.common.MapWriter;
 import org.apache.solr.common.SolrException;
 import org.apache.solr.common.annotation.JsonProperty;
@@ -70,8 +71,6 @@ public class SolrZkClient implements Closeable {
 
   static final String NEWL = System.getProperty("line.separator");
 
-  static final int DEFAULT_CLIENT_CONNECT_TIMEOUT = 30000;
-
   private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
 
   private ConnectionManager connManager;
@@ -1127,8 +1126,8 @@ public NodeData(Stat stat, byte[] data) {
 
   public static class Builder {
     public String zkServerAddress;
-    public int zkClientTimeout = DEFAULT_CLIENT_CONNECT_TIMEOUT;
-    public int zkClientConnectTimeout = DEFAULT_CLIENT_CONNECT_TIMEOUT;
+    public int zkClientTimeout = SolrZkClientTimeout.DEFAULT_ZK_CLIENT_TIMEOUT;
+    public int zkClientConnectTimeout = SolrZkClientTimeout.DEFAULT_ZK_CONNECT_TIMEOUT;
     public OnReconnect onReconnect;
     public BeforeReconnect beforeReconnect;
     public ZkClientConnectionStrategy connectionStrategy;
@@ -1143,13 +1142,25 @@ public Builder withUrl(String server) {
       return this;
     }
 
-    public Builder withTimeout(int i, TimeUnit unit) {
-      this.zkClientTimeout = (int) unit.toMillis(i);
+    /**
+     * Sets the Zk client session timeout
+     *
+     * @param zkClientTimeout timeout value
+     * @param unit time unit
+     */
+    public Builder withTimeout(int zkClientTimeout, TimeUnit unit) {
+      this.zkClientTimeout = Math.toIntExact(unit.toMillis(zkClientTimeout));
       return this;
     }
 
-    public Builder withConnTimeOut(int i, TimeUnit unit) {
-      this.zkClientConnectTimeout = (int) unit.toMillis(i);
+    /**
+     * Sets the Zk connection timeout
+     *
+     * @param zkConnectTimeout timeout value
+     * @param unit time unit
+     */
+    public Builder withConnTimeOut(int zkConnectTimeout, TimeUnit unit) {
+      this.zkClientConnectTimeout = Math.toIntExact(unit.toMillis(zkConnectTimeout));
       return this;
     }