David Smiley created SOLR-18401:
-----------------------------------
Summary: HttpJettySolrClient: detect if request wasn't sent; means
retry-able
Key: SOLR-18401
URL: https://issues.apache.org/jira/browse/SOLR-18401
Project: Solr
Issue Type: Improvement
Components: SolrJ
Reporter: David Smiley
Assignee: David Smiley
h2. Problem
{{CloudSolrClient}} no longer retries a request that failed because the pooled
HTTP connection it was handed had already been closed. The request never
reached the server, yet the failure propagates to the caller instead of failing
over to another replica.
This is a regression from SOLR-18188, which removed {{CloudLegacySolrClient}}
(Apache HttpClient) and left the Jetty-based client as the only implementation.
The legacy client classified this case as a communication error:
{code:java}
@Override
protected boolean wasCommError(Throwable rootCause) {
return super.wasCommError(rootCause)
|| rootCause instanceof ConnectTimeoutException
|| rootCause instanceof NoHttpResponseException;
}
{code}
{{NoHttpResponseException}} is Apache HttpClient's signal for "the server
closed the pooled connection before reading the request" – provably never
processed, therefore safe to replay. The Jetty transport has no equivalent, and
the base implementation does not cover it:
{code:java}
/** Is this a communication error? We will retry if so. */
protected boolean wasCommError(Throwable t) {
return t instanceof SocketException || t instanceof UnknownHostException;
}
{code}
Over the Jetty transport the same condition surfaces as
{{{}ClosedChannelException{}}}, {{{}EofException: Connection reset by peer{}}},
or {{{}IOException: Broken pipe{}}}. All are plain {{{}IOException{}}}, none is
a {{{}SocketException{}}}, so {{wasCommError}} returns false and
{{CloudSolrClient}} gives up with retries still available:
{noformat}
CloudSolrClient request was not communication error it seems
CloudSolrClient Request to collection [collection1] failed due to (0)
java.io.IOException: Broken pipe, retry=0 maxRetries=5 commError=false
errorCode=0
{noformat}
{{LBSolrClient}} also declines, because updates are non-retryable there unless
the cause is a {{{}ConnectException{}}}:
{code:java}
} else if (isNonRetryable && isConnectException(rootCause)) {
ex = (!isZombie) ? makeServerAZombie(baseUrl, e) : e;
}
{code}
The outcome therefore depends on a race. If Jetty's connection pool has already
evicted the dead connection, the next attempt gets a {{ConnectException}} (a
{{{}SocketException{}}}), both layers retry, and the request succeeds. If the
pool hands out the not-yet-evicted dead connection, the request fails hard.
h2. User impact
Any {{CloudSolrClient}} deployment where idle pooled connections are reaped – a
server idle timeout, a load balancer, a firewall – now sees intermittent hard
failures on requests that never reached Solr. Solr 10.x and earlier retried
these.
h2. How it was found
{{RecoveryAfterSoftCommitTest}} went from zero CI failures to a sustained 3-10%
failure rate. Per fucit.org weekly failure rates:
{noformat}
2026-18 .. 2026-23 no failures
2026-24 3.4% (17/502)
2026-26 5.7%
2026-28 6.9%
2026-30 7.2%
2026-32 9.8%
2026-34 6.6%
{noformat}
ISO week 24 is 2026-06-08..14. SOLR-18188 landed 2026-06-10.
The test partitions the non-leader replica with a {{{}SocketProxy{}}}, then
keeps indexing. {{SolrTestCaseJ4.RandomizingCloudSolrClientBuilder}} randomizes
{{{}shardLeadersOnly{}}}, so roughly half of runs use a client that sends
updates to any replica – including the partitioned one. Archived CI stacks show
the client POSTing directly to the follower:
{noformat}
SolrServerException: IOException occurred when talking to server at:
http://127.0.0.1:35661/solr/collection1_shard1_replica_n3/update
{noformat}
Reproduced locally at a comparable rate (1 failure in 12 seeds); the failure is
a race, not seed-deterministic.
h2. Proposal
Rather than broadening {{wasCommError}} to all {{IOException}} – which would
replay updates that may have been applied – determine structurally whether any
bytes of the request reached the wire.
Jetty's {{Request.onRequestCommit(Request.CommitListener)}} fires from
{{{}HttpSender.headersToCommit(){}}}, which runs in the write callback's
{{onSuccess()}} – after the header bytes were successfully written to the
{{{}EndPoint{}}}. If commit never fired, nothing of the request was sent, so
the server cannot have seen it and a retry is safe even for a non-idempotent
update. This is exactly the {{NoHttpResponseException}} invariant, recovered on
the Jetty transport.
Note the converse does not hold: "committed" only means the bytes entered the
kernel send buffer, not that the server processed them. That errs conservative
– we still do not retry – so there is no behavior regression.
Sketch:
* In {{{}HttpJettySolrClient{}}}, attach a commit listener when the request is
built (alongside the existing {{req.attribute(REQ_PRINCIPAL_KEY, ...)}} and
listener wiring) and record whether commit fired.
* In the {{catch (ExecutionException)}} block, when the cause is an
{{IOException}} and the request was never committed, throw a distinct exception
rather than the generic {{{}"IOException occurred when talking to server at:
..."{}}}.
* Teach {{CloudSolrClient.wasCommError}} and {{LBSolrClient}} to treat that
exception as retryable.
Two options for the exception type:
# Reuse {{{}ConnectException{}}}. No changes outside {{{}solrj-jetty{}}}: it
extends {{SocketException}} so {{wasCommError}} passes, and
{{LBSolrClient.isConnectException}} already accepts it. Minimal, but
semantically inaccurate – nothing refused the connection.
# Add a marker type in solrj core, e.g. {{{}SolrRequestNotSentException
extends IOException{}}}, and reference it from {{wasCommError}} and
{{{}LBSolrClient{}}}. {{solrj-jetty}} already depends on solrj. Slightly larger
diff, but it names the invariant that makes the retry safe. Preferred.
h2. Also in scope
* {{HttpJettySolrClient}} wraps Jetty's HTTP/2 "session closed"
{{IllegalStateException}} in a bare {{{}IOException{}}}, which hits the same
classification gap. A lost session is by definition a request that was never
sent.
* The async send path needs the same handling, or async callers keep the
current behavior.
h2. Not proposed
Configuring {{{}CloudSolrClient{}}}'s internal client to use HTTP/1.1 was
considered. Both transports pool keep-alive connections under
{{{}setIdleTimeout(-1){}}}, so a reused dead connection yields the same untyped
{{IOException}} either way; it changes the odds, not the failure mode.
_(disclaimer: written by AI, of course)_
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
