SOLR-18304: Optimize collapse performance for String fields in Solr 9.x and later by bartoszfidrysiak · Pull Request #4621 · apache/solr

bartoszfidrysiak · 2026-07-07T16:26:50Z

Description

Migrating to Lucene90DocValuesProducer in Solr 9 revealed a significant performance regression in collapse queries sorted by string fields, due to the extra overhead of LZ4 decompression. Solr 9’s collapse implementation does not apply any optimizations and always calls Lucene’s TermOrdValLeafComparator.copy(), which triggers LZ4.decompress() for every document processed by the collapse query. This decompression overhead did not exist in Solr 8.

SortFieldsCompare.testAndSetGroupValues() / setGroupValues()
  → TermOrdValLeafComparator.copy()
    → BaseSortedDocValues.lookupOrd()
      → TermsDict.seekExact()
        → TermsDict.decompressBlock()
          → LZ4.decompress()   ← bottleneck

Solution

This PR proposes two improvements:

Load string-sorted doc values lazily for group heads, materializing the string only when a competing document appears.
Avoid loading or materializing string-sorted doc values for documents in the same segment during collapse. Use ordinals instead - they’re numeric, cheaper to compare, and don’t need decompression.

The first improvement focuses on scenarios where many collapse groups contain only a single document, or where collapse sorting uses multiple fields with a string field acting as a tie-breaker.

The second improvement is expected to deliver major gains in cases where many documents originate from the same segment.

Tests

Four new tests were added in TestCollapseQParserPlugin:

testCollapseStringSortLazyLoadingTieDoesNotEvictGroupHead - verifies that when two documents in the same group have an equal string sort value, the first-seen document remains the group head (a tie must not trigger eviction). Covers both single-segment (ordinal fast path) and multi-segment (slow path) cases.
testCollapseStringSortOrdinalFastPathMultiClauseTieBreaking - verifies that when clause-1 of a multi-clause sort ties on ordinal comparison, clause-2 correctly decides the winner. Also exercises the remaining-values copy loop with a cross-segment competitor.
testCollapseStringSortWithoutDocValuesSkipsLazyLoadingAndOrdinalFastPath- verifies that sorting on a string field without SORTED DocValues produces correct results via the eager field-comparator path, ensuring the lazy loading and ordinal fast path are safely bypassed when unavailable.
testCollapseStringSortOrdinalFastPathDescendingWithMissingValues - verifies that missing values rank last even under a descending sort, where the missing-value sentinel (missingOrd = -1) combined with reverseMul = -1 must still produce the correct ordering in the ordinal fast path.

In addition to TestCollapseQParserPlugin, the CollapsingSearch benchmark was introduced to compare average execution times for collapse queries across different sort field combinations. In the benchmark, documents from the same groups are distributed evenly across all segments. The benchmark was executed locally on my machine against Solr 9.10.1, 10.x branch, main branch, and corresponding snapshot versions, which include the enhancements.

Solr 9.10.1 vs Solr 9.10.1-SNAPSHOT

10.x branch vs 10.x-SNAPSHOT

main branch vs main-SNAPSHOT

Conclusions

The collapseByDateAndStr benchmark shows that the snapshot (with the enhancements) performs significantly better regardless of the number of segments. This is because the string field serves only as a tiebreaker in the collapse sort, so in most cases comparing dates is sufficient to determine the winner. In addition, string doc values are loaded lazily, which avoids eagerly materializing the string value when it is not needed.
The collapseByStr benchmark shows that the snapshot (with the enhancements) delivers significantly better performance only when the number of segments is small, especially when most documents from the same group are located in the same segment. In the single-segment case, string doc values do not need to be materialized to pick a winner, since comparing element ordinals is enough and is both safe and efficient. According to the benchmark data, these two improvements combined made collapseByStr much faster for a single segment. With many segments, however, and with documents from the same groups spread evenly across them, the ordinal fast path provides no benefit because most comparisons still require string materialization.

Checklist

Please review the following and check all that apply:

I have reviewed the guidelines for How to Contribute and my code conforms to the standards described there to the best of my ability.
I have created a Jira issue and added the issue ID to my pull request title.
I have given Solr maintainers access to contribute to my PR branch. (optional but recommended, not available for branches on forks living under an organisation)
I have developed this patch against the main branch.
I have run ./gradlew check.
I have added tests for my changes.
I have added documentation for the Reference Guide
I have added a changelog entry for my change

…rdinal fast path comparison for string sorted doc values from the same segment

…ze()

dsmiley

Great work here!

dsmiley · 2026-07-10T04:43:22Z

@@ -0,0 +1,7 @@
+title: Fix collapse on String performance regression due to Lucene upgrade
+type:


"changed"

This is an optimization; I would use wording reflective of this. If theoretically it was fast and then became slow for a short period, we could say this is a regression. But it's been so long that I don't think we can think of this as a regression in terms of category here and telling users about it. The title could add more info the affected versions that were slower. Title is a misnomer... you can use a few sentences if you wish.

Good point 👍 Let me set "changed" value in the change log type. I will also change PR title to: "Optimize collapse performance for String fields in Solr 9.x and later"

dsmiley · 2026-07-10T04:45:25Z

