Fix thread safety issue with ordinal terms collector - #1006
Merged
Conversation
sarthakn7
approved these changes
Jul 22, 2026
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.
Problem
GlobalOrdinalLookupinstances are cached perIndexReaderand shared across all concurrent search requests. The cached object held a singleSortedDocValues/SortedSetDocValuesinstance, which is not thread-safe — Lucene's term dictionary uses mutable LZ4 decompression buffers internally (blockBuffer,blockInput,currentCompressedBlockStart/End).When two requests both reach the
reduce()phase of anOrdinalTermsCollectorManagerat the same time, theirlookupGlobalOrdinalcalls operate on the sameTermsDictdecompression state simultaneously. One thread'sdecompressBlock()corrupts the buffers the other is reading, producing:The
OrdinalMap(used for segment-to-global ordinal mapping during collection) is thread-safe — it is backed by read-only packed integer arrays. Only the term lookup path is affected.Fix
Separate the thread-safe part (
OrdinalMap) from the non-thread-safe part (SortedDocValues/SortedSetDocValues):GlobalOrdinalLookup(cached, shared) now holds only theOrdinalMapand value count.getSegmentMapping()is unchanged.createTermLookup(IndexReader)method returns a lightweight per-requestTermLookupthat holds its own private array of segment-level doc values. It uses the cachedOrdinalMapto routelookupGlobalOrdinal(ord)to the correct segment and local ordinal.OrdinalTermsCollectorManager.reduce()now callscreateTermLookupto get a fresh, request-privateTermLookupbefore resolving ordinals to strings.Getting per-segment
SortedDocValuesviaDocValues.getSorted(leafReader, field)is cheap (a pointer into already-loaded segment data). The expensiveOrdinalMap.build()call remains cached as before.Testing
GlobalOrdinalLookupTestandOrdinalTermsCollectorManagerTesttests pass unchanged.testConcurrentTermLookup: 8 threads × 500 iterations each callingcreateTermLookupand resolving ordinals concurrently from the same cachedGlobalOrdinalLookup, asserting no errors or incorrect results.