From eba410ebb3376fe5cabb164d50bb35303a52bdbc Mon Sep 17 00:00:00 2001 From: chan-dx Date: Fri, 28 Aug 2026 17:16:57 +0100 Subject: [PATCH 01/15] SOLR-18402: SolrJ transports classify their own failures --- .../apache/solr/client/solrj/SolrClient.java | 17 ++++++++++++ .../client/solrj/impl/CloudSolrClient.java | 3 ++- .../client/solrj/impl/HttpJdkSolrClient.java | 8 ++++++ .../client/solrj/impl/HttpSolrClient.java | 17 ++++++++++++ .../solrj/impl/HttpJdkSolrClientTest.java | 26 +++++++++++++++++++ 5 files changed, 70 insertions(+), 1 deletion(-) diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/SolrClient.java b/solr/solrj/src/java/org/apache/solr/client/solrj/SolrClient.java index e83ce93f7a5b..9e3b9d2a1aaa 100644 --- a/solr/solrj/src/java/org/apache/solr/client/solrj/SolrClient.java +++ b/solr/solrj/src/java/org/apache/solr/client/solrj/SolrClient.java @@ -1194,6 +1194,23 @@ public final NamedList request(final SolrRequest request) return request(request, null); } + /** + * Whether the failure proves the request never reached the server, making a replay safe even when + * the request isn't idempotent. Only the transport can answer this; the default is {@code false}, + * meaning "cannot tell" rather than "the request was sent". + */ + public boolean wasRequestUnsent(Throwable t) { + return false; + } + + /** + * Whether this is a transport-level communication failure rather than a response from the server. + * Implementations must keep {@link #wasRequestUnsent} a subset of this. + */ + public boolean wasCommError(Throwable t) { + return false; + } + /** * This method defines the context in which this Solr client is being used (e.g. for internal * communication between Solr nodes or as an external client). The default value is {@code diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudSolrClient.java b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudSolrClient.java index f5a250fb2e8f..6c9ce5643df7 100644 --- a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudSolrClient.java +++ b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudSolrClient.java @@ -210,7 +210,8 @@ public ClusterState getClusterState() { * Is this a communication error? We will retry if so. The whole cause chain is inspected, since a * transport may report the underlying failure wrapped at any depth. */ - protected boolean wasCommError(Throwable t) { + @Override + public boolean wasCommError(Throwable t) { return SolrException.hasCause(t, SocketException.class) || SolrException.hasCause(t, UnknownHostException.class) || SolrException.hasCause(t, RequestNotSentException.class); diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpJdkSolrClient.java b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpJdkSolrClient.java index 722b241cef6d..c539d86bc37b 100644 --- a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpJdkSolrClient.java +++ b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpJdkSolrClient.java @@ -29,6 +29,7 @@ import java.net.URI; import java.net.URISyntaxException; import java.net.http.HttpClient; +import java.net.http.HttpConnectTimeoutException; import java.net.http.HttpRequest; import java.net.http.HttpResponse; import java.net.http.HttpTimeoutException; @@ -229,6 +230,13 @@ public NamedList request(SolrRequest solrRequest, String collection) return requestWithBaseUrl(null, solrRequest, collection); } + /** A connect timeout means the connection was never established, so nothing was written. */ + @Override + public boolean wasRequestUnsent(Throwable t) { + return super.wasRequestUnsent(t) + || SolrException.hasCause(t, HttpConnectTimeoutException.class); + } + protected PreparedRequest prepareRequest( String overrideBaseUrl, SolrRequest solrRequest, String collection) throws SolrServerException, IOException { diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpSolrClient.java b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpSolrClient.java index fccc4357fcde..8f1e8766c2b6 100644 --- a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpSolrClient.java +++ b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpSolrClient.java @@ -22,7 +22,10 @@ import java.io.InputStream; import java.lang.invoke.MethodHandles; import java.lang.reflect.Constructor; +import java.net.ConnectException; import java.net.MalformedURLException; +import java.net.SocketException; +import java.net.UnknownHostException; import java.nio.charset.Charset; import java.nio.charset.StandardCharsets; import java.util.Base64; @@ -36,6 +39,7 @@ import java.util.function.BiConsumer; import java.util.function.Function; import org.apache.solr.client.solrj.RemoteSolrException; +import org.apache.solr.client.solrj.RequestNotSentException; import org.apache.solr.client.solrj.SolrClient; import org.apache.solr.client.solrj.SolrRequest; import org.apache.solr.client.solrj.SolrServerException; @@ -367,6 +371,19 @@ public Set getUrlParamNames() { return urlParamNames; } + @Override + public boolean wasRequestUnsent(Throwable t) { + return SolrException.hasCause(t, RequestNotSentException.class) + || SolrException.hasCause(t, ConnectException.class); + } + + @Override + public boolean wasCommError(Throwable t) { + return SolrException.hasCause(t, SocketException.class) + || SolrException.hasCause(t, UnknownHostException.class) + || wasRequestUnsent(t); + } + /** * @lucene.internal */ diff --git a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/HttpJdkSolrClientTest.java b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/HttpJdkSolrClientTest.java index 9f9233f375e8..eb2b3640c6ba 100644 --- a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/HttpJdkSolrClientTest.java +++ b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/HttpJdkSolrClientTest.java @@ -18,13 +18,17 @@ package org.apache.solr.client.solrj.impl; import java.io.IOException; +import java.net.ConnectException; import java.net.CookieHandler; import java.net.CookieManager; import java.net.ServerSocket; import java.net.Socket; +import java.net.SocketException; import java.net.URI; import java.net.URISyntaxException; +import java.net.UnknownHostException; import java.net.http.HttpClient; +import java.net.http.HttpConnectTimeoutException; import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; @@ -41,6 +45,7 @@ import org.apache.lucene.util.NamedThreadFactory; import org.apache.solr.client.api.util.SolrVersion; import org.apache.solr.client.solrj.RemoteSolrException; +import org.apache.solr.client.solrj.RequestNotSentException; import org.apache.solr.client.solrj.SolrRequest; import org.apache.solr.client.solrj.SolrServerException; import org.apache.solr.client.solrj.request.JavaBinRequestWriter; @@ -735,6 +740,27 @@ private HttpJdkSolrClient.Builder builder(String url) { return builder(url, DEFAULT_CONNECTION_TIMEOUT, DEFAULT_CONNECTION_TIMEOUT); } + @Test + public void testErrorClassification() throws Exception { + String url = solrTestRule.getBaseUrl() + DEBUG_SERVLET_PATH; + try (HttpJdkSolrClient client = builder(url).build()) { + IOException unsent = + new RequestNotSentException("Broken pipe", new IOException("Broken pipe")); + assertTrue(client.wasRequestUnsent(new SolrServerException("wrapped", unsent))); + assertTrue(client.wasRequestUnsent(new ConnectException("Connection refused"))); + assertTrue(client.wasRequestUnsent(new HttpConnectTimeoutException("timed out"))); + + // A bare IOException may have been sent and applied, so it is never proof of the contrary. + assertFalse(client.wasRequestUnsent(new IOException("Broken pipe"))); + assertFalse(client.wasRequestUnsent(new UnknownHostException("nosuchhost"))); + + assertTrue(client.wasCommError(new UnknownHostException("nosuchhost"))); + assertTrue(client.wasCommError(new SocketException("Connection reset"))); + assertTrue(client.wasCommError(new HttpConnectTimeoutException("timed out"))); + assertFalse(client.wasCommError(new IOException("Broken pipe"))); + } + } + private byte[] javabinResponse() { String[] str = JAVABIN_STR.split(" "); byte[] bytes = new byte[str.length]; From b55044babe6ac8a10cad6290919d418b7f3dc3e1 Mon Sep 17 00:00:00 2001 From: chan-dx Date: Fri, 28 Aug 2026 17:18:51 +0100 Subject: [PATCH 02/15] SOLR-18402: CloudSolrClient asks its transport if it was a comm error --- .../solr/client/solrj/impl/CloudSolrClient.java | 17 +++++++---------- .../solrj/impl/CloudSolrClientCacheTest.java | 2 +- 2 files changed, 8 insertions(+), 11 deletions(-) diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudSolrClient.java b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudSolrClient.java index 6c9ce5643df7..88d5808ca327 100644 --- a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudSolrClient.java +++ b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudSolrClient.java @@ -21,8 +21,6 @@ import java.io.IOException; import java.lang.invoke.MethodHandles; -import java.net.SocketException; -import java.net.UnknownHostException; import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; @@ -48,7 +46,6 @@ import java.util.concurrent.locks.ReentrantLock; import java.util.function.Supplier; import java.util.stream.Collectors; -import org.apache.solr.client.solrj.RequestNotSentException; import org.apache.solr.client.solrj.SolrClient; import org.apache.solr.client.solrj.SolrRequest; import org.apache.solr.client.solrj.SolrRequest.SolrRequestType; @@ -206,15 +203,15 @@ public ClusterState getClusterState() { return getClusterStateProvider().getClusterState(); } - /** - * Is this a communication error? We will retry if so. The whole cause chain is inspected, since a - * transport may report the underlying failure wrapped at any depth. - */ + /** Is this a communication error? We will retry if so. Answered by the underlying transport. */ @Override public boolean wasCommError(Throwable t) { - return SolrException.hasCause(t, SocketException.class) - || SolrException.hasCause(t, UnknownHostException.class) - || SolrException.hasCause(t, RequestNotSentException.class); + return getHttpClient().wasCommError(t); + } + + @Override + public boolean wasRequestUnsent(Throwable t) { + return getHttpClient().wasRequestUnsent(t); } @Override diff --git a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudSolrClientCacheTest.java b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudSolrClientCacheTest.java index 0b671fb81d40..4b8217750c6c 100644 --- a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudSolrClientCacheTest.java +++ b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudSolrClientCacheTest.java @@ -433,7 +433,7 @@ public ClusterStateProvider getClusterStateProvider() { @Override public HttpSolrClient getHttpClient() { - throw new UnsupportedOperationException(); + return mock(HttpSolrClient.class); } @FunctionalInterface From 18fcce96ee72b29237689e5a728f64556ee9c67b Mon Sep 17 00:00:00 2001 From: chan-dx Date: Fri, 28 Aug 2026 17:20:49 +0100 Subject: [PATCH 03/15] SOLR-18402: LBSolrClient asks the transport instead of matching class names --- .../client/solrj/impl/LBAsyncSolrClient.java | 10 +++------- .../solr/client/solrj/impl/LBSolrClient.java | 18 ++---------------- .../impl/LBSolrClientRetryUnsentTest.java | 7 +++++++ 3 files changed, 12 insertions(+), 23 deletions(-) diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/LBAsyncSolrClient.java b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/LBAsyncSolrClient.java index 88657cbca149..5d9d59ee0f08 100644 --- a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/LBAsyncSolrClient.java +++ b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/LBAsyncSolrClient.java @@ -17,7 +17,6 @@ package org.apache.solr.client.solrj.impl; import java.io.IOException; -import java.net.ConnectException; import java.net.SocketException; import java.net.SocketTimeoutException; import java.util.concurrent.CompletableFuture; @@ -25,7 +24,6 @@ import java.util.concurrent.TimeoutException; import java.util.concurrent.atomic.AtomicReference; import org.apache.solr.client.solrj.RemoteSolrException; -import org.apache.solr.client.solrj.RequestNotSentException; import org.apache.solr.client.solrj.SolrClient; import org.apache.solr.client.solrj.SolrRequest; import org.apache.solr.client.solrj.SolrRequest.SolrRequestType; @@ -203,7 +201,7 @@ private void onFailedRequest( listener.onFailure(e, false); } } catch (SocketException e) { - if (!isNonRetryable || e instanceof ConnectException) { + if (!isNonRetryable || getClient(endpoint).wasRequestUnsent(e)) { listener.onFailure((!isZombie) ? makeServerAZombie(endpoint, e) : e, true); } else { listener.onFailure(e, false); @@ -219,9 +217,7 @@ private void onFailedRequest( if (!isNonRetryable && (rootCause instanceof IOException || rootCause instanceof TimeoutException)) { listener.onFailure((!isZombie) ? makeServerAZombie(endpoint, e) : e, true); - } else if (isNonRetryable - && (isConnectException(rootCause) - || SolrException.hasCause(e, RequestNotSentException.class))) { + } else if (isNonRetryable && getClient(endpoint).wasRequestUnsent(e)) { // Nothing of the request reached the server, so replaying it elsewhere is safe even though // it isn't idempotent. listener.onFailure((!isZombie) ? makeServerAZombie(endpoint, e) : e, true); @@ -229,7 +225,7 @@ private void onFailedRequest( listener.onFailure(e, false); } } catch (IOException e) { - if (!isNonRetryable || isConnectException(e) || e instanceof RequestNotSentException) { + if (!isNonRetryable || getClient(endpoint).wasRequestUnsent(e)) { listener.onFailure((!isZombie) ? makeServerAZombie(endpoint, e) : e, true); } else { listener.onFailure(e, false); diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/LBSolrClient.java b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/LBSolrClient.java index 4a9d63cd375d..2265637cea3a 100644 --- a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/LBSolrClient.java +++ b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/LBSolrClient.java @@ -20,10 +20,8 @@ import java.io.IOException; import java.lang.invoke.MethodHandles; import java.lang.ref.WeakReference; -import java.net.ConnectException; import java.net.SocketException; import java.net.SocketTimeoutException; -import java.net.http.HttpConnectTimeoutException; import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; @@ -43,7 +41,6 @@ import java.util.concurrent.atomic.AtomicInteger; import java.util.stream.Collectors; import org.apache.solr.client.solrj.RemoteSolrException; -import org.apache.solr.client.solrj.RequestNotSentException; import org.apache.solr.client.solrj.SolrClient; import org.apache.solr.client.solrj.SolrRequest; import org.apache.solr.client.solrj.SolrRequest.SolrRequestType; @@ -656,7 +653,7 @@ protected Exception doRequest( throw e; } } catch (SocketException e) { - if (!isNonRetryable || e instanceof ConnectException) { + if (!isNonRetryable || getClient(baseUrl).wasRequestUnsent(e)) { ex = (!isZombie) ? makeServerAZombie(baseUrl, e) : e; } else { throw e; @@ -672,9 +669,7 @@ protected Exception doRequest( if (!isNonRetryable && (rootCause instanceof IOException || rootCause instanceof TimeoutException)) { ex = (!isZombie) ? makeServerAZombie(baseUrl, e) : e; - } else if (isNonRetryable - && (isConnectException(rootCause) - || SolrException.hasCause(e, RequestNotSentException.class))) { + } else if (isNonRetryable && getClient(baseUrl).wasRequestUnsent(e)) { // Nothing of the request reached the server, so replaying it elsewhere is safe even though // it isn't idempotent. ex = (!isZombie) ? makeServerAZombie(baseUrl, e) : e; @@ -688,15 +683,6 @@ protected Exception doRequest( return ex; } - protected boolean isConnectException(Throwable t) { - if (t instanceof ConnectException || t instanceof HttpConnectTimeoutException) { - return true; - } - // Check for common connection timeout exceptions by name to avoid hard dependencies on - // specific HTTP client libraries (e.g., Jetty or Apache HttpClient). - return t != null && t.getClass().getName().endsWith("ConnectTimeoutException"); - } - protected abstract SolrClient getClient(Endpoint endpoint); private void startAliveCheckExecutor() { diff --git a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/LBSolrClientRetryUnsentTest.java b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/LBSolrClientRetryUnsentTest.java index 3163616e2722..2347918df291 100644 --- a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/LBSolrClientRetryUnsentTest.java +++ b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/LBSolrClientRetryUnsentTest.java @@ -27,6 +27,7 @@ import org.apache.solr.client.solrj.SolrServerException; import org.apache.solr.client.solrj.request.QueryRequest; import org.apache.solr.client.solrj.request.UpdateRequest; +import org.apache.solr.common.SolrException; import org.apache.solr.common.util.NamedList; import org.junit.Test; @@ -55,6 +56,12 @@ private static class FailFirstEndpoint extends LBSolrClient { @Override protected SolrClient getClient(Endpoint endpoint) { return new SolrClient() { + // Stands in for a transport; the LB asks it rather than inspecting the exception itself. + @Override + public boolean wasRequestUnsent(Throwable t) { + return SolrException.hasCause(t, RequestNotSentException.class); + } + @Override public NamedList request(SolrRequest request, String collection) throws SolrServerException, IOException { From 1100f0341f9cce07506c724bad5c21c110829f79 Mon Sep 17 00:00:00 2001 From: chan-dx Date: Fri, 28 Aug 2026 17:36:10 +0100 Subject: [PATCH 04/15] SOLR-18402: CloudSolrClient only replays an update the transport proves unsent --- .../client/solrj/impl/CloudSolrClient.java | 7 ++- .../solrj/impl/CloudSolrClientCacheTest.java | 46 ++++++++++++++++++- 2 files changed, 51 insertions(+), 2 deletions(-) diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudSolrClient.java b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudSolrClient.java index 88d5808ca327..87852615b9ab 100644 --- a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudSolrClient.java +++ b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudSolrClient.java @@ -717,6 +717,10 @@ protected NamedList requestWithRetryOnStaleState( : SolrException.ErrorCode.UNKNOWN.code; final boolean wasCommError = wasCommError(exc); + // An update may already have been applied; only replay one the transport proves never + // arrived. + final boolean mayReplay = + request.getRequestType() != SolrRequestType.UPDATE || wasRequestUnsent(exc); if (wasCommError || (exc instanceof RouteException @@ -748,7 +752,8 @@ protected NamedList requestWithRetryOnStaleState( } } } - if (retryCount < MAX_STALE_RETRIES) { // if it is a communication error , we must try again + // if it is a communication error , we must try again + if (mayReplay && retryCount < MAX_STALE_RETRIES) { // may be, we have a stale version of the collection state, // and we could not get any information from the server // it is probably not worth trying again and again because diff --git a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudSolrClientCacheTest.java b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudSolrClientCacheTest.java index 4b8217750c6c..e6319992f6a1 100644 --- a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudSolrClientCacheTest.java +++ b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudSolrClientCacheTest.java @@ -123,7 +123,7 @@ protected LBSolrClient createOrGetLbClient(HttpSolrClient myClient) { return new ConnectException("TEST"); } if (i == 2) { - return new SocketException("TEST"); + return new ConnectException("TEST"); } if (i == 3) { return new ConnectException("TEST"); @@ -140,6 +140,50 @@ protected LBSolrClient createOrGetLbClient(HttpSolrClient myClient) { } } + /** + * An update may already have been applied by the time a communication error surfaces, so it is + * replayed only when the transport proves the request never arrived. + */ + public void testUpdateIsNotReplayedWhenItMayHaveBeenApplied() throws Exception { + String collName = "gettingstarted"; + Set livenodes = new HashSet<>(); + Map refs = new HashMap<>(); + Map colls = new HashMap<>(); + + Map> responses = new HashMap<>(); + LBJettySolrClient mockLbclient = getMockLbHttpSolrClient(responses); + AtomicInteger lbhttpRequestCount = new AtomicInteger(); + try (ClusterStateProvider clusterStateProvider = getStateProvider(livenodes, refs); + CloudSolrClient cloudClient = + new RandomizingCloudSolrClientBuilder(clusterStateProvider) { + @Override + protected LBSolrClient createOrGetLbClient(HttpSolrClient myClient) { + return mockLbclient; + } + }.build()) { + livenodes.addAll(Set.of("192.168.1.108:7574_solr", "192.168.1.108:8983_solr")); + refs.put(collName, new ClusterState.CollectionRef(loadCollection(collName, 1))); + colls.put(collName, loadCollection(collName, 1)); + + // Not a ConnectException: the transport cannot prove this request never left. + responses.put( + "request", + o -> { + lbhttpRequestCount.incrementAndGet(); + return new SocketException("TEST"); + }); + UpdateRequest update = new UpdateRequest().add("id", "123", "desc", "Something 0"); + + // The routing randomization decides whether the failure arrives wrapped, so match the cause. + Exception thrown = expectThrows(Exception.class, () -> cloudClient.request(update, collName)); + assertTrue( + "the transport's failure must reach the caller", + SolrException.hasCause(thrown, SocketException.class)); + assertEquals( + "an update that may have been applied must not be replayed", 1, lbhttpRequestCount.get()); + } + } + public void testStaleStateRetrySkipsStateVersionBeforeWait() throws Exception { String collName = "gettingstarted"; Set liveNodes = new HashSet<>(Set.of("192.168.1.108:8983_solr")); From ae653891bee58911214e8d04a3680a6ec18c6ee3 Mon Sep 17 00:00:00 2001 From: chan-dx Date: Fri, 28 Aug 2026 17:39:34 +0100 Subject: [PATCH 05/15] SOLR-18402: HttpJettySolrClient classifies its own transport failures --- .../solrj/jetty/HttpJettySolrClient.java | 10 +++++++ .../solrj/jetty/HttpJettySolrClientTest.java | 26 +++++++++++++++++++ 2 files changed, 36 insertions(+) diff --git a/solr/solrj-jetty/src/java/org/apache/solr/client/solrj/jetty/HttpJettySolrClient.java b/solr/solrj-jetty/src/java/org/apache/solr/client/solrj/jetty/HttpJettySolrClient.java index 77e1f7e3f1ad..f8fa7f958d87 100644 --- a/solr/solrj-jetty/src/java/org/apache/solr/client/solrj/jetty/HttpJettySolrClient.java +++ b/solr/solrj-jetty/src/java/org/apache/solr/client/solrj/jetty/HttpJettySolrClient.java @@ -22,6 +22,7 @@ import java.lang.invoke.MethodHandles; import java.lang.reflect.InvocationTargetException; import java.net.ConnectException; +import java.nio.channels.ClosedChannelException; import java.util.ArrayList; import java.util.Collection; import java.util.Iterator; @@ -50,6 +51,7 @@ import org.apache.solr.client.solrj.request.RequestWriter; import org.apache.solr.client.solrj.response.ResponseParser; import org.apache.solr.client.solrj.util.ClientUtils; +import org.apache.solr.common.SolrException; import org.apache.solr.common.params.CommonParams; import org.apache.solr.common.params.ModifiableSolrParams; import org.apache.solr.common.util.ContentStream; @@ -85,6 +87,7 @@ import org.eclipse.jetty.http2.client.HTTP2Client; import org.eclipse.jetty.http2.client.transport.HttpClientTransportOverHTTP2; import org.eclipse.jetty.io.ClientConnector; +import org.eclipse.jetty.io.EofException; import org.eclipse.jetty.util.ssl.KeyStoreScanner; import org.eclipse.jetty.util.ssl.SslContextFactory; import org.slf4j.Logger; @@ -568,6 +571,13 @@ public R requestWithBaseUrl( } } + @Override + public boolean wasCommError(Throwable t) { + return super.wasCommError(t) + || SolrException.hasCause(t, EofException.class) + || SolrException.hasCause(t, ClosedChannelException.class); + } + @Override protected LBSolrClient createLBSolrClient() { return new LBJettySolrClient.Builder(this).build(); diff --git a/solr/solrj-jetty/src/test/org/apache/solr/client/solrj/jetty/HttpJettySolrClientTest.java b/solr/solrj-jetty/src/test/org/apache/solr/client/solrj/jetty/HttpJettySolrClientTest.java index 11bf3e3c85a2..a9b613f133a0 100644 --- a/solr/solrj-jetty/src/test/org/apache/solr/client/solrj/jetty/HttpJettySolrClientTest.java +++ b/solr/solrj-jetty/src/test/org/apache/solr/client/solrj/jetty/HttpJettySolrClientTest.java @@ -22,14 +22,17 @@ import java.io.IOException; import java.io.InputStream; +import java.nio.channels.ClosedChannelException; import java.nio.charset.StandardCharsets; import java.util.Base64; +import java.util.List; import java.util.concurrent.TimeUnit; import java.util.stream.Collectors; import java.util.stream.IntStream; import org.apache.lucene.tests.util.LuceneTestCase; import org.apache.solr.client.api.util.SolrVersion; import org.apache.solr.client.solrj.RemoteSolrException; +import org.apache.solr.client.solrj.RequestNotSentException; import org.apache.solr.client.solrj.SolrClient; import org.apache.solr.client.solrj.SolrRequest; import org.apache.solr.client.solrj.SolrServerException; @@ -53,6 +56,7 @@ import org.apache.solr.util.ServletFixtures.DebugServlet; import org.eclipse.jetty.client.WWWAuthenticationProtocolHandler; import org.eclipse.jetty.http.HttpStatus; +import org.eclipse.jetty.io.EofException; import org.eclipse.jetty.util.ssl.SslContextFactory; import org.junit.Test; @@ -743,6 +747,28 @@ public void testRequestTimeoutWithHttpClient() throws Exception { } } + @Test + public void testErrorClassification() throws Exception { + String url = solrTestRule.getBaseUrl() + DEBUG_SERVLET_PATH; + try (HttpJettySolrClient client = + (HttpJettySolrClient) builder(url, DEFAULT_CONNECTION_TIMEOUT, 0).build()) { + IOException unsent = + new RequestNotSentException("Broken pipe", new IOException("Broken pipe")); + assertTrue(client.wasRequestUnsent(new SolrServerException("wrapped", unsent))); + assertTrue(client.wasCommError(new SolrServerException("wrapped", unsent))); + + // Jetty's own connection-lost types are communication errors, but they say nothing about + // whether the request was delivered, so they must never claim it was unsent. + for (Throwable lost : + List.of(new EofException("Connection reset by peer"), new ClosedChannelException())) { + assertTrue(lost.getClass().getName(), client.wasCommError(lost)); + assertFalse(lost.getClass().getName(), client.wasRequestUnsent(lost)); + } + + assertFalse(client.wasCommError(new IOException("Broken pipe"))); + } + } + private static void assertIsTimeout(Throwable t) { assertThat(t.getMessage(), containsStringIgnoringCase("Timeout")); } From 9a9085357fdea4440544c4bbe1492ee0c31d34ed Mon Sep 17 00:00:00 2001 From: chan-dx Date: Fri, 28 Aug 2026 17:40:27 +0100 Subject: [PATCH 06/15] SOLR-18402: Update changelog --- .../SOLR-18402-consolidate-retry-classification.yml | 10 ++++++++++ 1 file changed, 10 insertions(+) create mode 100644 changelog/unreleased/SOLR-18402-consolidate-retry-classification.yml diff --git a/changelog/unreleased/SOLR-18402-consolidate-retry-classification.yml b/changelog/unreleased/SOLR-18402-consolidate-retry-classification.yml new file mode 100644 index 000000000000..73a7c38c08de --- /dev/null +++ b/changelog/unreleased/SOLR-18402-consolidate-retry-classification.yml @@ -0,0 +1,10 @@ +title: > + SolrJ: transports now classify their own failures via SolrClient.wasRequestUnsent / + wasCommError; CloudSolrClient and LBSolrClient ask instead of matching exception types. + CloudSolrClient now replays an update only when the transport proves it was never sent. +type: changed +authors: + - name: Han Chan +links: + - name: SOLR-18402 + url: https://issues.apache.org/jira/browse/SOLR-18402 From 29c17d025f6e1767363100b60ec0dae4a380310c Mon Sep 17 00:00:00 2001 From: chan-dx Date: Sat, 29 Aug 2026 19:26:13 +0100 Subject: [PATCH 07/15] SOLR-18402: LBSolrClient: fail over on a bare IOException, as the async client does --- ...18402-consolidate-retry-classification.yml | 3 +- .../solr/client/solrj/impl/LBSolrClient.java | 7 ++++ .../impl/LBSolrClientRetryUnsentTest.java | 35 +++++++++++++++++++ 3 files changed, 44 insertions(+), 1 deletion(-) diff --git a/changelog/unreleased/SOLR-18402-consolidate-retry-classification.yml b/changelog/unreleased/SOLR-18402-consolidate-retry-classification.yml index 73a7c38c08de..ec7167ca29bf 100644 --- a/changelog/unreleased/SOLR-18402-consolidate-retry-classification.yml +++ b/changelog/unreleased/SOLR-18402-consolidate-retry-classification.yml @@ -1,7 +1,8 @@ title: > SolrJ: transports now classify their own failures via SolrClient.wasRequestUnsent / wasCommError; CloudSolrClient and LBSolrClient ask instead of matching exception types. - CloudSolrClient now replays an update only when the transport proves it was never sent. + CloudSolrClient now replays an update only when the transport proves it was never sent, and + LBSolrClient fails over on a bare IOException instead of aborting. type: changed authors: - name: Han Chan diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/LBSolrClient.java b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/LBSolrClient.java index 2265637cea3a..526fd47a676b 100644 --- a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/LBSolrClient.java +++ b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/LBSolrClient.java @@ -676,6 +676,13 @@ protected Exception doRequest( } else { throw e; } + } catch (IOException e) { + // A transport may throw one directly rather than wrapping it in a SolrServerException. + if (!isNonRetryable || getClient(baseUrl).wasRequestUnsent(e)) { + ex = (!isZombie) ? makeServerAZombie(baseUrl, e) : e; + } else { + throw e; + } } catch (Exception e) { throw new SolrServerException(e); } diff --git a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/LBSolrClientRetryUnsentTest.java b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/LBSolrClientRetryUnsentTest.java index 2347918df291..572b0e8bd0d9 100644 --- a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/LBSolrClientRetryUnsentTest.java +++ b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/LBSolrClientRetryUnsentTest.java @@ -126,4 +126,39 @@ public void testQueryIsStillRetriedOnAnyIOException() throws Exception { List.of(DEAD_HOST_1.getBaseUrl(), DEAD_HOST_2.getBaseUrl()), requestReturningAttemptedUrls(maybeSentException(), new QueryRequest())); } + + /** + * A transport may throw an {@link IOException} directly rather than wrapping it in a {@link + * SolrServerException}, as HttpJdkSolrClient does. LBAsyncSolrClient has always handled that; the + * synchronous path used to let it reach the catch-all and abort with no failover. + */ + @Test + public void testQueryIsRetriedOnBareIOException() throws Exception { + assertEquals( + List.of(DEAD_HOST_1.getBaseUrl(), DEAD_HOST_2.getBaseUrl()), + requestReturningAttemptedUrls(new IOException("Broken pipe"), new QueryRequest())); + } + + @Test + public void testUpdateIsNotRetriedOnBareIOException() { + LBSolrClient.Req req = + new LBSolrClient.Req(new UpdateRequest().add("id", "1"), List.of(DEAD_HOST_1, DEAD_HOST_2)); + try (FailFirstEndpoint client = new FailFirstEndpoint(new IOException("Broken pipe"))) { + expectThrows(IOException.class, () -> client.request(req)); + assertEquals(List.of(DEAD_HOST_1.getBaseUrl()), client.attempted); + } + } + + /** + * Parity with LBAsyncSolrClient, which already retried a bare {@link RequestNotSentException}. + */ + @Test + public void testUpdateIsRetriedOnBareRequestNotSentException() throws Exception { + IOException onTheWire = new IOException("Broken pipe"); + assertEquals( + List.of(DEAD_HOST_1.getBaseUrl(), DEAD_HOST_2.getBaseUrl()), + requestReturningAttemptedUrls( + new RequestNotSentException(onTheWire.getMessage(), onTheWire), + new UpdateRequest().add("id", "1"))); + } } From 5d646f657bfcc8ceda584c3d9e3675081a60f274 Mon Sep 17 00:00:00 2001 From: chan-dx Date: Sat, 29 Aug 2026 19:29:23 +0100 Subject: [PATCH 08/15] SOLR-18402: Move failure classification tests off the Jetty & Jdk fixture Collect both transports' cases in one SolrTestCase against a dead URL, so the classification is checked without booting anything. The negative cases are the point: a bare IOException and a post-commit EofException are communication failures that prove nothing about delivery. --- .../solrj/jetty/HttpJettySolrClientTest.java | 26 ---- .../solrj/impl/HttpJdkSolrClientTest.java | 26 ---- .../SolrClientErrorClassificationTest.java | 123 ++++++++++++++++++ 3 files changed, 123 insertions(+), 52 deletions(-) create mode 100644 solr/solrj/src/test/org/apache/solr/client/solrj/impl/SolrClientErrorClassificationTest.java diff --git a/solr/solrj-jetty/src/test/org/apache/solr/client/solrj/jetty/HttpJettySolrClientTest.java b/solr/solrj-jetty/src/test/org/apache/solr/client/solrj/jetty/HttpJettySolrClientTest.java index a9b613f133a0..11bf3e3c85a2 100644 --- a/solr/solrj-jetty/src/test/org/apache/solr/client/solrj/jetty/HttpJettySolrClientTest.java +++ b/solr/solrj-jetty/src/test/org/apache/solr/client/solrj/jetty/HttpJettySolrClientTest.java @@ -22,17 +22,14 @@ import java.io.IOException; import java.io.InputStream; -import java.nio.channels.ClosedChannelException; import java.nio.charset.StandardCharsets; import java.util.Base64; -import java.util.List; import java.util.concurrent.TimeUnit; import java.util.stream.Collectors; import java.util.stream.IntStream; import org.apache.lucene.tests.util.LuceneTestCase; import org.apache.solr.client.api.util.SolrVersion; import org.apache.solr.client.solrj.RemoteSolrException; -import org.apache.solr.client.solrj.RequestNotSentException; import org.apache.solr.client.solrj.SolrClient; import org.apache.solr.client.solrj.SolrRequest; import org.apache.solr.client.solrj.SolrServerException; @@ -56,7 +53,6 @@ import org.apache.solr.util.ServletFixtures.DebugServlet; import org.eclipse.jetty.client.WWWAuthenticationProtocolHandler; import org.eclipse.jetty.http.HttpStatus; -import org.eclipse.jetty.io.EofException; import org.eclipse.jetty.util.ssl.SslContextFactory; import org.junit.Test; @@ -747,28 +743,6 @@ public void testRequestTimeoutWithHttpClient() throws Exception { } } - @Test - public void testErrorClassification() throws Exception { - String url = solrTestRule.getBaseUrl() + DEBUG_SERVLET_PATH; - try (HttpJettySolrClient client = - (HttpJettySolrClient) builder(url, DEFAULT_CONNECTION_TIMEOUT, 0).build()) { - IOException unsent = - new RequestNotSentException("Broken pipe", new IOException("Broken pipe")); - assertTrue(client.wasRequestUnsent(new SolrServerException("wrapped", unsent))); - assertTrue(client.wasCommError(new SolrServerException("wrapped", unsent))); - - // Jetty's own connection-lost types are communication errors, but they say nothing about - // whether the request was delivered, so they must never claim it was unsent. - for (Throwable lost : - List.of(new EofException("Connection reset by peer"), new ClosedChannelException())) { - assertTrue(lost.getClass().getName(), client.wasCommError(lost)); - assertFalse(lost.getClass().getName(), client.wasRequestUnsent(lost)); - } - - assertFalse(client.wasCommError(new IOException("Broken pipe"))); - } - } - private static void assertIsTimeout(Throwable t) { assertThat(t.getMessage(), containsStringIgnoringCase("Timeout")); } diff --git a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/HttpJdkSolrClientTest.java b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/HttpJdkSolrClientTest.java index eb2b3640c6ba..9f9233f375e8 100644 --- a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/HttpJdkSolrClientTest.java +++ b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/HttpJdkSolrClientTest.java @@ -18,17 +18,13 @@ package org.apache.solr.client.solrj.impl; import java.io.IOException; -import java.net.ConnectException; import java.net.CookieHandler; import java.net.CookieManager; import java.net.ServerSocket; import java.net.Socket; -import java.net.SocketException; import java.net.URI; import java.net.URISyntaxException; -import java.net.UnknownHostException; import java.net.http.HttpClient; -import java.net.http.HttpConnectTimeoutException; import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; @@ -45,7 +41,6 @@ import org.apache.lucene.util.NamedThreadFactory; import org.apache.solr.client.api.util.SolrVersion; import org.apache.solr.client.solrj.RemoteSolrException; -import org.apache.solr.client.solrj.RequestNotSentException; import org.apache.solr.client.solrj.SolrRequest; import org.apache.solr.client.solrj.SolrServerException; import org.apache.solr.client.solrj.request.JavaBinRequestWriter; @@ -740,27 +735,6 @@ private HttpJdkSolrClient.Builder builder(String url) { return builder(url, DEFAULT_CONNECTION_TIMEOUT, DEFAULT_CONNECTION_TIMEOUT); } - @Test - public void testErrorClassification() throws Exception { - String url = solrTestRule.getBaseUrl() + DEBUG_SERVLET_PATH; - try (HttpJdkSolrClient client = builder(url).build()) { - IOException unsent = - new RequestNotSentException("Broken pipe", new IOException("Broken pipe")); - assertTrue(client.wasRequestUnsent(new SolrServerException("wrapped", unsent))); - assertTrue(client.wasRequestUnsent(new ConnectException("Connection refused"))); - assertTrue(client.wasRequestUnsent(new HttpConnectTimeoutException("timed out"))); - - // A bare IOException may have been sent and applied, so it is never proof of the contrary. - assertFalse(client.wasRequestUnsent(new IOException("Broken pipe"))); - assertFalse(client.wasRequestUnsent(new UnknownHostException("nosuchhost"))); - - assertTrue(client.wasCommError(new UnknownHostException("nosuchhost"))); - assertTrue(client.wasCommError(new SocketException("Connection reset"))); - assertTrue(client.wasCommError(new HttpConnectTimeoutException("timed out"))); - assertFalse(client.wasCommError(new IOException("Broken pipe"))); - } - } - private byte[] javabinResponse() { String[] str = JAVABIN_STR.split(" "); byte[] bytes = new byte[str.length]; diff --git a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/SolrClientErrorClassificationTest.java b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/SolrClientErrorClassificationTest.java new file mode 100644 index 000000000000..d9ea47391ef8 --- /dev/null +++ b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/SolrClientErrorClassificationTest.java @@ -0,0 +1,123 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.solr.client.solrj.impl; + +import java.io.IOException; +import java.net.ConnectException; +import java.net.SocketException; +import java.net.UnknownHostException; +import java.net.http.HttpConnectTimeoutException; +import java.nio.channels.ClosedChannelException; +import org.apache.solr.SolrTestCase; +import org.apache.solr.client.solrj.RequestNotSentException; +import org.apache.solr.client.solrj.SolrClient; +import org.apache.solr.client.solrj.SolrRequest; +import org.apache.solr.client.solrj.SolrServerException; +import org.apache.solr.client.solrj.jetty.HttpJettySolrClient; +import org.apache.solr.common.util.NamedList; +import org.eclipse.jetty.io.EofException; +import org.junit.Test; + +/** + * {@link SolrClient#wasRequestUnsent} and {@link SolrClient#wasCommError} are pure functions of the + * failure, so each transport's answers can be asserted directly rather than raced for through an + * integration test. No server is needed; the clients are never asked to send anything. + * + *

The negative cases matter most: {@code wasRequestUnsent} returning false means "cannot tell", + * and treating a failure as unsent when it isn't would replay a non-idempotent update. + */ +public class SolrClientErrorClassificationTest extends SolrTestCase { + + private static final String DEAD_URL = "http://127.0.0.1:1/solr"; + + private static SolrServerException wrapped(Throwable cause) { + return new SolrServerException("wrapped", cause); + } + + private static RequestNotSentException unsent() { + return new RequestNotSentException("Broken pipe", new IOException("Broken pipe")); + } + + /** Every HTTP transport inherits these from {@link HttpSolrClient}. */ + private static void assertSharedHttpClassification(HttpSolrClient client) { + // The transport stated the answer; it holds whether it is the failure or nested inside one. + assertTrue(client.wasRequestUnsent(unsent())); + assertTrue(client.wasCommError(unsent())); + assertTrue(client.wasRequestUnsent(wrapped(unsent()))); + assertTrue(client.wasCommError(wrapped(unsent()))); + + // Nothing was written because nothing connected. + assertTrue(client.wasRequestUnsent(new ConnectException("Connection refused"))); + assertTrue(client.wasCommError(new ConnectException("Connection refused"))); + + // A comm error, but no proof either way about delivery. + assertFalse(client.wasRequestUnsent(new SocketException("Connection reset"))); + assertTrue(client.wasCommError(new SocketException("Connection reset"))); + assertFalse(client.wasRequestUnsent(new UnknownHostException("nosuchhost"))); + assertTrue(client.wasCommError(new UnknownHostException("nosuchhost"))); + + // A bare IOException may have been sent and applied, so it proves nothing. + assertFalse(client.wasRequestUnsent(new IOException("Broken pipe"))); + assertFalse(client.wasCommError(new IOException("Broken pipe"))); + } + + @Test + public void testHttpJdkSolrClientClassification() throws Exception { + try (HttpJdkSolrClient client = new HttpJdkSolrClient.Builder(DEAD_URL).build()) { + assertSharedHttpClassification(client); + + // The connection was never established, so the request cannot have been written. + assertTrue(client.wasRequestUnsent(new HttpConnectTimeoutException("timed out"))); + assertTrue(client.wasCommError(new HttpConnectTimeoutException("timed out"))); + } + } + + @Test + public void testHttpJettySolrClientClassification() throws Exception { + try (HttpJettySolrClient client = new HttpJettySolrClient.Builder(DEAD_URL).build()) { + assertSharedHttpClassification(client); + + // Jetty's connection-lost types are communication errors, but a connection can end after the + // request was fully written, so they must never claim it was unsent. Whether it was is + // answered at the throw site by the request-commit listener instead. + for (Throwable lost : + new Throwable[] { + new EofException("Connection reset by peer"), new ClosedChannelException() + }) { + assertTrue(lost.getClass().getName(), client.wasCommError(lost)); + assertFalse(lost.getClass().getName(), client.wasRequestUnsent(lost)); + } + } + } + + /** A plain {@link SolrClient} cannot tell, and must never claim otherwise. */ + @Test + public void testDefaultIsAlwaysFalse() { + SolrClient client = + new SolrClient() { + @Override + public NamedList request(SolrRequest request, String collection) { + throw new UnsupportedOperationException(); + } + + @Override + public void close() {} + }; + assertFalse(client.wasRequestUnsent(unsent())); + assertFalse(client.wasCommError(new SocketException("Connection reset"))); + } +} From 7ad47d9688ec9a366fbfe607b0b48042cc3ff8cf Mon Sep 17 00:00:00 2001 From: chan-dx Date: Sun, 30 Aug 2026 16:11:58 +0100 Subject: [PATCH 09/15] SOLR-18402: Make the cache refresh path explicit and testable --- .../solr/client/solrj/impl/CloudSolrClientCacheTest.java | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudSolrClientCacheTest.java b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudSolrClientCacheTest.java index e6319992f6a1..e94a84ce00e1 100644 --- a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudSolrClientCacheTest.java +++ b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudSolrClientCacheTest.java @@ -409,6 +409,9 @@ private static class RecordingCloudSolrClient extends CloudSolrClient implements private volatile Invocation defaultInvocation; private final List stateHistory = Collections.synchronizedList(new ArrayList<>()); private final NamedList okResponse; + // Answers "cannot tell" to both classification predicates, which these tests do not exercise. + // Stub it if a test needs a communication error. + private final HttpSolrClient httpClient = mock(HttpSolrClient.class); RecordingCloudSolrClient(ClusterStateProvider provider, int refreshThreads) { this(provider, true, true, false, refreshThreads); @@ -477,7 +480,7 @@ public ClusterStateProvider getClusterStateProvider() { @Override public HttpSolrClient getHttpClient() { - return mock(HttpSolrClient.class); + return httpClient; } @FunctionalInterface From 4a095752a15718f4557d36221e464b9baa35c880 Mon Sep 17 00:00:00 2001 From: chan-dx Date: Sun, 30 Aug 2026 16:11:59 +0100 Subject: [PATCH 10/15] SOLR-18402: Modify mayReplay logic to mayReplayAfterCommError. --- .../client/solrj/impl/CloudSolrClient.java | 15 +++-- .../solrj/impl/CloudSolrClientCacheTest.java | 60 ++++++++++++++++--- 2 files changed, 61 insertions(+), 14 deletions(-) diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudSolrClient.java b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudSolrClient.java index 87852615b9ab..36832f62716c 100644 --- a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudSolrClient.java +++ b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudSolrClient.java @@ -717,9 +717,10 @@ protected NamedList requestWithRetryOnStaleState( : SolrException.ErrorCode.UNKNOWN.code; final boolean wasCommError = wasCommError(exc); - // An update may already have been applied; only replay one the transport proves never - // arrived. - final boolean mayReplay = + // A communication error says nothing about whether an update was applied; only replay one + // the transport proves never arrived. A 503 is the server declining to process it, so that + // path is unaffected. + final boolean mayReplayAfterCommError = request.getRequestType() != SolrRequestType.UPDATE || wasRequestUnsent(exc); if (wasCommError @@ -753,7 +754,7 @@ protected NamedList requestWithRetryOnStaleState( } } // if it is a communication error , we must try again - if (mayReplay && retryCount < MAX_STALE_RETRIES) { + if ((!wasCommError || mayReplayAfterCommError) && retryCount < MAX_STALE_RETRIES) { // may be, we have a stale version of the collection state, // and we could not get any information from the server // it is probably not worth trying again and again because @@ -816,11 +817,13 @@ protected NamedList requestWithRetryOnStaleState( for (DocCollection ext : requestedCollections) { DocCollection latestStateFromZk = getDocCollection(ext.getName(), null); if (latestStateFromZk.getZNodeVersion() != ext.getZNodeVersion()) { - // looks like we couldn't reach the server because the state was stale == retry - stateWasStale = true; // we just pulled state from ZK, so update the cache so that the retry uses it collectionStateCache.put( ext.getName(), new ExpiringCachedDocCollection(latestStateFromZk)); + if (mayReplayAfterCommError) { + // looks like we couldn't reach the server because the state was stale == retry + stateWasStale = true; + } } } } diff --git a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudSolrClientCacheTest.java b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudSolrClientCacheTest.java index e94a84ce00e1..80e980345a80 100644 --- a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudSolrClientCacheTest.java +++ b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudSolrClientCacheTest.java @@ -45,6 +45,7 @@ import java.util.function.Function; import java.util.function.Supplier; import org.apache.solr.SolrTestCaseJ4; +import org.apache.solr.client.solrj.RemoteSolrException; import org.apache.solr.client.solrj.SolrRequest; import org.apache.solr.client.solrj.SolrServerException; import org.apache.solr.client.solrj.jetty.LBJettySolrClient; @@ -148,7 +149,6 @@ public void testUpdateIsNotReplayedWhenItMayHaveBeenApplied() throws Exception { String collName = "gettingstarted"; Set livenodes = new HashSet<>(); Map refs = new HashMap<>(); - Map colls = new HashMap<>(); Map> responses = new HashMap<>(); LBJettySolrClient mockLbclient = getMockLbHttpSolrClient(responses); @@ -160,10 +160,12 @@ public void testUpdateIsNotReplayedWhenItMayHaveBeenApplied() throws Exception { protected LBSolrClient createOrGetLbClient(HttpSolrClient myClient) { return mockLbclient; } - }.build()) { + } + // Pin the routing so the update takes the load-balanced path and the transport's + // failure arrives unwrapped. + .sendUpdatesToAnyReplica().build()) { livenodes.addAll(Set.of("192.168.1.108:7574_solr", "192.168.1.108:8983_solr")); refs.put(collName, new ClusterState.CollectionRef(loadCollection(collName, 1))); - colls.put(collName, loadCollection(collName, 1)); // Not a ConnectException: the transport cannot prove this request never left. responses.put( @@ -174,16 +176,58 @@ protected LBSolrClient createOrGetLbClient(HttpSolrClient myClient) { }); UpdateRequest update = new UpdateRequest().add("id", "123", "desc", "Something 0"); - // The routing randomization decides whether the failure arrives wrapped, so match the cause. - Exception thrown = expectThrows(Exception.class, () -> cloudClient.request(update, collName)); - assertTrue( - "the transport's failure must reach the caller", - SolrException.hasCause(thrown, SocketException.class)); + expectThrows(SocketException.class, () -> cloudClient.request(update, collName)); assertEquals( "an update that may have been applied must not be replayed", 1, lbhttpRequestCount.get()); } } + /** + * A 503 is the server declining to process the update, not a communication failure, so it stays + * retryable. {@link CloudSolrClient#directUpdate} raises this shape when a shard replica is + * unavailable. + */ + public void testUpdateIsRetriedOnRouteExceptionWith503() throws Exception { + String collName = "gettingstarted"; + Set livenodes = new HashSet<>(); + Map refs = new HashMap<>(); + + Map> responses = new HashMap<>(); + NamedList okResponse = new NamedList<>(); + okResponse.add("responseHeader", new NamedList<>(Map.of("status", 0))); + LBJettySolrClient mockLbclient = getMockLbHttpSolrClient(responses); + AtomicInteger lbhttpRequestCount = new AtomicInteger(); + try (ClusterStateProvider clusterStateProvider = getStateProvider(livenodes, refs); + CloudSolrClient cloudClient = + new RandomizingCloudSolrClientBuilder(clusterStateProvider) { + @Override + protected LBSolrClient createOrGetLbClient(HttpSolrClient myClient) { + return mockLbclient; + } + }.sendUpdatesToAnyReplica().build()) { + livenodes.addAll(Set.of("192.168.1.108:7574_solr", "192.168.1.108:8983_solr")); + refs.put(collName, new ClusterState.CollectionRef(loadCollection(collName, 1))); + + NamedList shardFailures = new NamedList<>(); + shardFailures.add( + "http://127.0.0.1:8983/solr/gettingstarted_shard1_replica_n1", + new RemoteSolrException("127.0.0.1:8983", 503, "Service Unavailable", null)); + responses.put( + "request", + o -> { + if (lbhttpRequestCount.incrementAndGet() == 1) { + return new CloudSolrClient.RouteException( + SolrException.ErrorCode.SERVICE_UNAVAILABLE, shardFailures, Map.of()); + } + return okResponse; + }); + + UpdateRequest update = new UpdateRequest().add("id", "123", "desc", "Something 0"); + cloudClient.request(update, collName); + assertEquals("a 503 must still be retried", 2, lbhttpRequestCount.get()); + } + } + public void testStaleStateRetrySkipsStateVersionBeforeWait() throws Exception { String collName = "gettingstarted"; Set liveNodes = new HashSet<>(Set.of("192.168.1.108:8983_solr")); From 57cc8e62dfeaa5a743e69dae0cd45f813dd11ae3 Mon Sep 17 00:00:00 2001 From: chan-dx Date: Sun, 30 Aug 2026 16:11:59 +0100 Subject: [PATCH 11/15] SOLR-18402: HttpJettySolrClient: report a lost HTTP/2 session as EofException --- .../apache/solr/client/solrj/jetty/HttpJettySolrClient.java | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/solr/solrj-jetty/src/java/org/apache/solr/client/solrj/jetty/HttpJettySolrClient.java b/solr/solrj-jetty/src/java/org/apache/solr/client/solrj/jetty/HttpJettySolrClient.java index f8fa7f958d87..af8b087d239f 100644 --- a/solr/solrj-jetty/src/java/org/apache/solr/client/solrj/jetty/HttpJettySolrClient.java +++ b/solr/solrj-jetty/src/java/org/apache/solr/client/solrj/jetty/HttpJettySolrClient.java @@ -531,9 +531,11 @@ public NamedList request(SolrRequest solrRequest, String collection) // Jetty HTTP/2 throws IllegalStateException ("session closed") when the connection is lost. abortCause = e; throw committed.get() - ? new SolrServerException("Connection lost at: " + url, new IOException(e)) + ? new SolrServerException( + "Connection lost at: " + url, new EofException("HTTP/2 session closed", e)) : new SolrServerException( - "Connection lost at: " + url, new RequestNotSentException(e.getMessage(), e)); + "Connection failed before the request was sent to: " + url, + new RequestNotSentException(e.getMessage(), e)); } catch (SolrServerException | RuntimeException sse) { abortCause = sse; throw sse; From f7b3a9bcac8b2d64dea7625f3c26d6910fcc52f9 Mon Sep 17 00:00:00 2001 From: chan-dx Date: Sun, 30 Aug 2026 16:11:59 +0100 Subject: [PATCH 12/15] SOLR-18402: Update changelog --- .../SOLR-18402-consolidate-retry-classification.yml | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/changelog/unreleased/SOLR-18402-consolidate-retry-classification.yml b/changelog/unreleased/SOLR-18402-consolidate-retry-classification.yml index ec7167ca29bf..b095a3967373 100644 --- a/changelog/unreleased/SOLR-18402-consolidate-retry-classification.yml +++ b/changelog/unreleased/SOLR-18402-consolidate-retry-classification.yml @@ -1,8 +1,6 @@ title: > - SolrJ: transports now classify their own failures via SolrClient.wasRequestUnsent / - wasCommError; CloudSolrClient and LBSolrClient ask instead of matching exception types. - CloudSolrClient now replays an update only when the transport proves it was never sent, and - LBSolrClient fails over on a bare IOException instead of aborting. + SolrJ transports now classify their own failures via SolrClient.wasRequestUnsent / + wasCommError, and CloudSolrClient replays an update only when the transport proves it unsent type: changed authors: - name: Han Chan From 308961dc08a1e429b80cd5afe47d817565684979 Mon Sep 17 00:00:00 2001 From: chan-dx Date: Sun, 30 Aug 2026 16:11:59 +0100 Subject: [PATCH 13/15] SOLR-18402: Verifies that Jetty connection-loss errors are classified as communication failures without falsely claiming the request was unsent, even when wrapped or raised after the request was already committed. --- .../solrj/impl/SolrClientErrorClassificationTest.java | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/SolrClientErrorClassificationTest.java b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/SolrClientErrorClassificationTest.java index d9ea47391ef8..7bc5ef833176 100644 --- a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/SolrClientErrorClassificationTest.java +++ b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/SolrClientErrorClassificationTest.java @@ -96,10 +96,15 @@ public void testHttpJettySolrClientClassification() throws Exception { // answered at the throw site by the request-commit listener instead. for (Throwable lost : new Throwable[] { - new EofException("Connection reset by peer"), new ClosedChannelException() + new EofException("Connection reset by peer"), + new ClosedChannelException(), + // The shape HttpJettySolrClient raises once an HTTP/2 session is lost after commit. + new EofException("HTTP/2 session closed", new IllegalStateException("session closed")) }) { assertTrue(lost.getClass().getName(), client.wasCommError(lost)); + assertTrue(lost.getClass().getName(), client.wasCommError(wrapped(lost))); assertFalse(lost.getClass().getName(), client.wasRequestUnsent(lost)); + assertFalse(lost.getClass().getName(), client.wasRequestUnsent(wrapped(lost))); } } } From 2b88ce89a3ccea5f67890601e696659e01c678a7 Mon Sep 17 00:00:00 2001 From: chan-dx Date: Mon, 31 Aug 2026 13:47:56 +0100 Subject: [PATCH 14/15] SOLR-18402: Stop replaying updates on a 503 --- .../client/solrj/impl/CloudSolrClient.java | 12 ++++----- .../solrj/impl/CloudSolrClientCacheTest.java | 26 +++++++++---------- 2 files changed, 19 insertions(+), 19 deletions(-) diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudSolrClient.java b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudSolrClient.java index 36832f62716c..cacea7286cee 100644 --- a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudSolrClient.java +++ b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudSolrClient.java @@ -717,10 +717,10 @@ protected NamedList requestWithRetryOnStaleState( : SolrException.ErrorCode.UNKNOWN.code; final boolean wasCommError = wasCommError(exc); - // A communication error says nothing about whether an update was applied; only replay one - // the transport proves never arrived. A 503 is the server declining to process it, so that - // path is unaffected. - final boolean mayReplayAfterCommError = + // Neither a comm error nor a 503 proves an update went unapplied: directUpdate raises + // RouteException only after collecting every shard's result. Replay only what the transport + // proves never arrived. + final boolean mayReplay = request.getRequestType() != SolrRequestType.UPDATE || wasRequestUnsent(exc); if (wasCommError @@ -754,7 +754,7 @@ protected NamedList requestWithRetryOnStaleState( } } // if it is a communication error , we must try again - if ((!wasCommError || mayReplayAfterCommError) && retryCount < MAX_STALE_RETRIES) { + if (mayReplay && retryCount < MAX_STALE_RETRIES) { // may be, we have a stale version of the collection state, // and we could not get any information from the server // it is probably not worth trying again and again because @@ -820,7 +820,7 @@ protected NamedList requestWithRetryOnStaleState( // we just pulled state from ZK, so update the cache so that the retry uses it collectionStateCache.put( ext.getName(), new ExpiringCachedDocCollection(latestStateFromZk)); - if (mayReplayAfterCommError) { + if (mayReplay) { // looks like we couldn't reach the server because the state was stale == retry stateWasStale = true; } diff --git a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudSolrClientCacheTest.java b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudSolrClientCacheTest.java index 80e980345a80..f713b86b9cbc 100644 --- a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudSolrClientCacheTest.java +++ b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudSolrClientCacheTest.java @@ -183,18 +183,16 @@ protected LBSolrClient createOrGetLbClient(HttpSolrClient myClient) { } /** - * A 503 is the server declining to process the update, not a communication failure, so it stays - * retryable. {@link CloudSolrClient#directUpdate} raises this shape when a shard replica is - * unavailable. + * {@link CloudSolrClient#directUpdate} raises a {@link CloudSolrClient.RouteException} only after + * collecting every shard's result, so a 503 from one shard can follow success on another and a + * replay would re-apply those. */ - public void testUpdateIsRetriedOnRouteExceptionWith503() throws Exception { + public void testUpdateIsNotRetriedOnRouteExceptionWith503() throws Exception { String collName = "gettingstarted"; Set livenodes = new HashSet<>(); Map refs = new HashMap<>(); Map> responses = new HashMap<>(); - NamedList okResponse = new NamedList<>(); - okResponse.add("responseHeader", new NamedList<>(Map.of("status", 0))); LBJettySolrClient mockLbclient = getMockLbHttpSolrClient(responses); AtomicInteger lbhttpRequestCount = new AtomicInteger(); try (ClusterStateProvider clusterStateProvider = getStateProvider(livenodes, refs); @@ -215,16 +213,18 @@ protected LBSolrClient createOrGetLbClient(HttpSolrClient myClient) { responses.put( "request", o -> { - if (lbhttpRequestCount.incrementAndGet() == 1) { - return new CloudSolrClient.RouteException( - SolrException.ErrorCode.SERVICE_UNAVAILABLE, shardFailures, Map.of()); - } - return okResponse; + lbhttpRequestCount.incrementAndGet(); + return new CloudSolrClient.RouteException( + SolrException.ErrorCode.SERVICE_UNAVAILABLE, shardFailures, Map.of()); }); UpdateRequest update = new UpdateRequest().add("id", "123", "desc", "Something 0"); - cloudClient.request(update, collName); - assertEquals("a 503 must still be retried", 2, lbhttpRequestCount.get()); + expectThrows( + CloudSolrClient.RouteException.class, () -> cloudClient.request(update, collName)); + assertEquals( + "a 503 may follow partial success, so it must not be replayed", + 1, + lbhttpRequestCount.get()); } } From 82aabf7b34b434ef561c95020ace206a038bd842 Mon Sep 17 00:00:00 2001 From: chan-dx Date: Mon, 31 Aug 2026 15:22:16 +0100 Subject: [PATCH 15/15] SOLR-18402: Update upgrade note --- .../upgrade-notes/pages/major-changes-in-solr-10.adoc | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/solr/solr-ref-guide/modules/upgrade-notes/pages/major-changes-in-solr-10.adoc b/solr/solr-ref-guide/modules/upgrade-notes/pages/major-changes-in-solr-10.adoc index cfa96a938654..c782c326ac1f 100644 --- a/solr/solr-ref-guide/modules/upgrade-notes/pages/major-changes-in-solr-10.adoc +++ b/solr/solr-ref-guide/modules/upgrade-notes/pages/major-changes-in-solr-10.adoc @@ -103,6 +103,12 @@ Its builder will dynamically detect if solr-jetty is available and use that, oth CommonParams.QT has been un-deprecated. Nonetheless, if your code makes explicit reference to "qt" when constructing a standard request, there is usually a better way. +`CloudSolrClient` now retries a failed update only when the transport can prove the request never reached the server. +Previously any communication error, or a 503, caused a retry, which could re-send an update that had already been partially applied. + +`SolrClient` gains `wasRequestUnsent(Throwable)` and `wasCommError(Throwable)`, both defaulting to `false` and overridden per transport. +`CloudSolrClient.wasCommError` is now `public`, and `LBSolrClient.isConnectException` has been removed; override `wasRequestUnsent` on the transport client instead. + === Jetty Configuration Solr 10.1 upgrades the server to Eclipse Jetty 12.1, which removed Jetty's directory-scanning deployer (the `DeploymentManager` and `ContextProvider` classes).