chan-dx commented on code in PR #4829:
URL: https://github.com/apache/solr/pull/4829#discussion_r3945176957
##########
solr/solrj/src/java/org/apache/solr/client/solrj/impl/LBSolrClient.java:
##########
@@ -672,31 +669,27 @@ protected Exception doRequest(
if (!isNonRetryable
&& (rootCause instanceof IOException || rootCause instanceof
TimeoutException)) {
ex = (!isZombie) ? makeServerAZombie(baseUrl, e) : e;
- } else if (isNonRetryable
- && (isConnectException(rootCause)
- || SolrException.hasCause(e, RequestNotSentException.class))) {
+ } else if (isNonRetryable && getClient(baseUrl).wasRequestUnsent(e)) {
// 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;
}
+ } catch (IOException e) {
Review Comment:
Fixed, four catch blocks to one, via a new protected `mayFailOver(...)`
(LBSolrClient lines
[645](https://github.com/apache/solr/pull/4829/changes#diff-8a5ab47986f979f1134864e1352f6617e8d425f756018da7e0c1fa191fcc8fafR645),
[662](https://github.com/apache/solr/pull/4829/changes#diff-8a5ab47986f979f1134864e1352f6617e8d425f756018da7e0c1fa191fcc8fafR662))
`LBAsyncSolrClient` shares it, so its near-copy is gone too.
**Notes:**
Behaviour-identical except one row. `wasRequestUnsent` sat behind
`isNonRetryable`, so `&&` short-circuited it away for retryable requests and
only `getRootCause()` decided. That misses [HttpJettySolrClient lines
525](https://github.com/apache/solr/pull/4829/changes#diff-d9d3226ba9b2b957fefa595b02eaaea2bf33d62b14cbe22792cd086441ec4d03L525)'s
pre-commit shape: `SolrServerException` -> `RequestNotSentException` ->
`IllegalStateException`, where the root cause is the `IllegalStateException`.
Effect: an update proven unsent fails over, an identical query doesn't;
[`testQueryIsRetriedWhenUnsentButRootCauseIsNotIO`](https://github.com/apache/solr/pull/4829/changes#diff-c0dc3e29a92f73ff3bb17b34c0642bcfe16e62e26f38f6b5e6299f475b07efa4R156)
covers it. That's a behaviour improvement, separable from the refactor. Let me
know if you'd rather I reverse it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
