Repository navigation
SOLR-18613: Consistently enforce LBSolrClient request attempt limits - #5297
Draft
nick-boss-tech wants to merge 7 commits into
Draft
nick-boss-tech wants to merge 7 commits into
nick-boss-tech wants to merge 7 commits into
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🤖 AI text below 🤖 (reviewed by Nick Shanin)
Draft status is due to Nick Shanin's open-PR cap; the change is complete and ready for review.
Jira: SOLR-18613
Context: this is PR 1 of the three-part plan proposed in the dev-list thread "Limiting LBSolrClient retries (inside CloudSolrClient & HttpShardHandler)", started by Chris Hostetter on October 26 and endorsed by Jason Gerlowski on November 2. This PR covers consistent LB enforcement only; CloudSolrClient controls (PR 2) and shard-request controls (PR 3) follow separately.
What happens today
Attempt limits are not enforced consistently across the live, zombie, and asynchronous paths.
The standalone
LBSolrClientpath can spend its configured attempt allowance on live endpoints and then continue into zombie fallback. It can also revisit failed endpoints or fail when the live snapshot is empty. In the asynchronous path, attempt-limit exhaustion is hidden behind the preceding endpoint failure.What this change does
Live and zombie dispatches share one allowance per execution, and attempt-limit exhaustion is identifiable.
LBSolrClientcounts each delegate dispatch, including the initial attempt, against the configured maximum.AttemptLimitExceededExceptioncarries the attempts made, the configured maximum, and the last failure as its cause.LBAsyncSolrClientpreserves this exception when failover stops.Standalone selection excludes endpoints already failed by that execution when the shared round-robin counter selects one again. This lets a request reach an untried healthy endpoint even when another request advances the counter between its attempts. The existing
pickServerhook still chooses from the remaining candidates. Tests cover this interleaving with limits of 3, 2, and unset, and a custom first-candidate picker.Two later commits are repairs only: a Javadoc rewrap from the formatter, and an MDC-aware executor in the new interleaving test, which the project's forbidden-apis check requires.
Proof
Verified on Linux on 2026-11-05 at
fe557633a169c34dcb835696470cc6e69ba00be4.Tidy leaves a clean tree, Error Prone compile passes, and root
check -x testpasses. Fresh test results total 49 passed, zero failures, errors, or skips:LBSolrClientAttemptLimitTest14/14,LBAsyncSolrClientAttemptLimitTest12/12,LBSolrClientTest8/8,LBAsyncSolrClientTest4/4,LBSolrClientRetryUnsentTest3/3,LB2SolrClientTest7/7, andLBHttpSolrClientBadInputTest1/1. Seed:5B407A2D9C1310E7.Both new selection tests fail against pre-fix PR head
2399d0312fa320563773ea87884ee4f534594974with only the current test file copied in. The primary request dispatches to the same failed endpoint twice and fails before reaching the healthy endpoint. Both tests pass at the verified head.Earlier synthetic-base proof established four original regressions: dispatch beyond the limit into zombie fallback, empty-live-array selection failure, repeated dispatches against an exhausted live snapshot, and hidden async exhaustion. That evidence is carried forward and was not rerun in this gate. The other 20 new methods pin existing behavior.
Limits
Nothing outside the load-balanced clients' failover accounting changes.
The allowance covers LB delegate dispatches, including the initial attempt and zombie fallback. HTTP-client-internal retries and background health checks are outside it. Existing endpoint health, zombie revival, retry eligibility, and retry-on-unsent-request rules are retained.
Changelog: changelog/unreleased/SOLR-18613-lb-attempt-limits.yml
Nick Shanin directed the work and takes responsibility for this contribution.