Index Sorting: the full picture (SOLR-15390/13681/12239/17310/12230) — for review, happy to split#4648
Open
serhiy-bzhezytskyy wants to merge 9 commits into
Open
Conversation
…tTerminateEarly Solr carried its own @deprecated copy of EarlyTerminatingSortingCollector to implement the segmentTerminateEarly query parameter. Lucene's TopFieldCollector now performs the same per-segment early termination natively when the search sort is a prefix of the index sort, and exposes it via isEarlyTerminated(). buildAndRunCollectorChain now inspects the TopFieldCollector after the search instead of wrapping it, and buildTopDocsCollector lowers the total-hits threshold to the page size on the segmentTerminateEarly path so termination can fire. The segmentTerminatedEarly response header is reported exactly as before (absent when not requested, false when the sort/merge-policy combination is not eligible, true when a segment terminated early). The deprecated collector is removed.
Adds an <indexSort> element to <indexConfig> that sets Lucene's index sort directly, instead of the indirect route through a SortingMergePolicyFactory (a merge policy that does no merging and only carries a Sort). The value is a standard sort spec over docValues fields. When both an <indexSort> and a SortingMergePolicy sort are configured, the <indexSort> takes precedence; a mismatch is logged. The segmentTerminateEarly eligibility check now consults the configured index sort, falling back to the merge policy sort. Existing nested-document handling (the parent field for child-doc schemas) applies to the directly configured sort as well. Enabling index sorting on a collection with existing unsorted segments still requires re-sorting them (see the RESORTINDEX core admin action); it is not applied on reload automatically.
…index Enabling index sorting on a collection created without it currently fails on core reload with an IndexWriter.validateIndexSort error, forcing a delete + reindex from source. This adds a RESORTINDEX core-admin action that re-sorts the existing index into the requested sort order in place, using the LUCENE-9484 mechanism: each segment reader is wrapped in a SortingCodecReader and merged into a fresh sort-configured IndexWriter via addIndexes(CodecReader...), and the result is swapped in with modifyIndexProps (as RestoreCore/replication do), then the writer and searcher are reopened. Notes: - addIndexes does not auto-sort unsorted readers (LUCENE-8505 removed that), so the explicit SortingCodecReader wrap is what performs the merge-based re-sort. - Not supported in SolrCloud mode. - Indexes containing child/nested documents are rejected (re-sorting would break the parent-child blocks), matching UPGRADEINDEX's restriction. - On a failed swap the original index.properties is restored (RestoreCore's rollback). The target sort is given by the sort request parameter using the usual Solr sort syntax; the fields must have docValues.
Exposes Lucene IndexWriterConfig.setLeafSorter via a new <segmentSort> element in <indexConfig>, so a collection can visit its segments (leaf readers) in creation-time order at search time. This is between-segment ordering, separate from index sorting (within-segment document order) and independent of merging; the main use is early termination on time-based indexes by searching the most recently created segments first. Supported values are TIME_ASC and TIME_DESC; the default (NONE) installs no leaf sorter. The value is parsed and validated when the config is loaded. Segment creation time is read from Lucene's per-segment timestamp diagnostic; a leaf that does not resolve to a SegmentReader is ordered last rather than failing the search. Builds on the approach in the earlier PR apache#2477 by Wei Wang.
With index sorting now configurable directly via <indexSort> in <indexConfig> (SOLR-13681), wrapping a merge policy solely to carry a Sort to the index writer is no longer needed. Deprecate both SortingMergePolicy and its SortingMergePolicyFactory, pointing users to <indexSort>. The classes still function (and are still used internally by index re-sorting, so remaining references are marked @SuppressWarnings("deprecation")); this only signals the configuration approach that should replace them.
- RESORTINDEX now falls back to the configured <indexSort> (SOLR-13681), not only a SortingMergePolicy, so the migration workflow (configure <indexSort>, then RESORTINDEX with no sort param) works end to end. - RESORTINDEX sets the core codec and schema similarity on the re-sort writer, so addIndexes no longer rewrites a per-field index with Lucene defaults; and it cleans up the old index dir on success / the orphaned resort dir on rollback. - <indexSort> fails fast with a clear error naming the element when the configured spec is not a usable index sort. - Tests: invalid <indexSort>; <indexSort> wins over a SortingMergePolicy sort; RESORTINDEX using a configured <indexSort>; a deterministic segmentTerminateEarly run against <indexSort>; SegmentTimeLeafSorter tolerates a non-SegmentReader leaf (sorts last). - Docs: RESORTINDEX points at <indexSort> instead of the deprecated SortingMergePolicyFactory; a Major Changes in Solr 11 upgrade note for the deprecation; and a note distinguishing <indexSort> from <segmentSort>.
…lues field
<segmentSort> now accepts either a time preset (TIME_ASC/TIME_DESC, by segment
creation time) or a field spec ("<field> asc|desc") that orders segments by
the per-segment min (asc) or max (desc) of a single-valued numeric docValues
field. Lucene's setLeafSorter takes an arbitrary Comparator<LeafReader>, so
this is not a Lucene limitation -- it broadens what the config exposes.
SegmentSort becomes a small parsed spec (NONE/TIME/FIELD); SegmentTimeLeafSorter
is renamed to SegmentLeafSorter and builds the comparator for both forms, with
the per-segment key memoized on the segment core cache key (the comparator runs
O(n log n) times per reader open) and unknown keys sorting last. Field sorts are
validated at config load (must be an existing single-valued numeric docValues
field). Comparing the raw docValues long is order-correct for all numeric types
because Solr stores float/double docValues in sortable-bits form.
Tests: field-sort comparator build + spec round-trip; rejection of unknown,
non-numeric, non-docValues, and multi-valued fields; garbage spec parsing.
Docs updated for both forms.
… multi-value legacy segments Single-valued FLOAT/DOUBLE docValues are stored by Solr as raw Float.floatToIntBits/Double.doubleToLongBits (NOT sortable bits), so the previous raw-long min/max ordered negative values incorrectly. Normalize each value to an order-preserving long via toSortableLong (int/long/date pass through; float/double are decoded then re-encoded to sortable bits). Also, for a legacy segment where the field was multi-valued, take the last (largest) value for a desc/max scan rather than only the first. Adds a regression test (two all-negative segments where raw floatToIntBits order diverges from numeric order) proven to fail without the fix.
- RESORTINDEX request-body schema now documents the <indexSort>-first fallback (matching resolveTargetSort and the ref-guide), and the two API model classes get class javadoc. - Fix a half-updated TODO wording in SolrIndexSearcher. - Tests: <segmentSort> naming a nonexistent field fails when the writer config is built; int field ordering both directions against a live core searcher; <indexSort> and <segmentSort> configured together.
github-actions
Bot
added
documentation
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
1 participant
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.
Follows up on the dev@ thread "State of Lucene Index Sorting support in Solr".
Purpose: this PR exists to make the whole shape of the index-sorting work reviewable in one place — as I offered on the list ("open a PR with all changes to see the full picture"). It is not a request to merge all of this as one lump. I'm very happy to split it into per-ticket PRs, and to fold the config piece into Christine's existing work rather than land my own — see the note at the bottom.
The picture (from David's "State of Lucene Index Sorting" report)
Index sorting works today only through the indirect
SortingMergePolicyFactory(a merge policy that does no merging, existing only to carry aSorttoIndexWriterConfig.setIndexSort()). This branch works through the open pieces end to end, in dependency order:TopFieldCollectorearly termination forsegmentTerminateEarly; remove the deprecatedEarlyTerminatingSortingCollector(Tomas said "feel free to take it")<indexSort>config in<indexConfig>, replacing the merge-policy indirectionRESORTINDEXcore-admin action to re-sort an existing index (also up standalone as #4644)<segmentSort>to configure Lucene's leaf sorterSortingMergePolicynow that<indexSort>replaces itCredit (important — please read)
main(the draft predatesMapWriterand some config changes). Christine owns this lane; I'd rather help land SOLR-13681: make Lucene's index sorting directly configurable in Solr #313 than land a competing change — this is here for the full picture, not to bypass her work.(I stripped
Co-authored-bytrailers from the commits deliberately — I didn't want to attribute code to Christine or Wei publicly without asking them first. Crediting them here in prose instead; happy to add trailers back if they'd prefer.)Open design questions (for the list / reviewers)
<indexSort>(within-segment) and<segmentSort>(between-segment) as separate<indexConfig>elements, vs. a unified<indexSorters>element. My reading of precedent leans separate (Lucene keepssetIndexSort/setLeafSorterdistinct; ES/OpenSearch expose only the within-segment axis) — but no strong opinion.Testing
Unit + SolrCloud (~40 tests) pass locally, incl.
TestSegmentSorting(testSegmentTerminateEarly,testSegmentTerminateEarlyWithIndexSort,testAtomicUpdateOfSegmentSortField) andTestEarlyTerminatingQueries. Compiles clean.What I'm asking
Direction and collaboration — especially with @cpoerschke on 13681, since that's the keystone and her active work. If the shape looks right, I'll break this into reviewable per-ticket PRs (15390 first — it's self-contained). No rush.