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

Jason Gerlowski commented on SOLR-17764:
----------------------------------------

I'd love to restart this discussion, if folks are open to it. It's kindof a 
shame that our shutdowns aren't graceful out of the box, and it feels like 
we're close to that if we can just solve this issue.

To revisit what seems like the main question earlier in the thread...
{quote}[Hoss] Why does this code care if (exc instanceof RouteException) ? ... 
why doesn't it retry on (wasCommError || (errorCode == 503)) ?
{quote}
I did a bit of git-spelunking here. A few findings:
 * RouteException was added as a part of SOLR-4816 to wrap any errors in CSC's 
update-routing code. Conceptually it's not wrapping a particular type of error 
(i.e. "comms errors only"). Instead it was initially intended as a generic 
wrapper for any exception arising from that update-routing codepath.
 * SOLR-11484 added the "exc instanceof RouteException" bit (without any status 
code clause), as a way to have CSC invalidate its clusterstate cache and retry. 
The intent here seems to have been along the lines of: "Any time we hit any 
problem doing fancy update-routing, just refresh the cache and try again"
 * Over time we noticed this was overly aggressive. SOLR-12649 narrowed 
RouteException retries to only happen on a 404 or 503 status code, and 
SOLR-12801 eventually dropped the "404" case leaving us with the logic that's 
there today. Neither ticket offers any rationale about why those particular 
status codes were picked, but...
 ** if I had to guess {*}I'd imagine that the handling of "503" wasn't trying 
to anticipate the node-shutdown case at all{*}, but rather some of the other 
scenarios where Solr emits this status code (e.g. [the receiving core is still 
loading|https://github.com/apache/solr/blob/main/solr/core/src/java/org/apache/solr/servlet/HttpSolrCall.java#L266],
 [the core is currently read 
only|https://github.com/apache/solr/blob/main/solr/core/src/java/org/apache/solr/update/DefaultSolrCoreState.java#L108],
 various [HttpShardHandler error 
cases|https://github.com/apache/solr/blob/main/solr/core/src/java/org/apache/solr/handler/component/HttpShardHandler.java#L255]
 around unhealthy clusterstate, etc.).

My main takeaway from this: node-shutdown seems to be absent from the design of 
this logic entirely!

AFAICT there's no evidence that past JIRAs were trying to address that 
particular scenario. And that's not a "knock" on those efforts - the 
graceful-shutdown capability added by SOLR-17744 is much newer than any of the 
JIRAs mentioned above, so there's no way they could've accommodated it.

All of this is to say: CloudSolrClient's 503 handling was never intended to 
handle node-shutdown, but it seems like there's consensus 
([David|https://issues.apache.org/jira/browse/SOLR-17764?focusedId=17979340&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-17979340],
 
[Hoss|https://issues.apache.org/jira/browse/SOLR-17764?focusedId=17980470&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-17980470]
 (perhaps slightly less clear) and 
[Mark|https://issues.apache.org/jira/browse/SOLR-17764?focusedId=17980492&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-17980492])
 that we should tweak the CSC logic to handle this new scenario.
----

It feels like we're close to consensus on this.  What do folks think about 
changing the retry-gating to:
{code:java}
    if (wasCommError || (errorCode == 503))
{code}

If there's not big objections I can open a PR with that approach?

> "graceful" jetty shutdown causes ChaosMonkeySafeLeaderWithPullReplicasTest 
> failures
> -----------------------------------------------------------------------------------
>
>                 Key: SOLR-17764
>                 URL: https://issues.apache.org/jira/browse/SOLR-17764
>             Project: Solr
>          Issue Type: Bug
>            Reporter: Chris M. Hostetter
>            Priority: Major
>              Labels: pull-request-available
>         Attachments: 
> E7F93005B9386058.OUTPUT-org.apache.solr.cloud.ChaosMonkeySafeLeaderWithPullReplicasTest.txt
>
>          Time Spent: 50m
>  Remaining Estimate: 0h
>
> Reviewing recent jenkins test failure metrics, I noticed that (Nightly) test 
> ChaosMonkeySafeLeaderWithPullReplicasTest started failing ~60% of the time 
> right around the time that SOLR-17744 was committed.
> Things i have observed:
>  * Seeds from failing runs seem to reliably reproduce the failure
>  ** These failures do *NOT* reproduce if i revert to just before SOLR-17744
>  * Ad-hoc testing I've done of seeds that do _not_ fail on first attempt seem 
> to reliably succeed on all subsequent attempts
>  ** Suggesting that the root cause is something deterministic in the 
> {{{}random(){}}}-ness of the test, and not something dependent on timing or 
> concurrency.



--
This message was sent by Atlassian Jira
(v10.3.26#10030026)

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to