This is an automated email from the ASF dual-hosted git repository.
dsmiley pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/solr.git
The following commit(s) were added to refs/heads/main by this push:
new a98481d9227 SOLR-18401: HttpJettySolrClient: detect if request wasn't
sent; retry (#4805)
a98481d9227 is described below
commit a98481d92279942a19148e5bf27bd5e6d15349a5
Author: David Smiley <[email protected]>
AuthorDate: Wed Aug 26 09:28:21 2026 -0400
SOLR-18401: HttpJettySolrClient: detect if request wasn't sent; retry
(#4805)
HttpJettySolrClient: detect if request wasn't sent; throw a new
RequestNotSentException. CloudSolrClient & LBSolrClient will detect it and
retry -- typically on another replica.
---
.../unreleased/SOLR-18401-retry-unsent-request.yml | 9 ++
.../solr/cloud/TestMiniSolrCloudClusterSSL.java | 5 +-
.../client/solrj/jetty/HttpJettySolrClient.java | 34 +++++-
.../solr/client/solrj/RequestNotSentException.java | 36 ++++++
.../solr/client/solrj/impl/CloudSolrClient.java | 12 +-
.../solr/client/solrj/impl/LBAsyncSolrClient.java | 9 +-
.../solr/client/solrj/impl/LBSolrClient.java | 7 +-
.../java/org/apache/solr/common/SolrException.java | 20 ++++
.../solrj/impl/LBSolrClientRetryUnsentTest.java | 122 +++++++++++++++++++++
9 files changed, 242 insertions(+), 12 deletions(-)
diff --git a/changelog/unreleased/SOLR-18401-retry-unsent-request.yml
b/changelog/unreleased/SOLR-18401-retry-unsent-request.yml
new file mode 100644
index 00000000000..919810e11bc
--- /dev/null
+++ b/changelog/unreleased/SOLR-18401-retry-unsent-request.yml
@@ -0,0 +1,9 @@
+title: >
+ HttpJettySolrClient: detect if request wasn't sent; means retry-able.
+ CloudSolrClient & LBSolrClient will detect it.
+type: changed
+authors:
+ - name: David Smiley
+links:
+ - name: SOLR-18401
+ url: https://issues.apache.org/jira/browse/SOLR-18401
diff --git
a/solr/core/src/test/org/apache/solr/cloud/TestMiniSolrCloudClusterSSL.java
b/solr/core/src/test/org/apache/solr/cloud/TestMiniSolrCloudClusterSSL.java
index 8fbbe214fe0..39d6561381c 100644
--- a/solr/core/src/test/org/apache/solr/cloud/TestMiniSolrCloudClusterSSL.java
+++ b/solr/core/src/test/org/apache/solr/cloud/TestMiniSolrCloudClusterSSL.java
@@ -31,6 +31,7 @@ import org.apache.solr.client.solrj.impl.SolrHttpConstants;
import org.apache.solr.client.solrj.jetty.HttpJettySolrClient;
import org.apache.solr.client.solrj.request.CollectionAdminRequest;
import org.apache.solr.client.solrj.request.CoreAdminRequest;
+import org.apache.solr.common.SolrException;
import org.apache.solr.common.params.CoreAdminParams.CoreAdminAction;
import org.apache.solr.embedded.JettyConfig;
import org.apache.solr.embedded.JettySolrRunner;
@@ -205,8 +206,8 @@ public class TestMiniSolrCloudClusterSSL extends
SolrTestCaseJ4 {
}
});
assertTrue(
- "Expected an root cause SSL Exception, got: " + ex.toString(),
- ex.getCause() instanceof SSLException);
+ "Expected an SSL Exception in the cause chain, got: " + ex,
+ SolrException.hasCause(ex, SSLException.class));
}
} finally {
cluster.shutdown();
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 e561dc4d056..77e1f7e3f1a 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
@@ -36,7 +36,9 @@ import java.util.concurrent.RejectedExecutionException;
import java.util.concurrent.Semaphore;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.TimeoutException;
+import java.util.concurrent.atomic.AtomicBoolean;
import org.apache.solr.client.api.util.SolrVersion;
+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;
@@ -473,9 +475,26 @@ public class HttpJettySolrClient extends HttpSolrClient {
String url = getRequestUrl(solrRequest, collection);
Throwable abortCause = null;
Request req = null;
+ // Jetty notifies "commit" once the request headers have been written to
the network. Until then
+ // nothing of the request has reached the server, so a failure is safe to
retry elsewhere.
+ AtomicBoolean committed = new AtomicBoolean();
try {
InputStreamResponseListener listener = new
InputStreamReleaseTrackingResponseListener();
- req = sendRequest(makeRequest(solrRequest, url, false), listener);
+ MakeRequestReturnValue mrrv = makeRequest(solrRequest, url, false);
+ mrrv.request.onRequestCommit(r -> committed.set(true));
+ try {
+ req = sendRequest(mrrv, listener);
+ } catch (IOException e) {
+ // Writing the body can fail on this thread rather than
asynchronously, typically when the
+ // pooled connection was already closed.
+ abortCause = e;
+ req = mrrv.request;
+ throw committed.get()
+ ? new SolrServerException("IOException occurred when talking to
server at: " + url, e)
+ : new SolrServerException(
+ "Connection failed before the request was sent to: " + url,
+ new RequestNotSentException(e.getMessage(), e));
+ }
// only waits for headers, so use the idle timeout
Response response = listener.get(idleTimeoutMillis,
TimeUnit.MILLISECONDS);
url = req.getURI().toString();
@@ -497,14 +516,21 @@ public class HttpJettySolrClient extends HttpSolrClient {
if (cause instanceof SolrServerException) {
throw (SolrServerException) cause;
} else if (cause instanceof IOException) {
- throw new SolrServerException(
- "IOException occurred when talking to server at: " + url, cause);
+ throw committed.get()
+ ? new SolrServerException(
+ "IOException occurred when talking to server at: " + url,
cause)
+ : new SolrServerException(
+ "Connection failed before the request was sent to: " + url,
+ new RequestNotSentException(cause.getMessage(), cause));
}
throw new SolrServerException(cause.getMessage(), cause);
} catch (IllegalStateException e) {
// Jetty HTTP/2 throws IllegalStateException ("session closed") when the
connection is lost.
abortCause = e;
- throw new SolrServerException("Connection lost at: " + url, new
IOException(e));
+ throw committed.get()
+ ? new SolrServerException("Connection lost at: " + url, new
IOException(e))
+ : new SolrServerException(
+ "Connection lost at: " + url, new
RequestNotSentException(e.getMessage(), e));
} catch (SolrServerException | RuntimeException sse) {
abortCause = sse;
throw sse;
diff --git
a/solr/solrj/src/java/org/apache/solr/client/solrj/RequestNotSentException.java
b/solr/solrj/src/java/org/apache/solr/client/solrj/RequestNotSentException.java
new file mode 100644
index 00000000000..566dd691a5d
--- /dev/null
+++
b/solr/solrj/src/java/org/apache/solr/client/solrj/RequestNotSentException.java
@@ -0,0 +1,36 @@
+/*
+ * 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;
+
+import java.io.IOException;
+import java.io.Serial;
+
+/**
+ * Indicates that a request failed before any of it was written to the
network, so the server cannot
+ * have processed it. Retrying such a request on another node is safe even
when it is not
+ * idempotent.
+ *
+ * <p>Typically a pooled connection that the server had already closed.
+ */
+public class RequestNotSentException extends IOException {
+
+ @Serial private static final long serialVersionUID = 1L;
+
+ public RequestNotSentException(String message, Throwable cause) {
+ super(message, cause);
+ }
+}
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 7df7eddc38f..f5a250fb2e8 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
@@ -48,6 +48,7 @@ import java.util.concurrent.locks.Lock;
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;
@@ -205,9 +206,14 @@ public abstract class CloudSolrClient extends SolrClient {
return getClusterStateProvider().getClusterState();
}
- /** Is this a communication error? We will retry if so. */
+ /**
+ * 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) {
- return t instanceof SocketException || t instanceof UnknownHostException;
+ return SolrException.hasCause(t, SocketException.class)
+ || SolrException.hasCause(t, UnknownHostException.class)
+ || SolrException.hasCause(t, RequestNotSentException.class);
}
@Override
@@ -712,7 +718,7 @@ public abstract class CloudSolrClient extends SolrClient {
? ((SolrException) rootCause).code()
: SolrException.ErrorCode.UNKNOWN.code;
- final boolean wasCommError = wasCommError(rootCause);
+ final boolean wasCommError = wasCommError(exc);
if (wasCommError
|| (exc instanceof RouteException
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 5a91c0314ba..88657cbca14 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
@@ -25,6 +25,7 @@ import java.util.concurrent.CompletionException;
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;
@@ -218,13 +219,17 @@ public abstract class LBAsyncSolrClient extends
LBSolrClient {
if (!isNonRetryable
&& (rootCause instanceof IOException || rootCause instanceof
TimeoutException)) {
listener.onFailure((!isZombie) ? makeServerAZombie(endpoint, e) : e,
true);
- } else if (isNonRetryable && isConnectException(rootCause)) {
+ } else if (isNonRetryable
+ && (isConnectException(rootCause)
+ || SolrException.hasCause(e, RequestNotSentException.class))) {
+ // 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);
} else {
listener.onFailure(e, false);
}
} catch (IOException e) {
- if (!isNonRetryable || isConnectException(e)) {
+ if (!isNonRetryable || isConnectException(e) || e instanceof
RequestNotSentException) {
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 eefbd08b2b1..4a9d63cd375 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
@@ -43,6 +43,7 @@ import java.util.concurrent.TimeoutException;
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;
@@ -671,7 +672,11 @@ public abstract class LBSolrClient extends SolrClient {
if (!isNonRetryable
&& (rootCause instanceof IOException || rootCause instanceof
TimeoutException)) {
ex = (!isZombie) ? makeServerAZombie(baseUrl, e) : e;
- } else if (isNonRetryable && isConnectException(rootCause)) {
+ } else if (isNonRetryable
+ && (isConnectException(rootCause)
+ || SolrException.hasCause(e, RequestNotSentException.class))) {
+ // 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;
} else {
throw e;
diff --git a/solr/solrj/src/java/org/apache/solr/common/SolrException.java
b/solr/solrj/src/java/org/apache/solr/common/SolrException.java
index b8eceee17af..fd9f43f9bc4 100644
--- a/solr/solrj/src/java/org/apache/solr/common/SolrException.java
+++ b/solr/solrj/src/java/org/apache/solr/common/SolrException.java
@@ -154,6 +154,26 @@ public class SolrException extends RuntimeException {
return t;
}
+ /** Cause chains are shallow in practice; the cap only guards against a
cyclic chain. */
+ private static final int MAX_CAUSE_DEPTH = 100;
+
+ /**
+ * Whether {@code t} or anything in its cause chain is of the given type.
Prefer this to {@link
+ * #getRootCause} when classifying a failure, since a transport may report
it wrapped at any
+ * depth.
+ */
+ public static boolean hasCause(Throwable t, Class<? extends Throwable> type)
{
+ int depth = 0;
+ for (Throwable cause = t;
+ cause != null && depth++ < MAX_CAUSE_DEPTH;
+ cause = cause.getCause()) {
+ if (type.isInstance(cause)) {
+ return true;
+ }
+ }
+ return false;
+ }
+
/**
* Ensure that the provided tragic exception is wrapped in a 5xx
SolrException
*
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
new file mode 100644
index 00000000000..3163616e272
--- /dev/null
+++
b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/LBSolrClientRetryUnsentTest.java
@@ -0,0 +1,122 @@
+/*
+ * 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.util.ArrayList;
+import java.util.List;
+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.SolrRequest.SolrRequestType;
+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.util.NamedList;
+import org.junit.Test;
+
+/**
+ * A failure that proves the request never reached the server is safe to
replay even when
+ * LBSolrClient would otherwise refuse to retry the request. {@link
RequestNotSentException} is that
+ * proof.
+ */
+public class LBSolrClientRetryUnsentTest extends SolrTestCase {
+
+ private static final LBSolrClient.Endpoint DEAD_HOST_1 =
+ new LBSolrClient.Endpoint("http://127.0.0.1:1/solr";);
+ private static final LBSolrClient.Endpoint DEAD_HOST_2 =
+ new LBSolrClient.Endpoint("http://127.0.0.1:2/solr";);
+
+ /** Fails whatever endpoint is tried first with {@code failure}; any later
endpoint succeeds. */
+ private static class FailFirstEndpoint extends LBSolrClient {
+ final List<String> attempted = new ArrayList<>();
+ private final Exception failure;
+
+ FailFirstEndpoint(Exception failure) {
+ super(List.of(DEAD_HOST_1, DEAD_HOST_2));
+ this.failure = failure;
+ }
+
+ @Override
+ protected SolrClient getClient(Endpoint endpoint) {
+ return new SolrClient() {
+ @Override
+ public NamedList<Object> request(SolrRequest<?> request, String
collection)
+ throws SolrServerException, IOException {
+ attempted.add(endpoint.getBaseUrl());
+ if (attempted.size() > 1) {
+ return new NamedList<>();
+ }
+ if (failure instanceof SolrServerException sse) {
+ throw sse;
+ }
+ throw (IOException) failure;
+ }
+
+ @Override
+ public void close() {}
+ };
+ }
+ }
+
+ private static SolrServerException unsentException() {
+ IOException onTheWire = new IOException("Broken pipe");
+ return new SolrServerException(
+ "Connection failed before the request was sent to: " +
DEAD_HOST_1.getUrl(),
+ new RequestNotSentException(onTheWire.getMessage(), onTheWire));
+ }
+
+ private static SolrServerException maybeSentException() {
+ return new SolrServerException(
+ "IOException occurred when talking to server at: " +
DEAD_HOST_1.getUrl(),
+ new IOException("Broken pipe"));
+ }
+
+ private static List<String> requestReturningAttemptedUrls(
+ Exception failure, SolrRequest<?> request) throws Exception {
+ try (FailFirstEndpoint client = new FailFirstEndpoint(failure)) {
+ client.request(new LBSolrClient.Req(request, List.of(DEAD_HOST_1,
DEAD_HOST_2)));
+ return client.attempted;
+ }
+ }
+
+ @Test
+ public void testUpdateIsRetriedWhenRequestWasNeverSent() throws Exception {
+ assertEquals(
+ List.of(DEAD_HOST_1.getBaseUrl(), DEAD_HOST_2.getBaseUrl()),
+ requestReturningAttemptedUrls(unsentException(), new
UpdateRequest().add("id", "1")));
+ }
+
+ /** LBSolrClient classifies {@link SolrRequestType#UPDATE} as non-retryable.
*/
+ @Test
+ public void testRequestThatMayHaveBeenReceivedIsNotRetried() {
+ LBSolrClient.Req req =
+ new LBSolrClient.Req(new UpdateRequest().add("id", "1"),
List.of(DEAD_HOST_1, DEAD_HOST_2));
+ try (FailFirstEndpoint client = new
FailFirstEndpoint(maybeSentException())) {
+ expectThrows(SolrServerException.class, () -> client.request(req));
+ assertEquals(List.of(DEAD_HOST_1.getBaseUrl()), client.attempted);
+ }
+ }
+
+ @Test
+ public void testQueryIsStillRetriedOnAnyIOException() throws Exception {
+ assertEquals(
+ List.of(DEAD_HOST_1.getBaseUrl(), DEAD_HOST_2.getBaseUrl()),
+ requestReturningAttemptedUrls(maybeSentException(), new
QueryRequest()));
+ }
+}
