[
https://issues.apache.org/jira/browse/SOLR-18492?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18121653#comment-18121653
]
Marc Byrd commented on SOLR-18492:
----------------------------------
Update: I now have a deterministic reproduction, and a test that fails on main
and passes with the suggested one-line fix.
The trigger is a collection with no replicas, not a timing race.
getRemoteCoreUrl() falls back to replicas in any state (down, recovering, or on
a dead node) when it can't find an active one. So it only returns null, which
is what leads to action = RETRY, when the cluster state shows no replica on
another node at all. Stale cluster state alone rarely gets there, which
explains why my earlier repro attempts failed. Cases that do get there include:
- a collection with zero replicas (for example created with createNodeSet=EMPTY)
- a collection whose only replicas are assigned to this node, but whose cores
aren't loaded (during startup, or after a failed core init)
- a collection not yet in this node's cluster state, or one with no active
slices
Repro: create a collection with createNodeSet=EMPTY, then send the same select
request via V1 (/solr/<coll>/select) and via V2 (/api/c/<coll>/select). Both
return 404. V1 goes through SolrServlet's RETRY re-dispatch first (you can see
the "Action RETRY" span event); V2 never does.
Test: TestV2RetryOnMissingCore in the opentelemetry module, using the same
in-memory span exporter setup as TestDistributedTracing. It asserts that the
"Action RETRY" event fires for both the V1 and the V2 request. On current main
it fails (V1 retried, V2 did not); with "if (action == RETRY) return;" added
after the REMOTEPROXY check in V2HttpCall.init() it passes. Happy to open a PR
with the fix and test.
Two corrections to the original description:
1. Scope is narrower than I suggested. V2 admin endpoints are unaffected in
practice: GET /api/collections/<coll> on the same empty collection returns 200
with correct data, because those requests end up as ADMIN anyway. Only
core-level V2 requests (select, update, etc. under /api/c/<coll>/) lose the
retry. So the "more exposed in 10.1 because the Admin UI is V2-only" argument
is weaker than I made it sound.
2. Impact is limited. V2HttpCall still calls forceUpdateCollection() before the
RETRY is lost, so the next request sees fresh state. The practical effect is
one failed request per stale-state event, rather than a transparent retry.
Minor priority seems more accurate than Major.
A small related oddity: in this situation V2 returns "Cannot find API for the
path: /c/<coll>/select", which points at a missing API rather than a collection
with no available replica. The one-line fix doesn't change that message, since
the retry pass ends up in the same admin handling. Mentioning it in case it's
worth a separate look.
> V2HttpCall never retries on stale cluster state - RETRY action silently
> overwritten to ADMIN
> --------------------------------------------------------------------------------------------
>
> Key: SOLR-18492
> URL: https://issues.apache.org/jira/browse/SOLR-18492
> Project: Solr
> Issue Type: Bug
> Affects Versions: 10.0, 10.1
> Reporter: Marc Byrd
> Priority: Minor
>
> Found as a side discovery while working SOLR-18487 (see that ticket and
> {{apache/solr#4973}}) - a separate, pre-existing issue, but with the same
> exposure pattern as that bug.
> {{HttpSolrCall.extractRemotePath()}} sets {{action = RETRY}} (when it can't
> resolve a remote core URL for a collection, e.g. due to stale local ZK state)
> without returning:
> {code:java}
> cores.getZkController().zkStateReader.forceUpdateCollection(collectionName);
> action = RETRY;
> // falls through here, no return
> {code}
> {{V2HttpCall.init()}} only checks for {{action == REMOTEPROXY}} afterward -
> no equivalent check for {{RETRY}}:
> {code:java}
> if (core == null) {
> extractRemotePath(collectionName);
> if (action == REMOTEPROXY) {
> action = ADMIN_OR_REMOTEPROXY;
> ...
> return;
> }
> // no check for RETRY here
> }
> ...
> if (core == null) {
> initAdminRequest(path); // unconditionally sets action = ADMIN
> return;
> }
> {code}
> So when {{action == RETRY}}, execution falls through to the unconditional
> {{core == null}} check below, overwriting {{action}} to {{ADMIN}}.
> {{SolrServlet.dispatch()}}'s retry handling ({{case RETRY ->
> dispatch(request, response, true)}}) can therefore never fire for a V2
> request - a stale-cluster-state V2 request gets silently misclassified as a
> plain admin request instead of transparently retrying after the forced state
> refresh.
> V1 already handles this correctly - {{HttpSolrCall.init()}}'s own (non-V2)
> dispatch logic has {{if (action != null) return;}} immediately after its
> equivalent {{REMOTEPROXY}} check, letting {{RETRY}} reach
> {{SolrServlet.dispatch()}} properly. {{V2HttpCall}}'s override is simply
> missing the equivalent line.
> Age: confirmed present in {{releases/solr/10.0.0}} and current
> {{branch_10_1}}/{{main}} - this predates 10.1 and is not a new regression.
> Why this matters more in 10.1: the defect is old, but exposure to it has
> grown, for the same reason as SOLR-18487/SOLR-18324. Before 10.1, an admin
> action hitting this exact staleness condition from the Admin UI went through
> V1 (which already handles {{RETRY}} correctly). SOLR-15752 made the Admin UI
> V2-exclusive, so the same action now routes through the broken path
> unconditionally. Separately, SOLR-18072 and related V2 work keep adding admin
> operations with no V1 equivalent at all, so some operations have no fallback
> to mask this gap even for users who haven't deliberately adopted the V2 UI.
> Same shape as SOLR-18487: a latent gap in V2's cross-node dispatch plumbing,
> freshly exposed by 10.1 removing the V1 safety net that was accidentally
> covering for it.
> Suggested fix: add {{if (action == RETRY) return;}} right after the existing
> {{REMOTEPROXY}} check in {{V2HttpCall.init()}}, mirroring {{HttpSolrCall}}'s
> own correct pattern.
> Reproduction: confirmed only by direct code reading. Four separate attempts
> at an automated repro (immediate cross-node query, async collection creation
> exploiting the window before a replica is marked active, querying a node
> immediately after it joins an already-populated cluster, and investigating
> direct {{ZkStateReader}} cache manipulation) did not succeed without
> resorting to reflection into private internals or hand-crafted raw ZK state,
> which seemed too fragile to be worth it. This suggests the staleness window
> may be narrow in practice even though the dead-code path itself is
> unambiguous.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
