From 9d84a2a8c047bbe07ee5cb844df3e443b0f9cbdf Mon Sep 17 00:00:00 2001 From: Nikhilesh Jannu Date: Thu, 27 Nov 2025 20:50:23 -0800 Subject: [PATCH 1/2] @SOLR-18001 fixing the intermittent test failure (https://issues.apache.org/jira/browse/SOLR-18001) --- .../LBHttp2SolrClientIntegrationTest.java | 80 ++++++++++++++++--- 1 file changed, 70 insertions(+), 10 deletions(-) diff --git a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/LBHttp2SolrClientIntegrationTest.java b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/LBHttp2SolrClientIntegrationTest.java index 3f24984f90e..700866caf8d 100644 --- a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/LBHttp2SolrClientIntegrationTest.java +++ b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/LBHttp2SolrClientIntegrationTest.java @@ -110,7 +110,12 @@ private void addDocs(SolrInstance solrInstance) throws IOException, SolrServerEx public void tearDown() throws Exception { for (SolrInstance aSolr : solr) { if (aSolr != null) { - aSolr.tearDown(); + try { + aSolr.tearDown(); + } catch (Exception e) { + // Log but don't fail the test due to teardown issues + log.warn("Exception during tearDown of {}: {}", aSolr.name, e.getMessage()); + } } } super.tearDown(); @@ -159,8 +164,17 @@ public void testSimple() throws Exception { assertEquals(3, names.size()); // Kill a server and test again - solr[1].jetty.stop(); - solr[1].jetty = null; + try { + solr[1].jetty.stop(); + } catch (Exception e) { + log.warn("Exception stopping jetty during test: {}", e.getMessage()); + } finally { + solr[1].jetty = null; + } + + // Give some time for the LB client to detect the server is down + Thread.sleep(100); + names.clear(); for (int i = 0; i < solr.length; i++) { resp = h.lbClient.query(solrQuery); @@ -187,8 +201,17 @@ public void testTwoServers() throws Exception { try (var h = client(baseSolrEndpoints)) { SolrQuery solrQuery = new SolrQuery("*:*"); QueryResponse resp = null; - solr[0].jetty.stop(); - solr[0].jetty = null; + + try { + solr[0].jetty.stop(); + } catch (Exception e) { + log.warn("Exception stopping jetty[0] during test: {}", e.getMessage()); + } finally { + solr[0].jetty = null; + } + + Thread.sleep(100); // Give LB client time to detect server down + resp = h.lbClient.query(solrQuery); String name = resp.getResults().get(0).getFieldValue("name").toString(); assertEquals("solr/collection11", name); @@ -196,8 +219,14 @@ public void testTwoServers() throws Exception { name = resp.getResults().get(0).getFieldValue("name").toString(); assertEquals("solr/collection11", name); - solr[1].jetty.stop(); - solr[1].jetty = null; + try { + solr[1].jetty.stop(); + } catch (Exception e) { + log.warn("Exception stopping jetty[1] during test: {}", e.getMessage()); + } finally { + solr[1].jetty = null; + } + startJettyAndWaitForAliveCheckQuery(solr[0]); resp = h.lbClient.query(solrQuery); @@ -224,6 +253,23 @@ private void startJettyAndWaitForAliveCheckQuery(SolrInstance solrInstance) thro } } + private static void stopJettyWithTimeout(JettySolrRunner jetty, long timeoutMs) throws Exception { + final var future = java.util.concurrent.CompletableFuture.runAsync(() -> { + try { + jetty.stop(); + } catch (Exception e) { + throw new RuntimeException(e); + } + }); + + try { + future.get(timeoutMs, TimeUnit.MILLISECONDS); + } catch (java.util.concurrent.TimeoutException e) { + future.cancel(true); + throw new Exception("Jetty shutdown timed out after " + timeoutMs + "ms", e); + } + } + private static class SolrInstance { String name; Path homeDir; @@ -292,7 +338,16 @@ public void setUp() throws Exception { } public void tearDown() throws Exception { - if (jetty != null) jetty.stop(); + if (jetty != null) { + try { + // Use a timeout wrapper for shutdown + stopJettyWithTimeout(jetty, 5000); + } catch (Exception e) { + log.warn("Exception stopping jetty for {}: {}", name, e.getMessage()); + } finally { + jetty = null; + } + } IOUtils.rm(homeDir); } @@ -326,11 +381,16 @@ private static class LBClientHolder implements AutoCloseable { @Override public void close() { - lbClient.close(); + try { + lbClient.close(); + } catch (Exception e) { + log.warn("Exception closing LB client: {}", e.getMessage()); + } try { delegate.close(); } catch (IOException ioe) { - throw new UncheckedIOException(ioe); + log.warn("Exception closing delegate client: {}", ioe.getMessage()); + // Don't throw exception during cleanup to avoid masking test failures } } } From b435d34925dcac80a1cdedc06b1d432aff2cce5e Mon Sep 17 00:00:00 2001 From: Nikhilesh Jannu Date: Thu, 27 Nov 2025 21:29:06 -0800 Subject: [PATCH 2/2] Fixing the issues reported by ./gradlew check --- .../LBHttp2SolrClientIntegrationTest.java | 61 +++++++++++-------- 1 file changed, 36 insertions(+), 25 deletions(-) diff --git a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/LBHttp2SolrClientIntegrationTest.java b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/LBHttp2SolrClientIntegrationTest.java index 700866caf8d..84a0283ee79 100644 --- a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/LBHttp2SolrClientIntegrationTest.java +++ b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/LBHttp2SolrClientIntegrationTest.java @@ -17,7 +17,6 @@ package org.apache.solr.client.solrj.impl; import java.io.IOException; -import java.io.UncheckedIOException; import java.lang.invoke.MethodHandles; import java.nio.file.Files; import java.nio.file.Path; @@ -35,6 +34,8 @@ import org.apache.solr.client.solrj.response.QueryResponse; import org.apache.solr.client.solrj.response.SolrResponseBase; import org.apache.solr.common.SolrInputDocument; +import org.apache.solr.common.util.ExecutorUtil; +import org.apache.solr.common.util.SolrNamedThreadFactory; import org.apache.solr.embedded.JettyConfig; import org.apache.solr.embedded.JettySolrRunner; import org.apache.solr.util.LogLevel; @@ -114,7 +115,7 @@ public void tearDown() throws Exception { aSolr.tearDown(); } catch (Exception e) { // Log but don't fail the test due to teardown issues - log.warn("Exception during tearDown of {}: {}", aSolr.name, e.getMessage()); + log.warn("Exception during tearDown of {}", aSolr.name, e); } } } @@ -167,14 +168,14 @@ public void testSimple() throws Exception { try { solr[1].jetty.stop(); } catch (Exception e) { - log.warn("Exception stopping jetty during test: {}", e.getMessage()); + log.warn("Exception stopping jetty during test", e); } finally { solr[1].jetty = null; } - + // Give some time for the LB client to detect the server is down Thread.sleep(100); - + names.clear(); for (int i = 0; i < solr.length; i++) { resp = h.lbClient.query(solrQuery); @@ -201,17 +202,17 @@ public void testTwoServers() throws Exception { try (var h = client(baseSolrEndpoints)) { SolrQuery solrQuery = new SolrQuery("*:*"); QueryResponse resp = null; - + try { solr[0].jetty.stop(); } catch (Exception e) { - log.warn("Exception stopping jetty[0] during test: {}", e.getMessage()); + log.warn("Exception stopping jetty[0] during test", e); } finally { solr[0].jetty = null; } - + Thread.sleep(100); // Give LB client time to detect server down - + resp = h.lbClient.query(solrQuery); String name = resp.getResults().get(0).getFieldValue("name").toString(); assertEquals("solr/collection11", name); @@ -222,11 +223,11 @@ public void testTwoServers() throws Exception { try { solr[1].jetty.stop(); } catch (Exception e) { - log.warn("Exception stopping jetty[1] during test: {}", e.getMessage()); + log.warn("Exception stopping jetty[1] during test", e); } finally { solr[1].jetty = null; } - + startJettyAndWaitForAliveCheckQuery(solr[0]); resp = h.lbClient.query(solrQuery); @@ -254,19 +255,29 @@ private void startJettyAndWaitForAliveCheckQuery(SolrInstance solrInstance) thro } private static void stopJettyWithTimeout(JettySolrRunner jetty, long timeoutMs) throws Exception { - final var future = java.util.concurrent.CompletableFuture.runAsync(() -> { + final var executor = + ExecutorUtil.newMDCAwareSingleThreadExecutor(new SolrNamedThreadFactory("jetty-shutdown")); + + try { + final var future = + java.util.concurrent.CompletableFuture.runAsync( + () -> { + try { + jetty.stop(); + } catch (Exception e) { + throw new RuntimeException(e); + } + }, + executor); + try { - jetty.stop(); - } catch (Exception e) { - throw new RuntimeException(e); + future.get(timeoutMs, TimeUnit.MILLISECONDS); + } catch (java.util.concurrent.TimeoutException e) { + future.cancel(true); + throw new Exception("Jetty shutdown timed out after " + timeoutMs + "ms", e); } - }); - - try { - future.get(timeoutMs, TimeUnit.MILLISECONDS); - } catch (java.util.concurrent.TimeoutException e) { - future.cancel(true); - throw new Exception("Jetty shutdown timed out after " + timeoutMs + "ms", e); + } finally { + executor.shutdown(); } } @@ -343,7 +354,7 @@ public void tearDown() throws Exception { // Use a timeout wrapper for shutdown stopJettyWithTimeout(jetty, 5000); } catch (Exception e) { - log.warn("Exception stopping jetty for {}: {}", name, e.getMessage()); + log.warn("Exception stopping jetty for {}", name, e); } finally { jetty = null; } @@ -384,12 +395,12 @@ public void close() { try { lbClient.close(); } catch (Exception e) { - log.warn("Exception closing LB client: {}", e.getMessage()); + log.warn("Exception closing LB client", e); } try { delegate.close(); } catch (IOException ioe) { - log.warn("Exception closing delegate client: {}", ioe.getMessage()); + log.warn("Exception closing delegate client", ioe); // Don't throw exception during cleanup to avoid masking test failures } }