SOLR-16435: Add Request timeout to Http2SolrClient (#1046)

solr/CHANGES.txt

@@ -32,7 +32,8 @@ Other Changes
 ==================  9.2.0 ==================
 New Features
 ---------------------
-(No changes)
+
+* SOLR-16435: Add Request timeout to Http2SolrClient (Tomás Fernández Löbbe)
 
 Improvements
 ---------------------

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

@@ -128,6 +128,7 @@ public class Http2SolrClient extends SolrClient {
   private HttpClient httpClient;
   private volatile Set<String> queryParams = Collections.emptySet();
   private int idleTimeout;
+  private int requestTimeout;
 
   private ResponseParser parser = new BinaryResponseParser();
   private volatile RequestWriter requestWriter = new BinaryRequestWriter();
@@ -170,6 +171,11 @@ protected Http2SolrClient(String serverBaseUrl, Builder builder) {
     } else {
       basicAuthAuthorizationStr = null;
     }
+    if (builder.requestTimeout == null) {
+      requestTimeout = -1;
+    } else {
+      requestTimeout = builder.requestTimeout;
+    }
     assert ObjectReleaseTracker.track(this);
   }
 
@@ -465,7 +471,7 @@ public NamedList<Object> request(SolrRequest<?> solrRequest, String collection)
       throw new RuntimeException(e);
     } catch (TimeoutException e) {
       throw new SolrServerException(
-          "Timeout occured while waiting response from server at: " + req.getURI(), e);
+          "Timeout occurred while waiting response from server at: " + req.getURI(), e);
     } catch (ExecutionException e) {
       Throwable cause = e.getCause();
       abortCause = cause;
@@ -476,7 +482,7 @@ public NamedList<Object> request(SolrRequest<?> solrRequest, String collection)
         throw (SolrServerException) cause;
       } else if (cause instanceof IOException) {
         throw new SolrServerException(
-            "IOException occured when talking to server at: " + getBaseURL(), cause);
+            "IOException occurred when talking to server at: " + getBaseURL(), cause);
       }
       throw new SolrServerException(cause.getMessage(), cause);
     } catch (SolrServerException | RuntimeException sse) {
@@ -528,7 +534,11 @@ private Request makeRequest(SolrRequest<?> solrRequest, String collection)
 
   private void decorateRequest(Request req, SolrRequest<?> solrRequest) {
     req.header(HttpHeader.ACCEPT_ENCODING, null);
-    req.timeout(idleTimeout, TimeUnit.MILLISECONDS);
+    if (requestTimeout > 0) {
+      req.timeout(requestTimeout, TimeUnit.MILLISECONDS);
+    } else {
+      req.timeout(idleTimeout, TimeUnit.MILLISECONDS);
+    }
     if (solrRequest.getUserPrincipal() != null) {
       req.attribute(REQ_PRINCIPAL_KEY, solrRequest.getUserPrincipal());
     }
@@ -922,6 +932,7 @@ public static class Builder {
     private SSLConfig sslConfig = defaultSSLConfig;
     private Integer idleTimeout;
     private Integer connectionTimeout;
+    private Integer requestTimeout;
     private Integer maxConnectionsPerHost;
     private String basicAuthUser;
     private String basicAuthPassword;
@@ -1022,6 +1033,17 @@ public Builder connectionTimeout(int connectionTimeOut) {
       this.connectionTimeout = connectionTimeOut;
       return this;
     }
+
+    /**
+     * Set a timeout in milliseconds for requests issued by this client.
+     *
+     * @param requestTimeout The timeout in milliseconds
+     * @return this Builder.
+     */
+    public Builder requestTimeout(int requestTimeout) {
+      this.requestTimeout = requestTimeout;
+      return this;
+    }
   }
 
   public Set<String> getQueryParams() {

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

@@ -189,11 +189,11 @@ public void tearDown() throws Exception {
     super.tearDown();
   }
 
-  private Http2SolrClient getHttp2SolrClient(String url, int connectionTimeOut, int socketTimeout) {
+  private Http2SolrClient.Builder getHttp2SolrClient(
+      String url, int connectionTimeOut, int socketTimeout) {
     return new Http2SolrClient.Builder(url)
         .connectionTimeout(connectionTimeOut)
-        .idleTimeout(socketTimeout)
-        .build();
+        .idleTimeout(socketTimeout);
   }
 
   private Http2SolrClient getHttp2SolrClient(String url) {
@@ -205,7 +205,8 @@ public void testTimeout() throws Exception {
     SolrQuery q = new SolrQuery("*:*");
     try (Http2SolrClient client =
         getHttp2SolrClient(
-            jetty.getBaseUrl().toString() + "/slow/foo", DEFAULT_CONNECTION_TIMEOUT, 2000)) {
+                jetty.getBaseUrl().toString() + "/slow/foo", DEFAULT_CONNECTION_TIMEOUT, 2000)
+            .build()) {
       client.query(q, SolrRequest.METHOD.GET);
       fail("No exception thrown.");
     } catch (SolrServerException e) {
@@ -218,14 +219,30 @@ public void test0IdleTimeout() throws Exception {
     SolrQuery q = new SolrQuery("*:*");
     try (Http2SolrClient client =
         getHttp2SolrClient(
-            jetty.getBaseUrl().toString() + "/debug/foo", DEFAULT_CONNECTION_TIMEOUT, 0)) {
+                jetty.getBaseUrl().toString() + "/debug/foo", DEFAULT_CONNECTION_TIMEOUT, 0)
+            .build()) {
       try {
         client.query(q, SolrRequest.METHOD.GET);
       } catch (BaseHttpSolrClient.RemoteSolrException ignored) {
       }
     }
   }
 
+  @Test
+  public void testRequestTimeout() throws Exception {
+    SolrQuery q = new SolrQuery("*:*");
+    try (Http2SolrClient client =
+        getHttp2SolrClient(
+                jetty.getBaseUrl().toString() + "/slow/foo", DEFAULT_CONNECTION_TIMEOUT, 0)
+            .requestTimeout(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 that SolrExceptions thrown by HttpSolrClient can correctly encapsulate http status codes
    * even when not on the list of ErrorCodes solr may return.