idle timeouts should cause servers to be added to the zombie list

Author: jvanneman

changelog/unreleased/SOLR-18002-add-unresponsive-servers-to-zombie-list.yml

@@ -0,0 +1,8 @@
+# See https://github.com/apache/solr/blob/main/dev-docs/changelog.adoc
+title: Add unresponsive servers to zombie list
+type: fixed # added, changed, fixed, deprecated, removed, dependency_update, security, other
+authors:
+  - name: James Vanneman
+links:
+  - name: SOLR-18002
+    url: https://issues.apache.org/jira/browse/SOLR-18002

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

@@ -21,6 +21,7 @@
 import java.net.SocketException;
 import java.net.SocketTimeoutException;
 import java.util.concurrent.CompletableFuture;
+import java.util.concurrent.TimeoutException;
 import java.util.concurrent.atomic.AtomicReference;
 import org.apache.solr.client.solrj.SolrClient;
 import org.apache.solr.client.solrj.SolrRequest;
@@ -208,7 +209,7 @@ private void onFailedRequest(
       }
     } catch (SolrServerException e) {
       Throwable rootCause = e.getRootCause();
-      if (!isNonRetryable && rootCause instanceof IOException) {
+      if (!isNonRetryable && (rootCause instanceof IOException || rootCause instanceof TimeoutException)) {
         listener.onFailure((!isZombie) ? makeServerAZombie(endpoint, e) : e, true);
       } else if (isNonRetryable && rootCause instanceof ConnectException) {
         listener.onFailure((!isZombie) ? makeServerAZombie(endpoint, e) : e, true);

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

@@ -38,6 +38,7 @@
 import java.util.concurrent.Executors;
 import java.util.concurrent.ScheduledExecutorService;
 import java.util.concurrent.TimeUnit;
+import java.util.concurrent.TimeoutException;
 import java.util.concurrent.atomic.AtomicInteger;
 import java.util.stream.Collectors;
 import org.apache.solr.client.solrj.SolrClient;
@@ -663,7 +664,7 @@ protected Exception doRequest(
       }
     } catch (SolrServerException e) {
       Throwable rootCause = e.getRootCause();
-      if (!isNonRetryable && rootCause instanceof IOException) {
+      if (!isNonRetryable && (rootCause instanceof IOException || rootCause instanceof TimeoutException)) {
         ex = (!isZombie) ? makeServerAZombie(baseUrl, e) : e;
       } else if (isNonRetryable && rootCause instanceof ConnectException) {
         ex = (!isZombie) ? makeServerAZombie(baseUrl, e) : e;

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

@@ -18,6 +18,7 @@
 
 import java.io.IOException;
 import java.io.UncheckedIOException;
+import java.net.ServerSocket;
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.util.ArrayList;
@@ -31,7 +32,9 @@
 import org.apache.solr.client.solrj.SolrClient;
 import org.apache.solr.client.solrj.SolrServerException;
 import org.apache.solr.client.solrj.jetty.HttpJettySolrClient;
+import org.apache.solr.client.solrj.jetty.LBJettySolrClient;
 import org.apache.solr.client.solrj.request.SolrQuery;
+import org.apache.solr.client.solrj.request.QueryRequest;
 import org.apache.solr.client.solrj.response.QueryResponse;
 import org.apache.solr.client.solrj.response.SolrResponseBase;
 import org.apache.solr.common.SolrInputDocument;
@@ -202,6 +205,29 @@ public void testTwoServers() throws Exception {
     }
   }
 
+  public void testTimeoutExceptionMarksServerAsZombie() throws Exception {
+    try (ZombieTestContext ctx = new ZombieTestContext()) {
+      LBSolrClient.Req lbReq = ctx.createQueryRequest();
+
+      try {
+        ctx.lbClient.request(lbReq);
+      } catch (Exception e) {
+      }
+
+      ctx.assertZombieState();
+    }
+  }
+
+  public void testTimeoutExceptionMarksServerAsZombieAsyncRequest() throws Exception {
+    try (ZombieTestContext ctx = new ZombieTestContext()) {
+      LBSolrClient.Req lbReq = ctx.createQueryRequest();
+
+      ctx.lbClient.requestAsync(lbReq).exceptionally(e -> null).get();
+
+      ctx.assertZombieState();
+    }
+  }
+
   private LBSolrClient.Endpoint[] bootstrapBaseSolrEndpoints(int max) {
     LBSolrClient.Endpoint[] solrUrls = new LBSolrClient.Endpoint[max];
     for (int i = 0; i < max; i++) {
@@ -330,4 +356,57 @@ public void close() {
       }
     }
   }
+
+  private class ZombieTestContext implements AutoCloseable {
+    final ServerSocket blackhole;
+    final LBSolrClient.Endpoint nonRoutableEndpoint;
+    final HttpJettySolrClient delegateClient;
+    final LBAsyncSolrClient lbClient;
+
+    ZombieTestContext() throws Exception {
+      //create a socket that allows a client to connect but causes them to hang until idleTimeout is triggered
+      blackhole = new ServerSocket(0);
+      int blackholePort = blackhole.getLocalPort();
+      nonRoutableEndpoint =
+          new LBSolrClient.Endpoint("http://localhost:" + blackholePort + "/solr");
+
+      delegateClient =
+          new HttpJettySolrClient.Builder()
+              .withConnectionTimeout(1000, TimeUnit.MILLISECONDS)
+              .withIdleTimeout(100, TimeUnit.MILLISECONDS)
+              .build();
+
+      lbClient = new LBJettySolrClient.Builder(delegateClient, nonRoutableEndpoint).build();
+    }
+
+    LBSolrClient.Req createQueryRequest() {
+      SolrQuery solrQuery = new SolrQuery("*:*");
+      QueryRequest queryRequest = new QueryRequest(solrQuery);
+
+      List<LBSolrClient.Endpoint> endpoints =
+          List.of(
+              new LBSolrClient.Endpoint(
+                  nonRoutableEndpoint.getBaseUrl(), solr[0].getDefaultCollection())
+              );
+      return new LBSolrClient.Req(queryRequest, endpoints);
+    }
+
+    void assertZombieState() {
+      assertTrue(
+          "Non-routable endpoint should be marked as zombie due to timeout",
+          lbClient.zombieServers.containsKey(
+              nonRoutableEndpoint.getBaseUrl() + "/" + solr[0].getDefaultCollection()));
+    }
+
+    @Override
+    public void close() {
+      lbClient.close();
+      delegateClient.close();
+      try {
+        blackhole.close();
+      } catch (IOException ioe) {
+
+      }
+    }
+  }
 }