Nick Shanin created SOLR-18539:
----------------------------------
Summary: QueryComponent.mergeIds counts a partial shard whose
documents are skipped in the merged totals, and adds the running total as the
approximateTotalHits fallback
Key: SOLR-18539
URL: https://issues.apache.org/jira/browse/SOLR-18539
Project: Solr
Issue Type: Bug
Components: query
Reporter: Nick Shanin
AI text below.
h2. Symptom
In a distributed search sorted on a field (so sort values need unmarshalling at
merge time), when one shard answers with partial results and no
\{{sort_values}} (the \{{timeAllowed}} / query-limit case that SOLR-14758 fixed
for an NPE and SOLR-15595 narrowed), the merging node drops that shard's
documents but still counts the shard in the merged totals. The response shows:
* \{{numFound}} larger than the number of documents reachable by paging,
because it includes the skipped shard's hits
* that same count reported with \{{numFoundExact=true}}: the skipped shard's
own count is exact, so nothing about the flag is wrong on the shard, but the
total it qualifies counts a shard that contributed no documents
* a \{{maxScore}} that no returned document attains, when the skipped shard
held the highest score
* an \{{approximateTotalHits}} that includes the skipped shard's approximate
count, when the total is written to the response header (11 instead of 4 in the
reproduction below)
The response header does carry \{{partialResults=true}}, so callers know the
result set is partial; nothing tells them the count is not the count of the
merged result.
A second defect sits in the same accumulation. When a shard response carries no
\{{approximateTotalHits}} header, the merge falls back to adding \{{numFound}},
but at that point \{{numFound}} is the running total of the shards merged so
far, not the current shard's count, which is only folded in afterwards. Two
shards with 2 and 3 documents and no headers merge to an approximate total of 2
instead of 5, in arrival order A then B (A the shard with 2 documents, B the
shard with 3; in the other order the total is 3).
h2. Mechanism
{\{QueryComponent.mergeIds}} (main,
\{{solr/core/src/java/org/apache/solr/handler/component/QueryComponent.java}})
folds each shard response into the merged totals before the partial-results
skip:
* \{{approximateTotalHits}} is accumulated around line 1160
* when a shard sends no \{{approximateTotalHits}} header, the fallback at line
1162 adds \{{numFound}}, the running merged total at that point, instead of the
current shard's own count, which is only folded in at line 1171
* \{{maxScore}} is merged around line 1169
* \{{numFound += docs.getNumFound()}} at line 1171
* the \{{hitCountIsExact}} fold is at line 1174
The skip itself is at line 1193: \{{if (thisResponseIsPartial &&
sortFieldValues.size() == 0 && needsUnmarshalling) continue;}}. The only
downward adjustment, \{{numFound--}} at line 1225, runs per document when the
queue push returns false, which happens only for a document id already pushed
(from any shard), so it never compensates for a shard that was skipped
wholesale. \{{setResultIdsAndResponseDocs}} then writes the accumulated values
into the response, including \{{setNumFoundExact(hitCountIsExact)}}.
The skipped shard does not rescue the exactness flag: in the SOLR-14758
scenario the timeout lands in \{{doFieldSortValues}} after the shard's search
has completed, so the shard's own count is exact and it sends
\{{numFoundExact=true}}; more generally the shard's exact flag comes from the
DocList hit-count relation, which the partial-results state does not downgrade.
So \{{hitCountIsExact}} survives the merge. The extreme case is every shard
skipped: on main the merged count is then the skipped shards' sum (5 for shards
reporting 2 and 3) and is reported exact. After the fix the count is zero, and
that zero must not be reported exact when a skipped shard reported hits.
The response flags are not totals and are not part of this defect:
\{{partialResults}} and the max-hits flag still fold in from every shard,
including a skipped one. {{[shards.info|http://shards.info/]}} also still
reports each shard's own totals, so its per-shard numbers do not sum to the
merged totals.
h2. Reproduction
At the merge level, using the mock harness from
\{{QueryComponentPartialResultsTest}} (added by SOLR-15595) with realistic
totals set on the fabricated shard lists (that test never sets \{{numFound}},
which is why this was not visible): shard1 partial with no \{{sort_values}}, 2
documents, \{{numFound=2}}, \{{numFoundExact=true}}, \{{maxScore=99.0}}, an
\{{approximateTotalHits}} header of 7 and \{{maxHitsTerminatedEarly=true}};
shard2 normal, 3 documents, \{{numFound=3}}, \{{numFoundExact=true}},
\{{maxScore=3.0}}, an \{{approximateTotalHits}} header of 4; sort on a field
requiring unmarshalling. On main the merged list holds 3 documents (correct)
with \{{numFound=5}}, \{{numFoundExact=true}}, \{{maxScore=99.0}} and
\{{approximateTotalHits=11}} (4 with the skipped shard excluded).
For the fallback: two shards with 2 and 3 documents, neither sending an
\{{approximateTotalHits}} header, one flagging \{{maxHitsTerminatedEarly}}; on
main the merged \{{approximateTotalHits}} is 2 instead of 5 in arrival order A
then B (3 in the other order).
End to end, the ingredients are the SOLR-14758 ones: a multi-shard collection,
a sorted query, and a \{{timeAllowed}} (or another query limit) that trips on
one shard while its sort values are being derived.
h2. Fix options
Three smallest changes present themselves:
* Test the skip condition before accumulating the shard's totals
(equivalently, move the \{{approximateTotalHits}} / \{{maxScore}} /
\{{numFound}} / \{{hitCountIsExact}} accumulation below the skip), so a skipped
shard never enters the merged totals and the totals describe exactly the shards
whose documents are merged. In the extreme case where every shard is skipped,
the merged count is zero, and that zero must not be reported exact when a
skipped shard reported hits. The reorder alone does not give that: when no
shard is merged, no shard folds its flag either, so the flag keeps its initial
true. That case needs a guard on top, the second option's move applied to the
no-shard-merged case only: clear the flag when no shard was merged and a
skipped shard reported hits.
* Keep the totals, but set \{{hitCountIsExact = false}} when a shard is
skipped, so the response stops asserting the count is exact. That leaves
\{{numFound}} and \{{maxScore}} counting the skipped shard, so it corrects only
one of the symptoms above.
* Correct the fallback: when a shard sends no \{{approximateTotalHits}}
header, add that shard's own count (\{{docs.getNumFound()}}) instead of the
running merged total.
The first change is the recommended one. Correcting the totals when the skip
fires instead means subtracting the skipped shard's contribution after the
fact. That can repair the counts, but it cannot correctly undo \{{maxScore}}: a
maximum cannot be decremented, so recovering the merged maximum without the
skipped shard would need per-shard tracking and a recomputation, and
\{{hitCountIsExact}} would need recomputation as well. Testing the skip before
accumulating leaves every total describing the same set of shards, so the
reorder is the sound fix. The third change is independent of the skip and is
part of the same fix: it is one line in the same accumulation, and without it
the merged approximate total stays wrong whenever a shard sends no header.
Related: SOLR-14758, SOLR-15595. SOLR-15144 reports a different symptom in the
same area (empty document list under \{{timeAllowed}}) and does not appear to
be the same mechanism. \{{CombinedQueryComponent.mergeIds}} folds its per-query
totals in before the same partial skip in the same way. Its
\{{approximateTotalHits}} fallback also adds the running per-query total, which
already includes the current shard, so two shards of 2 and 3 documents
accumulate 2 then 7 instead of 5 in arrival order A then B (the other order
accumulates 3 then 8). Its exactness fold is inverted as well: it keeps the
merged flag true only while shards report their counts as not exact, so an
all-exact merge reports inexact and an all-inexact merge reports exact. That
component is not covered here and is left as a follow-up, as one ticket
covering all three of its defects.
AI agents assisted with research, implementation, review, and drafting. Nick
Shanin directed the work and takes responsibility for this contribution.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
