-
Notifications
You must be signed in to change notification settings - Fork 25.6k
Wrap ES KNN queries with PatienceKNN query #127223
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
Conversation
|
🔍 Preview links for changed docs: 🔔 The preview site may take up to 3 minutes to finish building. These links will become live once it completes. |
|
Pinging @elastic/es-search-relevance (Team:Search Relevance) |
|
Hi @tteofili, I've created a changelog YAML for you. |
benwtrent
left a comment
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.
Whew, way cleaner! Still some room for simplifying. But the index setting looks much nicer to me :)
server/src/main/java/org/elasticsearch/index/mapper/vectors/DenseVectorFieldMapper.java
Outdated
Show resolved
Hide resolved
server/src/main/java/org/elasticsearch/index/mapper/vectors/DenseVectorFieldMapper.java
Outdated
Show resolved
Hide resolved
…nseVectorFieldMapper.java Co-authored-by: Benjamin Trent <[email protected]>
benwtrent
left a comment
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.
So clean! I like it, and the performance numbers are ⚡
|
I don't think so 😅
…On Mon, Jun 30, 2025, 1:04 PM Benjamin Trent ***@***.***> wrote:
***@***.**** commented on this pull request.
Whew, way cleaner! Still some room for simplifying. But the index setting
looks much nicer to me :)
------------------------------
In docs/reference/elasticsearch/index-settings/index-modules.md
<#127223 (comment)>
:
> @@ -259,3 +259,6 @@ $$$index-esql-stored-fields-sequential-proportion$$$
`index.esql.stored_fields_sequential_proportion`
: Tuning parameter for deciding when {{esql}} will load [Stored fields](/reference/elasticsearch/rest-apis/retrieve-selected-fields.md#stored-fields) using a strategy tuned for loading dense sequence of documents. Allows values between 0.0 and 1.0 and defaults to 0.2. Indices with documents smaller than 10kb may see speed improvements loading `text` fields by setting this lower.
+
+$$$index-dense-vector-hnsw-filter-heuristic$$$ `index.dense_vector.hnsw_filter_heuristic`
copy-paste error.
------------------------------
In
server/src/main/java/org/elasticsearch/index/mapper/vectors/DenseVectorFieldMapper.java
<#127223 (comment)>
:
> + Query returnedQuery = knnQuery;
+ if (indexOptions instanceof HnswIndexOptions hnswIndexOptions) {
+ returnedQuery = PatienceKnnVectorQuery.fromFloatQuery(knnQuery, 0.995, patience);
+ } else if (indexOptions instanceof Int8HnswIndexOptions hnswIndexOptions) {
+ returnedQuery = PatienceKnnVectorQuery.fromFloatQuery(knnQuery, 0.995, patience);
+ } else if (indexOptions instanceof Int4HnswIndexOptions hnswIndexOptions) {
+ returnedQuery = PatienceKnnVectorQuery.fromFloatQuery(knnQuery, 0.995, patience);
+ } else if (indexOptions instanceof BBQHnswIndexOptions hnswIndexOptions) {
+ returnedQuery = PatienceKnnVectorQuery.fromFloatQuery(knnQuery, 0.995, patience);
+ }
+ return returnedQuery;
Why do we have this big predicate branch?
Also, it seems that this 0.995 is just the default in Lucene.
Why can't we simplify all this and return
PatienceKnnVectorQuery.fromFloatQuery(knnQuery) ?
Similar question to the byte query.
------------------------------
In
server/src/main/java/org/elasticsearch/index/mapper/vectors/DenseVectorFieldMapper.java
<#127223 (comment)>
:
> @@ -171,6 +175,14 @@ public KnnSearchStrategy getKnnSearchStrategy() {
Setting.Property.Dynamic
);
+ public static final Setting<Boolean> HNSW_EARLY_TERMINATION = Setting.boolSetting(
+ "index.dense_vector.hnsw_early_termination",
⬇️ Suggested change
- "index.dense_vector.hnsw_early_termination",
+ "index.dense_vector.hnsw_enable_early_termination",
Maybe indicate that its enable ? I know its more verbose, but seems
clearer to me.
Unless you think we will adjust this in the future to accept the early
termination params itself (which seems like too many knobs to me).
—
Reply to this email directly, view it on GitHub
<#127223 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/BUCFISV6R74RUX2AMKRRWHD3GEYXPAVCNFSM6AAAAAB3V23AB6VHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDSNZRGEYDGNJQGA>
.
You are receiving this because you are subscribed to this thread.Message
ID: ***@***.***>
|
benwtrent
left a comment
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.
❤️
💔 Backport failedThe backport operation could not be completed due to the following error: You can use sqren/backport to manually backport by running |







This exposes an option to add the patience-based early termination defined in apache/lucene#14094 to KNN queries in ES.
This leverages
PatienceKnnVectorQuerywhich wraps ES KNN queries when mapping is one ofhnsw, int8_hnsw, int4_hnsw, bbq_hnswandearly_terminationis set totrue(withinindex_options).e.g.:
Currently this has the limitation that we can't profile that query because we can't extend the
PatienceKnnQuerysame as https://github.com/elastic/elasticsearch/blob/main/server/src/main/java/org/elasticsearch/search/vectors/ESKnnByteVectorQuery.java#L28-L38 because of visibility issues. However it's fixed in 10.3 branch via apache/lucene#14838 and we'll incorporate that as soon as 10.3 is released and included in ESmainbranch.