+
+    private int getActualSegmentCount(SolrBenchState state)
+        throws SolrServerException, IOException {
+      SolrQuery lukeQuery = new SolrQuery();


Don't use SolrQuery & client.query for anything other than search. Here use LukeRequest.

Actually, fetching the current number of segments was redundant, so I removed it. I initially added it to verify that the benchmark was indexing documents into the expected segments, but once I confirmed that, it was no longer needed.

dsmiley · 2026-07-10T04:58:27Z

+        if (values[testClause] instanceof LazyStringValue) {
+          LazyStringValue headVal = (LazyStringValue) values[testClause];


newer Java means can be one line

dsmiley · 2026-07-10T05:03:42Z

+    if (ord == missingOrd || ord < 0) {
+      return null;
+    }
+    return BytesRef.deepCopyOf(dv.lookupOrd(ord));


IMO the deepCopy should be a decision by the caller... and we add a line of javadocs to say the caller shouldn't hold a reference to it.

Well there's only one caller so... okay. Still; a bit of javadoc and calling this method materializeCopy would be better.

IMO the deepCopy should be a decision by the caller... and we add a line of javadocs to say the caller shouldn't hold a reference to it.

Fully agree 👍 Moved BytesRef.deepCopyOf to CollapsingQParserPlugin.

Copilot

Pull request overview

This PR addresses a Solr 9 collapse-query performance regression when collapse sorting includes STRING fields backed by Lucene’s SORTED DocValues (where per-doc lookupOrd() can trigger frequent LZ4 decompression after the Lucene upgrade). It does so by deferring string materialization until needed and by comparing ordinals within the same segment to avoid decompression work.

Changes:

Teach collapse group-head comparison to store STRING sort values lazily (store (dv, ord) and only lookupOrd() when a competitor actually requires it).
Add an ordinal “fast path” for STRING sort comparisons when both documents are from the same segment (safe ordinal comparison).
Add new correctness tests for ties, multi-clause tie-breaking, non-DV STRING sorts, and descending sorts with missing values; add a benchmark and benchmark schema support.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
solr/core/src/java/org/apache/solr/search/CollapsingQParserPlugin.java	Implements lazy STRING sort value handling and same-segment ordinal comparison fast path inside collapse group-head selection.
solr/core/src/java/org/apache/solr/search/LazyStringValue.java	New helper to defer `SortedDocValues.lookupOrd()` and deep-copy only when materialization is required.
solr/core/src/test/org/apache/solr/search/TestCollapseQParserPlugin.java	Adds new tests covering tie behavior, multi-clause tie-breaking, non-DV STRING sorts, and missing-value handling in descending sorts.
solr/benchmark/src/java/org/apache/solr/bench/search/CollapsingSearch.java	Adds a JMH benchmark to measure collapse performance across different sort patterns/segment counts.
solr/benchmark/src/resources/configs/cloud-minimal/conf/schema.xml	Adds `*_s_dv` dynamic field to support benchmark string docValues fields.
changelog/unreleased/SOLR-18304-fix-collapse-on-string-performance.yml	Adds changelog entry for SOLR-18304 (currently missing required `type` value).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

…edundant.

…patible with PR title

…stAndSetGroupValues method

…nchmark:spotlessJavaCheck

…lapsingQParserPlugin.java

bartoszfidrysiak added 6 commits July 7, 2026 18:19

bugfix: lazy string loading while initializing new collapse group & o…

70f6dac

…rdinal fast path comparison for string sorted doc values from the same segment

Added CollapsingSearch benchmark

e16d74a

Fixed issues in CollapsingSearch: forbidden APIs & code reformatting

53e7c16

fix: deep copy BytesRef from lookupOrd() in LazyStringValue.materiali…

826b05e

…ze()

fix: CollapsingSearch adjusted to use SolrBenchState in Solr 10

f296e9b

Added changelog: SOLR-18304-fix-collapse-on-string-performance.yml

47da758

github-actions Bot added tests cat:search labels Jul 7, 2026

bartoszfidrysiak marked this pull request as ready for review July 7, 2026 16:49

dsmiley requested review from dsmiley and joel-bernstein July 10, 2026 04:31

dsmiley reviewed Jul 10, 2026

View reviewed changes

dsmiley requested a review from Copilot July 10, 2026 05:06

Copilot started reviewing on behalf of dsmiley July 10, 2026 05:06 View session

Copilot AI reviewed Jul 10, 2026

View reviewed changes

Comment thread changelog/unreleased/SOLR-18304-fix-collapse-on-string-performance.yml Outdated

bartoszfidrysiak changed the title ~~SOLR-18304: Fix collapse on String performance regression due to Lucene upgrade~~ SOLR-18304: Optimize collapse performance for String fields in Solr 9.x and later Jul 10, 2026

bartoszfidrysiak added 6 commits July 10, 2026 08:57

reviewfix: Removed fetching actual segments from solr query. It was r…

75317fa

…edundant.

reviewfix: Set 'type' equal to 'changed' type in changelog file

f9fd8b0

reviewfix: Changed 'title' field in the changelog file to make it com…

e9865ef

…patible with PR title

reviewfix: Used pattern matching for instanceof in new code inside te…

9dbdf3a

…stAndSetGroupValues method

reviewfix: Removed empty line in CollapsingSearch to satisfy :solr:be…

19bbf73

…nchmark:spotlessJavaCheck

reviewfix: Moved BytesRef.deepCopyOf from LazyStringValue.java to Col…

b4645a4

…lapsingQParserPlugin.java

bartoszfidrysiak requested a review from dsmiley July 10, 2026 14:35

		@@ -0,0 +1,7 @@
		title: Fix collapse on String performance regression due to Lucene upgrade
		type:

		if (values[testClause] instanceof LazyStringValue) {
		LazyStringValue headVal = (LazyStringValue) values[testClause];

Uh oh!

Conversation

bartoszfidrysiak commented Jul 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Solution

Tests

Solr 9.10.1 vs Solr 9.10.1-SNAPSHOT

10.x branch vs 10.x-SNAPSHOT

main branch vs main-SNAPSHOT

Conclusions

Checklist

Uh oh!

dsmiley left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

bartoszfidrysiak commented Jul 7, 2026 •

edited

Loading