-
Notifications
You must be signed in to change notification settings - Fork 143
[Java][Fix] Multithreaded querying fails without synchronization #1082
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
rapids-bot
merged 13 commits into
rapidsai:branch-25.10
from
SearchScale:investigate-multithreaded-failures
Aug 1, 2025
Merged
Changes from 11 commits
Commits
Show all changes
13 commits
Select commit
Hold shift + click to select a range
53fa74a
Adding a test where running queries from different threads causes mem…
chatman 2f9dd8f
Disabling the test with closed resources
chatman 6c18331
Removing unnecessary resources from CagraSearchParams
chatman e80bf1d
Fix CAGRA search() by adding a synchronized block around the underlyi…
chatman 2955420
Revert "Fix CAGRA search() by adding a synchronized block around the …
chatman c1fdf83
Revert "Removing unnecessary resources from CagraSearchParams"
chatman be3f635
Revert "Disabling the test with closed resources"
chatman eaaedf2
Revert "Adding a test where running queries from different threads ca…
chatman 267626b
Merge remote-tracking branch 'origin/branch-25.08' into investigate-m…
chatman 1d7f161
Fix CuVSResources usage for multithreaded queries
chatman 6bd62ec
Fix log messages in the test
chatman 6e14a48
Merge branch 'branch-25.08' into investigate-multithreaded-failures
chatman b0bf5bd
Merge branch 'branch-25.10' into investigate-multithreaded-failures
chatman File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -16,12 +16,17 @@ | |
| package com.nvidia.cuvs; | ||
|
|
||
| import java.util.Arrays; | ||
| import java.util.Objects; | ||
| import java.util.function.LongToIntFunction; | ||
|
|
||
| /** | ||
| * HnswQuery holds the query vectors to be used while invoking search on the | ||
| * HNSW index. | ||
| * | ||
| * <p><strong>Thread Safety:</strong> Each HnswQuery instance should use its own | ||
| * CuVSResources object that is not shared with other threads. Sharing CuVSResources | ||
| * between threads can lead to memory allocation errors or JVM crashes. | ||
| * | ||
| * @since 25.02 | ||
| */ | ||
| public class HnswQuery { | ||
|
|
@@ -30,6 +35,7 @@ public class HnswQuery { | |
| private final LongToIntFunction mapping; | ||
| private final float[][] queryVectors; | ||
| private final int topK; | ||
| private final CuVSResources resources; | ||
|
|
||
| /** | ||
| * Constructs an instance of {@link HnswQuery} using queryVectors, mapping, and | ||
|
|
@@ -39,16 +45,19 @@ public class HnswQuery { | |
| * @param queryVectors 2D float query vector array | ||
| * @param mapping a function mapping ordinals (neighbor IDs) to custom user IDs | ||
| * @param topK the top k results to return | ||
| * @param resources CuVSResources instance to use for this query | ||
| */ | ||
| private HnswQuery( | ||
| HnswSearchParams hnswSearchParams, | ||
| float[][] queryVectors, | ||
| LongToIntFunction mapping, | ||
| int topK) { | ||
| int topK, | ||
| CuVSResources resources) { | ||
| this.hnswSearchParams = hnswSearchParams; | ||
| this.queryVectors = queryVectors; | ||
| this.mapping = mapping; | ||
| this.topK = topK; | ||
| this.resources = resources; | ||
| } | ||
|
|
||
| /** | ||
|
|
@@ -85,6 +94,15 @@ public int getTopK() { | |
| return topK; | ||
| } | ||
|
|
||
| /** | ||
| * Gets the CuVSResources instance for this query. | ||
| * | ||
| * @return the CuVSResources instance | ||
| */ | ||
| public CuVSResources getResources() { | ||
| return resources; | ||
| } | ||
|
|
||
| @Override | ||
| public String toString() { | ||
| return "HnswQuery [mapping=" | ||
|
|
@@ -97,14 +115,28 @@ public String toString() { | |
| } | ||
|
|
||
| /** | ||
| * Builder helps configure and create an instance of BruteForceQuery. | ||
| * Builder helps configure and create an instance of HnswQuery. | ||
| */ | ||
| public static class Builder { | ||
|
|
||
| private HnswSearchParams hnswSearchParams; | ||
| private float[][] queryVectors; | ||
| private LongToIntFunction mapping = SearchResults.IDENTITY_MAPPING; | ||
| private int topK = 2; | ||
| private final CuVSResources resources; | ||
|
|
||
| /** | ||
| * Constructor that requires CuVSResources. | ||
| * | ||
| * <p><strong>Important:</strong> The provided CuVSResources instance should not be | ||
| * shared with other threads. Each thread performing searches should create its own | ||
| * CuVSResources instance to avoid memory allocation conflicts and potential JVM crashes. | ||
| * | ||
| * @param resources the CuVSResources instance to use for this query (must not be shared between threads) | ||
| */ | ||
| public Builder(CuVSResources resources) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I was about to ask why this seems to be breaking the fluent-interface semantics of the Builder. It looks like this is to necessitate specifying the |
||
| this.resources = Objects.requireNonNull(resources, "resources cannot be null"); | ||
| } | ||
|
|
||
| /** | ||
| * Sets the instance of configured HnswSearchParams to be passed for search. | ||
|
|
@@ -157,7 +189,7 @@ public Builder withTopK(int topK) { | |
| * @return an instance of {@link HnswQuery} | ||
| */ | ||
| public HnswQuery build() { | ||
| return new HnswQuery(hnswSearchParams, queryVectors, mapping, topK); | ||
| return new HnswQuery(hnswSearchParams, queryVectors, mapping, topK, resources); | ||
| } | ||
| } | ||
| } | ||
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
D'oh! 👍