[jira] [Comment Edited] (SOLR-9355) ConcurrentUpdateSolrClient does not retry sending docs to followers

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

    [ https://issues.apache.org/jira/browse/SOLR-9355?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18103720#comment-18103720 ] 

David Smiley edited comment on SOLR-9355 at 8/11/26 5:02 PM:
-------------------------------------------------------------

_(99% AI generated):_ Closing this after ten years. The behavior it describes no longer exists, and the component it names (CUSC) was never responsible for it. Remaining improvements can being tracked in new issues rather than by rescoping this one.

h3. The observation was right; the attribution was not

The title — "does not retry sending docs to followers" — was accurate when filed. At that time {{SolrCmdDistributor.StdNode.checkRetry()}} was:

{code:java}
@Override
public boolean checkRetry() {
  return false;
}
{code}

{{StdNode}} is the leader-to-follower node, so follower sends were structurally excluded from retry. The retry machinery existed and worked — the triage loop handled 404/403/503 and {{ConnectException}}, with a retry counter — but only {{ForwardNode}} could reach it. The bound is named {{maxRetriesOnForward}}, which says as much.

The gate was therefore never {{ConcurrentUpdateSolrClient}}. That class is a queueing/streaming client with no per-request routing context and no knowledge of leaders or replicas; it does not decide retries and never did. Today's {{ConcurrentUpdateBaseSolrClient}} still performs no network retry — its only retry loop is contention on offering to the internal queue. Retry has always been decided in {{SolrCmdDistributor}}, where the routing knowledge lives.

h3. Why it is stale

* {{StdNode.checkRetry}} is no longer {{return false}}. It carries a real retriable test, so the reported behavior has been fixed by subsequent work.
* Leader Initiated Recovery, the harm described here, no longer exists — SOLR-11702 replaced it with {{ZkShardTerms}}. A failed replica send now marks the replica behind via {{ensureTermsIsHigher}} and sends it into recovery. Comparable damage, but the description's mechanism is gone.

h3. Answering the open question in the description

The description asks whether retrying at this point could cause versioning issues. It cannot, and it is worth recording since this uncertainty may be part of why the issue sat as long as it did.

The leader assigns {{_version_}} in {{versionAdd}} before fan-out in {{doDistribAdd}}, and a replica discards a duplicate via the drop rule ({{Math.abs(lastVersion) >= versionOnUpdate}}). Atomic update operators are resolved into a materialized document on the leader before distribution, so no {{inc}} ever crosses the leader-to-replica hop. Retrying to a follower is idempotent by construction.

That reasoning is specific to the leader-to-replica hop. It does not extend to the client-to-leader hop, where operations are still unresolved and a replayed {{inc}} does double-apply.

h3. Follow-up work

To be filed separately rather than re-scoping this issue:

# *Cause-chain unrolling in {{SolrCmdDistributor}}.* Both {{StdNode.checkRetry}} and {{ForwardNode.checkRetry}} test one fixed position in the exception cause chain, so a retriable cause wrapped by the async client or by Jetty's {{ClientConnector}} is missed and the replica goes into recovery on a transient glitch. PR #4678 already implements this and will be repointed.
# *{{ClosedChannelException}} retriability.* This is what the JDK transport reports as the root cause of a dropped update connection, and it is retriable nowhere in Solr today. PR #4678 names the gap and deliberately does not widen it.

_removed irrelevant bullet_



was (Author: dsmiley):
_(99% AI generated):_ Closing this after ten years. The behavior it describes no longer exists, and the component it names (CUSC) was never responsible for it. Remaining improvements can being tracked in new issues rather than by rescoping this one.

h3. The observation was right; the attribution was not

The title — "does not retry sending docs to followers" — was accurate when filed. At that time {{SolrCmdDistributor.StdNode.checkRetry()}} was:

{code:java}
@Override
public boolean checkRetry() {
  return false;
}
{code}

{{StdNode}} is the leader-to-follower node, so follower sends were structurally excluded from retry. The retry machinery existed and worked — the triage loop handled 404/403/503 and {{ConnectException}}, with a retry counter — but only {{ForwardNode}} could reach it. The bound is named {{maxRetriesOnForward}}, which says as much.

The gate was therefore never {{ConcurrentUpdateSolrClient}}. That class is a queueing/streaming client with no per-request routing context and no knowledge of leaders or replicas; it does not decide retries and never did. Today's {{ConcurrentUpdateBaseSolrClient}} still performs no network retry — its only retry loop is contention on offering to the internal queue. Retry has always been decided in {{SolrCmdDistributor}}, where the routing knowledge lives.

h3. Why it is stale

* {{StdNode.checkRetry}} is no longer {{return false}}. It carries a real retriable test, so the reported behavior has been fixed by subsequent work.
* Leader Initiated Recovery, the harm described here, no longer exists — SOLR-11702 replaced it with {{ZkShardTerms}}. A failed replica send now marks the replica behind via {{ensureTermsIsHigher}} and sends it into recovery. Comparable damage, but the description's mechanism is gone.

h3. Answering the open question in the description

The description asks whether retrying at this point could cause versioning issues. It cannot, and it is worth recording since this uncertainty may be part of why the issue sat as long as it did.

The leader assigns {{_version_}} in {{versionAdd}} before fan-out in {{doDistribAdd}}, and a replica discards a duplicate via the drop rule ({{Math.abs(lastVersion) >= versionOnUpdate}}). Atomic update operators are resolved into a materialized document on the leader before distribution, so no {{inc}} ever crosses the leader-to-replica hop. Retrying to a follower is idempotent by construction.

That reasoning is specific to the leader-to-replica hop. It does not extend to the client-to-leader hop, where operations are still unresolved and a replayed {{inc}} does double-apply.

h3. Follow-up work

Filed separately rather than rescoping this issue:

# *Cause-chain unrolling in {{SolrCmdDistributor}}.* Both {{StdNode.checkRetry}} and {{ForwardNode.checkRetry}} test one fixed position in the exception cause chain, so a retriable cause wrapped by the async client or by Jetty's {{ClientConnector}} is missed and the replica goes into recovery on a transient glitch. PR #4678 already implements this and will be repointed.
# *{{ClosedChannelException}} retriability.* This is what the JDK transport reports as the root cause of a dropped update connection, and it is retriable nowhere in Solr today. PR #4678 names the gap and deliberately does not widen it.
# *Client-side retry-ability.* {{CloudSolrClient}} and {{LBSolrClient}} each infer retry-ability from a different signal and disagree with each other, and neither inspects what the request contains. SOLR-18341 (new) proposes a virtual method on {{SolrRequest}} so each request declares whether it may be re-sent, with {{UpdateRequest}} overriding it to reject the two operations that are genuinely unsafe to replay.


> ConcurrentUpdateSolrClient does not retry sending docs to followers
> -------------------------------------------------------------------
>
>                 Key: SOLR-9355
>                 URL: https://issues.apache.org/jira/browse/SOLR-9355
>             Project: Solr
>          Issue Type: Improvement
>    Affects Versions: 5.3
>            Reporter: Erick Erickson
>            Priority: Major
>              Labels: pull-request-available
>          Time Spent: 20m
>  Remaining Estimate: 0h
>
> I'm a little out of my comfort zone, so mostly posting this for discussion. If, for any reason, ConcurrentUpdateSolrClient doesn't get a response from a replica (say a network glitch or the like), it _seems_ to initiate a LIR on the replica. This is around line 243 in 5.3.2, and the corresponding code in 6x looks unchanged on a quick perusal.
> This is fragile in the sense that a simple retry could avoid the replica going into recovery if the problem is transient.
> Whether there is retrying higher in the stack I'm not quite sure, but we're seeing this behavior in the field so I'm posting this here for discussion, I've had a back-channel discussion already so a JIRA seems in order.
> The other thing I'm not entirely sure of is whether retrying at this point in the process is something that will avoid versioning problems.



--
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