SOLR-18304: Optimize collapse performance for String fields in Solr 9.x and later#4621
SOLR-18304: Optimize collapse performance for String fields in Solr 9.x and later#4621bartoszfidrysiak wants to merge 14 commits into
Conversation
…rdinal fast path comparison for string sorted doc values from the same segment
There was a problem hiding this comment.
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 onlylookupOrd()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.
…patible with PR title
…stAndSetGroupValues method
…nchmark:spotlessJavaCheck
…lapsingQParserPlugin.java
dsmiley
left a comment
There was a problem hiding this comment.
Going to merge this tonight or maybe tomorrow.
Again -- great work here. The benchmark was above & beyond 🔥
|
@dsmiley |
|
I will after the LazyStringValue is moved |
|
@dsmiley
I'd like to confirm the intended approach before making the change. |
dsmiley
left a comment
There was a problem hiding this comment.
Woops... I had pending review feedback but didn't submit! Submitting now.
| import org.apache.lucene.index.SortedDocValues; | ||
| import org.apache.lucene.util.BytesRef; | ||
|
|
||
| final class LazyStringValue { |
There was a problem hiding this comment.
oh one thing... so this is only used by CollapsingQParserPlugin. Shouldn't it be an inner class?
Yes that thing is huge and has many classes ... but still. It's better in there than in the search package which is a big package of lots of stuff.
Related links:
Description
Migrating to
Lucene90DocValuesProducerin 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’sTermOrdValLeafComparator.copy(), which triggersLZ4.decompress()for every document processed by the collapse query. This decompression overhead did not exist in Solr 8.Solution
This PR proposes two improvements:
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, theCollapsingSearchbenchmark 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
collapseByDateAndStrbenchmark 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.collapseByStrbenchmark 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 madecollapseByStrmuch 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:
mainbranch../gradlew check.