Repository navigation
Fix HNSW graph merge concurrency in case of intra-merge executor (#16729) - #16765
Open
CH-Abhinav wants to merge 1 commit into
Open
CH-Abhinav wants to merge 1 commit into
CH-Abhinav wants to merge 1 commit into
Conversation
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.
Description
Fixes #16729.
Problem
In$N$ workers were previously constructed as greedy tasks where each worker executed an internal loop draining batches from a shared atomic counter ($N$ tasks were submitted in a single call to
HnswConcurrentMergeBuilder,workProgress) until all vectors were added. TheseTaskExecutor#invokeAll.When
ConcurrentMergeScheduler.CachedExecutoris briefly saturated at the time of submission (i.e.maxThreadCount - mergeThreads.size() - 1 <= 0due to concurrent merges), it falls back to executing the task inline on the calling thread. Because worker 0 was an unbounded loop draining all available batches, it consumed 100% of the merge's work inline before returning control to the submit loop. Even if sibling merges completed milliseconds later and freed up intra-merge thread capacity, helper threads were never enlisted, locking the entire merge to 1.00x concurrency (running single-threaded for its entire duration).Solution
[0, maxOrd)is partitioned into discrete batch tasks ofbatchSize(default 2048) and submitted toTaskExecutor#invokeAll.BlockingQueue<ConcurrentMergeWorker> workerPool = new ArrayBlockingQueue<>(workers.length)holds the pre-initialized workers (each with its ownRandomVectorScorerSupplierand scratch buffers).workerPool, executes a single slice[start, end), and releases the worker back in afinallyblock. If the calling thread must run a task inline, it only processes one batch (~50ms) rather than draining the whole merge, allowing subsequent batches to be scheduled onto helper threads as soon as sibling merges finish.workProgress,run(int maxOrd), andgetStartPos(int maxOrd)fromConcurrentMergeWorker.Tests
TestHnswConcurrentMergeSubmission:ConcurrentMergeScheduler,TieredMergePolicy, and vector mergesMIN_BIG_MERGE_MB).batchesByThread.size() > 1).@LuceneTestCase.Monsterand@Repeat(iterations = 3)given segment size and indexing requirements../gradlew check -x testpasses all precommit checks (licenses, ecj linter for main and test, forbidden APIs, and tidy formatting).TestConcurrentMergeScheduler,TestHnswFloatVectorGraph) pass without regressions.