[
https://issues.apache.org/jira/browse/SOLR-9355?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18099778#comment-18099778
]
Serhiy Bzhezytskyy commented on SOLR-9355:
------------------------------------------
Coming back to this after running into the same behaviour from a different
direction. Three answers to the questions in the description, and one defect
that I think is why the retry you wanted still does not always fire.
"Whether there is retrying higher in the stack I'm not quite sure"
There is now, though it arrived two years after this issue: SOLR-11881 added
retry for leader-to-follower updates in 7.5, in SolrCmdDistributor.
StdNode.checkRetry decides retriability and Req.shouldRetry bounds it at
solr.retries.to.followers (default 3), excluding delete-by-query.
"whether retrying at this point in the process is something that will avoid
versioning problems"
That got answered on SOLR-11881 rather than here. Mark Miller, 2018-05-02, on
why SocketException had previously been excluded:
_Yes it was, because it can happen mid request and we don't know if the request
failed or succeeded. Given we are counting on versions for retry though, this
actually shouldnt matter, so that should be fine._
So versioning is what makes an ambiguous replay harmless on this path, and that
is why the distributor retries types the user client cannot.
"a simple retry could avoid the replica going into recovery if the problem is
transient"
Still true, and still not happening for one common shape. checkRetry asks the
retriability question two different ways depending on which exception happens
to be outermost:
if (err.e instanceof SolrServerException) {
if (isRetriableException(((SolrServerException) err.e).getRootCause()))
return true; // unwraps
} else {
if (isRetriableException(err.e)) return true;
// top-level type only
}
isRetriableException is a leaf test on SocketException /
SocketTimeoutException. The async client reports a connection failure as a raw
ExecutionException wrapping the socket cause, so it takes the second branch,
the leaf test sees ExecutionException, and the update is not retried — the
replica goes into recovery on exactly the transient glitch described here.
ForwardNode.checkRetry has the same asymmetry, narrowed to ConnectException.
I have a PR up that makes both scan the cause chain instead of one fixed
position in it. The retriable set is unchanged and the retry bound is
untouched; only where the type is looked for changes. Tests were written first:
two of them fail on unmodified main and pass after.
Worth noting the chain has to be scanned rather than unrolled to the root
cause, because the retriable frame is not always at either end. Jetty's
ClientConnector wraps an underlying failure in a SocketException of its own, so
going straight to the root cause walks past it.
One thing this does not resolve, and a flag
This fixes classification, not the wider question of which exceptions should be
retriable. That question was asked on SOLR-11881 by Varun Thacker while adding
SocketTimeoutException — "Q: What all exceptions should we retry on?" — and
never answered.
Five places decide retriability today, each with its own set. Running each
predicate against real exception instances (main):
||exception||StdNode :591||ForwardNode :667||wasCommError :211||PeerSync
:443||LBSolrClient :687||
|ConnectException|retry|retry|retry|retry|retry|
|SocketException|retry| |retry| | |
|SocketTimeoutException|retry| | | | |
|UnknownHostException| | |retry| | |
|ClosedChannelException| | | | | |
|EOFException| | | | | |
ConnectException is the only type all five agree on. SocketTimeoutException is
retriable on the distributor path but not the cloud-client path, and
UnknownHostException the reverse; neither asymmetry looks deliberate.
ClosedChannelException is in none of them, and it is the root cause the JDK
transport reports for a dropped update connection — SOLR-17764 and PR #4643
cover the CloudSolrClient half of that, the distributor half is untouched.
(NoRouteToHostException is omitted from the table because it follows
SocketException by inheritance rather than by any explicit choice.)
Two smaller things underneath, both pre-existing: SolrException.getRootCause
carries "// TODO: This doesn't handle cause loops" and walks unbounded, and
PeerSync.connectTimeoutExceptionInChain hand-rolls the same unbounded walk — a
cause chain can be made cyclic, so both can spin.
LBSolrClient.isConnectException falls back to matching a class-name suffix to
avoid a hard dependency on a specific HTTP client, which reads less like
carelessness than like a missing shared way to ask this question.
I will raise that separately as a design question rather than widen anything
here.
> 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
>
> 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: [email protected]
For additional commands, e-mail: [email protected]
