[jira] [Resolved] (SOLR-18401) HttpJettySolrClient: detect if request wasn't sent; means retry-able

["David Smiley (Jira)" <ji...@apache.org>]

     [ https://issues.apache.org/jira/browse/SOLR-18401?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

David Smiley resolved SOLR-18401.
---------------------------------
    Fix Version/s: 10.1
       Resolution: Fixed

> 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
>            Priority: Major
>              Labels: pull-request-available
>             Fix For: 10.1
>
>          Time Spent: 1h
>  Remaining Estimate: 0h
>
> 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: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org