Skip to content

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
apache:mainfrom
nick-boss-tech:solr-7520-submit
Draft

nick-boss-tech wants to merge 8 commits into
apache:mainfrom
nick-boss-tech:solr-7520-submit

Conversation

@nick-boss-tech

@nick-boss-tech nick-boss-tech commented Oct 14, 2026 •

Copy link
Copy Markdown
Contributor

🤖 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 fq post filter, several shards, and grouping. The post filter's results are missing from the response. In CommandHandler.searchWithTimeLimiter at base commit 14c7aac0d15, the post filter is set as the collector (lines 254-257), the search runs, and nothing calls complete() on the filter afterwards. The non-grouped path in SolrIndexSearcher at the same base commit does call complete(). The ticket names the method finish; the current name is complete().

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 run CommandHandler. The search in CommandHandler.java runs inside a try, and its finally calls complete() when a post filter is set. The coordinator code is not changed.

What the coordinator returns

A distributed grouped response still has no analytics section, and this change does not add one.

Merge strategies run only for ungrouped responses. mergeIds is called from handleRegularResponses, and handleGroupedResponses runs 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 merged analytics section 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 mycount 3, 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 at 209cb27f1a4, and the second-phase check then moved into its own test method at a6dc792b36. 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

  • The coordinator gap above remains. The coordinator check covers grouped counts, not a merged analytics value.
  • The shard checks read shard 0 only. The second-phase check uses fixed top-group values, 5 and 50.
  • Ungrouped requests and standalone grouped requests (without 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.

…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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant