Repository navigation
SOLR-7520: The grouped path now calls complete() on the post filter after each shard search, as the non-grouped path does - #5138
Draft
nick-boss-tech wants to merge 8 commits into
Draft
nick-boss-tech wants to merge 8 commits into
nick-boss-tech wants to merge 8 commits into
Conversation
…rches Hypothetical, unrun regression test; see SOLR-7520-TESTING.md.
…ponse The shipped test asserted on the coordinator response, but merge strategies do not run for grouped responses at the coordinator, so that assertion cannot pass on this change alone. Assert on a first phase shard response instead, which is the behavior this change fixes. Also narrow the changelog title to what the change delivers, and drop the handoff doc.
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 🤖 (posted on behalf of Nick Shanin)
https://issues.apache.org/jira/browse/SOLR-7520
What happens today
A grouped distributed search never completes its post filter on the shards.
The Jira report describes a query with an
fqpost filter, several shards, and grouping. The post filter's results are missing from the response. InCommandHandler.searchWithTimeLimiterat base commit14c7aac0d15, the post filter is set as the collector (lines 254-257), the search runs, and nothing callscomplete()on the filter afterwards. The non-grouped path inSolrIndexSearcherat the same base commit does callcomplete(). The ticket names the methodfinish; the current name iscomplete().What this change does
Both distributed grouping phases now call
complete()on the post filter after each shard search, as the non-grouped path does.The first phase (
group.distributed.first) and the second phase (group.distributed.second) both runCommandHandler. The search in CommandHandler.java runs inside atry, and itsfinallycallscomplete()when a post filter is set. The coordinator code is not changed.What the coordinator returns
A distributed grouped response still has no
analyticssection, and this change does not add one.Merge strategies run only for ungrouped responses.
mergeIdsis called fromhandleRegularResponses, andhandleGroupedResponsesruns no merge strategy, so the coordinator never combines the shard sections. This PR puts the section into shard responses only, and the coordinator's grouped result is the same as before. Returning a mergedanalyticssection for grouped requests needs a design decision first, because both phases now carry the section and the follow-up must say which one to merge. I suggest a separate ticket for that.Proof
The first-phase assertion fails on the base code and passes with this change.
The AnalyticsMergeStrategyTest tests check three things. The first-phase and second-phase requests to one shard each expect
mycount3, since shard 0 holds docs 1, 2, and 7. A grouped request through the coordinator expects 11 groups and 11 matches.On base code
14c7aac0d15, with these tests, the first-phase assertion fails with "analytics section missing for grouped shard query" and the second-phase method fails with "analytics section missing for grouped second phase shard query".Verified at
10b6931e1c0(gate, 2026-10-05): AnalyticsMergeStrategyTest 1 of 1, AnalyticsQueryTest 1 of 1, TestGroupingSearch 17 of 17, TestDistributedGrouping 2 of 2. That gate covered the earlier assertion, which checked only that the section was present. The count, second-phase, and coordinator checks were added at209cb27f1a4, and the second-phase check then moved into its own test method ata6dc792b36. Verified 2026-10-15 at that head: AnalyticsMergeStrategyTest 2 of 2, and on the base code both methods fail, as described above. One formatting fix is still owed at this head: the project's formatter joins one wrapped call in the second-phase test onto a single line, so the test file is not yet in canonical format.Limits
analyticsvalue.5and50.group.distributed.*) are unchanged.Changelog: changelog/unreleased/SOLR-7520-grouping-postfilter-complete.yml
AI assistance
AI agents assisted with research, implementation, review, and drafting. Nick Shanin directed the work and takes responsibility for this contribution.