SOLR-17776: Harmonize SolrJ timeouts (#3357)

SolrJ: If Http2SolrClient.Builder.withHttpClient is used, (and it's used more in 9.8, shifted from deprecated Apache HttpClient  based clients), settings affecting HttpClient construction cannot be customized; an IllegalStateException will now be thrown if you try.
The connection timeout cannot be customized, but idle & request timeouts can be.  Callers (various places in Solr) no longer customize the connection timeout.

The HttpJdkSolrClient wasn't setting the connection timeout as per the builder configuration.

Co-authored-by: Luke Kot-Zaniewski <lkotzaniewsk@bloomberg.net>

Author: dsmiley

solr/CHANGES.txt

@@ -250,6 +250,13 @@ Improvements
 * SOLR-16951: Add PKI Auth Caching for both generation of the PKI Auth Tokens and validation of received tokens.
   The default PKI Auth validity TTL has been increased from 5 seconds to 10 seconds. (Houston Putman)
 
+* SOLR-17776: SolrJ: If Http2SolrClient.Builder.withHttpClient is used, (and it's used more in 9.8, shifted from deprecated Apache HttpClient
+  based clients), settings affecting HttpClient construction cannot be customized; an IllegalStateException will now be thrown if you try.
+  The connection timeout cannot be customized, but idle & request timeouts can be.  Callers (various places in Solr) no longer customize the connection timeout.
+  (David Smiley, Luke Kot-Zaniewski)
+
+* SOLR-17776: The HttpJdkSolrClient wasn't setting the connection timeout as per the builder configuration. (David Smiley)
+
 Optimizations
 ---------------------
 * SOLR-17578: Remove ZkController internal core supplier, for slightly faster reconnection after Zookeeper session loss. (Pierre Salagnac)

solr/core/src/java/org/apache/solr/cloud/Overseer.java

@@ -862,7 +862,6 @@ private void doCompatCheck(BiConsumer<String, Object> consumer) {
             new Http2SolrClient.Builder()
                 .withHttpClient(getCoreContainer().getDefaultHttpSolrClient())
                 .withIdleTimeout(30000, TimeUnit.MILLISECONDS)
-                .withConnectionTimeout(15000, TimeUnit.MILLISECONDS)
                 .build();
         var client =
             new CloudHttp2SolrClient.Builder(

solr/core/src/java/org/apache/solr/cloud/SyncStrategy.java

@@ -360,7 +360,6 @@ private void requestRecovery(
           try (SolrClient client =
               new Http2SolrClient.Builder(baseUrl)
                   .withHttpClient(solrClient)
-                  .withConnectionTimeout(30000, TimeUnit.MILLISECONDS)
                   .withIdleTimeout(120000, TimeUnit.MILLISECONDS)
                   .build()) {
             client.request(recoverRequestCmd);

solr/core/src/java/org/apache/solr/handler/IndexFetcher.java

@@ -179,8 +179,6 @@ public class IndexFetcher {
 
   private final Http2SolrClient solrClient;
 
-  private Integer connTimeout;
-
   private Integer soTimeout;
 
   private boolean skipCommitOnLeaderVersionZero = true;
@@ -258,7 +256,6 @@ private Http2SolrClient createSolrClient(
         .withHttpClient(updateShardHandler.getRecoveryOnlyHttpClient())
         .withBasicAuthCredentials(httpBasicAuthUser, httpBasicAuthPassword)
         .withIdleTimeout(soTimeout, TimeUnit.MILLISECONDS)
-        .withConnectionTimeout(connTimeout, TimeUnit.MILLISECONDS)
         .build();
   }
 
@@ -287,7 +284,6 @@ public IndexFetcher(
     String compress = (String) initArgs.get(COMPRESSION);
     useInternalCompression = ReplicationHandler.INTERNAL.equals(compress);
     useExternalCompression = ReplicationHandler.EXTERNAL.equals(compress);
-    connTimeout = getParameter(initArgs, HttpClientUtil.PROP_CONNECTION_TIMEOUT, 30000, null);
     soTimeout = getParameter(initArgs, HttpClientUtil.PROP_SO_TIMEOUT, 120000, null);
 
     String httpBasicAuthUser = (String) initArgs.get(HttpClientUtil.PROP_BASIC_AUTH_USER);

solr/solr-ref-guide/modules/upgrade-notes/pages/major-changes-in-solr-9.adoc

@@ -67,6 +67,14 @@ It is always strongly recommended that you fully reindex your documents after a
 In Solr 8, it was possible to add docValues to a schema without re-indexing via `UninvertDocValuesMergePolicy`, an advanced/expert utility.
 Due to changes in Lucene 9, that isn't possible any more.
 
+== Solr 9.9
+
+=== SolrJ
+
+If Http2SolrClient.Builder.withHttpClient is used, (and it's used more in 9.8, shifted from deprecated Apache HttpClient based clients), settings affecting HttpClient construction cannot be customized; an IllegalStateException will now be thrown if you try.
+The connection timeout cannot be customized, but idle & request timeouts can be.
+Callers (various places in Solr) no longer customize the connection timeout.
+
 == Solr 9.8.1
 === Configuration
 

solr/solrj-streaming/src/java/org/apache/solr/client/solrj/io/SolrClientCache.java

@@ -205,19 +205,14 @@ private static Http2SolrClient.Builder newHttp2SolrClientBuilder(
                 .withDefaultCollection(URLUtil.extractCoreFromCoreUrl(url));
     if (http2SolrClient != null) {
       builder.withHttpClient(http2SolrClient);
+      // cannot set connection timeout
+    } else {
+      builder.withConnectionTimeout(minConnTimeout, TimeUnit.MILLISECONDS);
     }
+    builder.withIdleTimeout(
+        Math.max(minSocketTimeout, builder.getIdleTimeoutMillis()), TimeUnit.MILLISECONDS);
     builder.withOptionalBasicAuthCredentials(basicAuthCredentials);
 
-    long idleTimeout = minSocketTimeout;
-    if (builder.getIdleTimeoutMillis() != null) {
-      idleTimeout = Math.max(idleTimeout, builder.getIdleTimeoutMillis());
-    }
-    builder.withIdleTimeout(idleTimeout, TimeUnit.MILLISECONDS);
-    long connTimeout = minConnTimeout;
-    if (builder.getConnectionTimeout() != null) {
-      connTimeout = Math.max(idleTimeout, builder.getConnectionTimeout());
-    }
-    builder.withConnectionTimeout(connTimeout, TimeUnit.MILLISECONDS);
     return builder;
   }
 

solr/solrj/src/java/org/apache/solr/client/solrj/impl/ConcurrentUpdateHttp2SolrClient.java

@@ -255,6 +255,7 @@ void sendUpdateStream() throws Exception {
               responseListener = out.getResponseListener();
             }
 
+            // just wait for the headers, so the idle timeout is sensible
             Response response =
                 responseListener.get(client.getIdleTimeout(), TimeUnit.MILLISECONDS);
             rspBody = responseListener.getInputStream();