SOLR-17433: Set default http request timeout to infinite

changelog/unreleased/SOLR-17433-changed-default-request-timeout.yml

@@ -0,0 +1,7 @@
+title: Changed default http request timeout to infinite
+type: changed
+authors:
+  - name: Vishnu Priya Chandra Sekar
+links:
+  - name: SOLR-17433
+    url: https://issues.apache.org/jira/browse/SOLR-17433

solr/solrj-jetty/src/test/org/apache/solr/client/solrj/jetty/ConcurrentUpdateJettySolrClientTest.java

@@ -47,7 +47,8 @@ public ConcurrentUpdateBaseSolrClient outcomeCountingConcurrentClient(
   public HttpSolrClient solrClient(Integer overrideIdleTimeoutMs) {
     var builder = new HttpJettySolrClient.Builder();
     if (overrideIdleTimeoutMs != null) {
-      builder.withIdleTimeout(overrideIdleTimeoutMs, TimeUnit.MILLISECONDS);
+      builder
+          .withIdleTimeout(overrideIdleTimeoutMs, TimeUnit.MILLISECONDS);
     }
     return builder.build();
   }

solr/solrj-jetty/src/test/org/apache/solr/client/solrj/jetty/HttpJettySolrClientTest.java

@@ -671,10 +671,7 @@ public void testBuilder() {
   public void testIdleTimeoutWithHttpClient() throws Exception {
     String url = solrTestRule.getBaseUrl() + SLOW_STREAM_SERVLET_PATH;
     try (var oldClient =
-        new HttpJettySolrClient.Builder(url)
-            .withRequestTimeout(Long.MAX_VALUE, TimeUnit.MILLISECONDS)
-            .withIdleTimeout(100, TimeUnit.MILLISECONDS)
-            .build()) {
+        new HttpJettySolrClient.Builder(url).withIdleTimeout(100, TimeUnit.MILLISECONDS).build()) {
 
       try (var onlyBaseUrlChangedClient =
           new HttpJettySolrClient.Builder(url).withHttpClient(oldClient).build()) {

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

@@ -448,7 +448,10 @@ private synchronized boolean maybeTryHeadRequestSync(URI uriNoQueryParams) {
   }
 
   private void decorateRequest(HttpRequest.Builder reqb, SolrRequest<?> solrRequest) {
-    reqb.timeout(Duration.of(requestTimeoutMillis, ChronoUnit.MILLIS));
+    // JDK does not allow non-positive value for request timeout.
+    if (requestTimeoutMillis > 0) {
+      reqb.timeout(Duration.of(requestTimeoutMillis, ChronoUnit.MILLIS));
+    }
 
     reqb.header("User-Agent", USER_AGENT);
     setBasicAuthHeader(solrRequest, reqb);

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

@@ -580,10 +580,16 @@ public B withRequestTimeout(long requestTimeout, TimeUnit unit) {
       return (B) this;
     }
 
+    /**
+     * Returns the request timeout in milliseconds.
+     *
+     * <p>If no request timeout is configured or if the configured value is non-positive, this
+     * method returns {@code 0}, indicating that requests will wait infinitely for a response.
+     *
+     * @return request timeout in milliseconds or {@code 0} if no valid timeout is configured.
+     */
     public long getRequestTimeoutMillis() {
-      return requestTimeoutMillis != null && requestTimeoutMillis > 0
-          ? requestTimeoutMillis
-          : getIdleTimeoutMillis();
+      return requestTimeoutMillis != null && requestTimeoutMillis > 0 ? requestTimeoutMillis : 0;
     }
 
     /**

solr/solrj/src/test/org/apache/solr/client/solrj/impl/ConcurrentUpdateJdkSolrClientTest.java

@@ -28,7 +28,11 @@ public HttpSolrClient solrClient(Integer overrideIdleTimeoutMs) {
     var builder =
         new HttpJdkSolrClient.Builder().withSSLContext(MockTrustManager.ALL_TRUSTING_SSL_CONTEXT);
     if (overrideIdleTimeoutMs != null) {
-      builder.withIdleTimeout(overrideIdleTimeoutMs, TimeUnit.MILLISECONDS);
+      builder
+          .withIdleTimeout(overrideIdleTimeoutMs, TimeUnit.MILLISECONDS)
+          // override the infinite request timeout with idle timeout to ensure idle requests times
+          // out.
+          .withRequestTimeout(overrideIdleTimeoutMs, TimeUnit.MILLISECONDS);
     }
     return builder.build();
   }

solr/solrj/src/test/org/apache/solr/client/solrj/impl/HttpJdkSolrClientTest.java

@@ -175,6 +175,8 @@ public void testRequestWithBaseUrl() throws Exception {
       client.requestWithBaseUrl(intendedUrl, new QueryRequest(q, SolrRequest.METHOD.GET), null);
       assertEquals(
           client.getParser().getWriterType(), DebugServlet.parameters.get(CommonParams.WT)[0]);
+      // verify whether the default request timeout is infinite.
+      assertEquals(0, client.requestTimeoutMillis);
     }
   }
 
@@ -206,19 +208,6 @@ public void testAsyncException() throws Exception {
     super.testAsyncExceptionBase();
   }
 
-  @Test
-  public void testTimeout() throws Exception {
-    SolrQuery q = new SolrQuery("*:*");
-    try (HttpJdkSolrClient client =
-        (HttpJdkSolrClient)
-            builder(solrTestRule.getBaseUrl() + SLOW_SERVLET_PATH, 500, 500).build()) {
-      client.query(q, SolrRequest.METHOD.GET);
-      fail("No exception thrown.");
-    } catch (SolrServerException e) {
-      assertTrue(e.getMessage().contains("timeout") || e.getMessage().contains("Timeout"));
-    }
-  }
-
   @Test
   public void test0IdleTimeout() throws Exception {
     SolrQuery q = new SolrQuery("*:*");

solr/solrj/src/test/org/apache/solr/client/solrj/impl/LB2SolrClientTest.java

@@ -405,6 +405,9 @@ private class TimeoutZombieTestContext implements AutoCloseable {
           new HttpJettySolrClient.Builder()
               .withConnectionTimeout(1000, TimeUnit.MILLISECONDS)
               .withIdleTimeout(1, TimeUnit.MILLISECONDS)
+              // override the infinite request timeout with idle timeout to ensure idle requests
+              // times out.
+              .withRequestTimeout(1, TimeUnit.MILLISECONDS)
               .build();
 
       lbClient = new LBJettySolrClient.Builder(delegateClient, nonRoutableEndpoint).build();