Add support for radial search in exact search #2174

VijayanB · 2024-10-01T20:40:50Z

Description

When threshold value is set, knn plugin will not be creating graph. Hence, when search request is trigged during that time, exact search will return valid results. However, radial search was never included as part of exact search. This will break radial search when threshold is added and radial search is requested. In this commit, new method
is introduced to accept min score and return documents that are greater than min score, similar to how radial search is performed by native engines. This search is independent of engine, but, radial search is supported only for FAISS engine out of all native engines.

Related Issues

Part of #1942

Check List

New functionality includes testing.
New functionality has been documented.
API changes companion pull request created.
Commits are signed per the DCO using --signoff.
Public documentation issue/PR created.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

VijayanB · 2024-10-01T20:41:01Z

This depends on #2136

VijayanB · 2024-10-01T20:43:03Z

I am skipping changelog. While merging back to main will update changelog.

src/main/java/org/opensearch/knn/index/query/ExactSearcher.java

junqiu-lei · 2024-10-01T22:48:47Z

src/main/java/org/opensearch/knn/index/query/KNNWeight.java

+     */
+    private boolean isExactSearchRequire(final LeafReaderContext context, final int filterIdsCount, final int annResultCount) {
+        if (annResultCount == 0 && isMissingNativeEngineFiles(context)) {
+            log.debug("Perform exact search after approximate search since no native engine files are available");


Is this case unexpected so we use debug level log?

Since this is in path of query, recommendation is to avoid info.

src/main/java/org/opensearch/knn/index/query/KNNWeight.java

src/main/java/org/opensearch/knn/index/query/ExactSearcher.java

shatejas

There doesn't seem to be updates in unit tests, will it be present in another iteration?

src/main/java/org/opensearch/knn/index/query/ExactSearcher.java

When threshold value is set, knn plugin will not be creating graph. Hence, when search request is trigged during that time, exact search will return valid results. However, radial search was never included as part of exact search. This will break radial search when threshold is added and radial search is requested. In this commit, new method is introduced to accept min score and return documents that are greater than min score, similar to how radial search is performed by native engines. This search is independent of engine, but, radial search is supported only for FAISS engine out of all native engines. Signed-off-by: Vijayan Balasubramanian <[email protected]>

VijayanB · 2024-10-03T06:12:47Z

There doesn't seem to be updates in unit tests, will it be present in another iteration?

Added unit test

navneet1v · 2024-10-03T06:28:29Z

src/test/java/org/opensearch/knn/index/query/KNNWeightTests.java

+    @SneakyThrows
+    public void testRadialSearch_whenNoEngineFiles_thenPerformExactSearch() {


Thought I see we are using this test to check ExactSearch for radial search and avoided the Unit test for ExactSearch with radial search threshold. See if you want to mock ExactSearcher search class and all its invocation to make it proper Unit test. Since exact search is not getting added more and more extra logic do you see we should have separate unit test class for ExactSearcher class.

Good point. I can create Github issue for refactor. I see two option, 1) Keep this PR as it is and refactor as part of GH issue since there are more than 3 test cases that uses exact search, or 2) I can only do it for radial search, as part of GH issue others can be moved later

I like the option 2. Lets do atleast for this test case and create a GH issue too for the test refactor.

updated this PR. Will create GH issue

Signed-off-by: Vijayan Balasubramanian <[email protected]>

src/test/java/org/opensearch/knn/index/query/ExactSearcherTests.java

vibrantvarun · 2024-10-03T21:33:55Z

src/main/java/org/opensearch/knn/index/query/ExactSearcher.java

@@ -71,15 +106,17 @@ private Map<Integer, Float> scoreAllDocs(KNNIterator iterator) throws IOExceptio
        return docToScore;
    }

-    private Map<Integer, Float> searchTopK(KNNIterator iterator, int k) throws IOException {
+    private Map<Integer, Float> searchTopCandidates(KNNIterator iterator, int limit, @NonNull Predicate<Float> filterScore)


Curious why k variable is renamed to limit?

This method will be called by radialSearch as well. Hence, renamed it from K to be more generic.

* Add support for radial search in exact search When threshold value is set, knn plugin will not be creating graph. Hence, when search request is trigged during that time, exact search will return valid results. However, radial search was never included as part of exact search. This will break radial search when threshold is added and radial search is requested. In this commit, new method is introduced to accept min score and return documents that are greater than min score, similar to how radial search is performed by native engines. This search is independent of engine, but, radial search is supported only for FAISS engine out of all native engines. Signed-off-by: Vijayan Balasubramanian <[email protected]> --------- Signed-off-by: Vijayan Balasubramanian <[email protected]>

…t search when there are no engine files (#2188) * Introduce new setting to configure when to build graph during segment creation (#2007) Added new updatable index setting "build_vector_data_structure_threshold", which will be considered when to build braph or not for native engines. This is noop for lucene. This depends on use lucene format as prerequisite. We don't need to add flag since it is only enable if lucene format is already enabled. Signed-off-by: Vijayan Balasubramanian <[email protected]> * Add integration test for binary vector values (#2142) Signed-off-by: Vijayan Balasubramanian <[email protected]> * Allow build graph greedily for quantization scenarios (#2175) Previosuly we only added support to build greedily for non quantization scenario. In this commit, we can remove that constraint, however, we cannot skip writing quanitization state since it is required irrespective of type of search is executed later. Signed-off-by: Vijayan Balasubramanian <[email protected]> * Add exact search if no native engine files are available (#2136) * Add exact search if no engine files are in segments When graph is not available, plugin will return empty results. With this change, exact search will be performed when only no engine file is available in segment. We also don't need version check or feature flag because, option to not build vector data structure will only be available post 2.17. If an index is created using pre 2.17 version, segment will always have engine files and this feature will never be called during search. --------- Signed-off-by: Vijayan Balasubramanian <[email protected]> * Add support for radial search in exact search (#2174) * Add support for radial search in exact search When threshold value is set, knn plugin will not be creating graph. Hence, when search request is trigged during that time, exact search will return valid results. However, radial search was never included as part of exact search. This will break radial search when threshold is added and radial search is requested. In this commit, new method is introduced to accept min score and return documents that are greater than min score, similar to how radial search is performed by native engines. This search is independent of engine, but, radial search is supported only for FAISS engine out of all native engines. Signed-off-by: Vijayan Balasubramanian <[email protected]> --------- Signed-off-by: Vijayan Balasubramanian <[email protected]>

…t search when there are no engine files (#2188) * Introduce new setting to configure when to build graph during segment creation (#2007) Added new updatable index setting "build_vector_data_structure_threshold", which will be considered when to build braph or not for native engines. This is noop for lucene. This depends on use lucene format as prerequisite. We don't need to add flag since it is only enable if lucene format is already enabled. Signed-off-by: Vijayan Balasubramanian <[email protected]> * Add integration test for binary vector values (#2142) Signed-off-by: Vijayan Balasubramanian <[email protected]> * Allow build graph greedily for quantization scenarios (#2175) Previosuly we only added support to build greedily for non quantization scenario. In this commit, we can remove that constraint, however, we cannot skip writing quanitization state since it is required irrespective of type of search is executed later. Signed-off-by: Vijayan Balasubramanian <[email protected]> * Add exact search if no native engine files are available (#2136) * Add exact search if no engine files are in segments When graph is not available, plugin will return empty results. With this change, exact search will be performed when only no engine file is available in segment. We also don't need version check or feature flag because, option to not build vector data structure will only be available post 2.17. If an index is created using pre 2.17 version, segment will always have engine files and this feature will never be called during search. --------- Signed-off-by: Vijayan Balasubramanian <[email protected]> * Add support for radial search in exact search (#2174) * Add support for radial search in exact search When threshold value is set, knn plugin will not be creating graph. Hence, when search request is trigged during that time, exact search will return valid results. However, radial search was never included as part of exact search. This will break radial search when threshold is added and radial search is requested. In this commit, new method is introduced to accept min score and return documents that are greater than min score, similar to how radial search is performed by native engines. This search is independent of engine, but, radial search is supported only for FAISS engine out of all native engines. Signed-off-by: Vijayan Balasubramanian <[email protected]> --------- Signed-off-by: Vijayan Balasubramanian <[email protected]> (cherry picked from commit 5a56829)

…nd perform exact search when there are no engine files (#2201) * Add support to build vector data structures greedily and perform exact search when there are no engine files (#2188) * Introduce new setting to configure when to build graph during segment creation (#2007) Added new updatable index setting "build_vector_data_structure_threshold", which will be considered when to build braph or not for native engines. This is noop for lucene. This depends on use lucene format as prerequisite. We don't need to add flag since it is only enable if lucene format is already enabled. Signed-off-by: Vijayan Balasubramanian <[email protected]> * Add integration test for binary vector values (#2142) Signed-off-by: Vijayan Balasubramanian <[email protected]> * Allow build graph greedily for quantization scenarios (#2175) Previosuly we only added support to build greedily for non quantization scenario. In this commit, we can remove that constraint, however, we cannot skip writing quanitization state since it is required irrespective of type of search is executed later. Signed-off-by: Vijayan Balasubramanian <[email protected]> * Add exact search if no native engine files are available (#2136) * Add exact search if no engine files are in segments When graph is not available, plugin will return empty results. With this change, exact search will be performed when only no engine file is available in segment. We also don't need version check or feature flag because, option to not build vector data structure will only be available post 2.17. If an index is created using pre 2.17 version, segment will always have engine files and this feature will never be called during search. --------- Signed-off-by: Vijayan Balasubramanian <[email protected]> * Add support for radial search in exact search (#2174) * Add support for radial search in exact search When threshold value is set, knn plugin will not be creating graph. Hence, when search request is trigged during that time, exact search will return valid results. However, radial search was never included as part of exact search. This will break radial search when threshold is added and radial search is requested. In this commit, new method is introduced to accept min score and return documents that are greater than min score, similar to how radial search is performed by native engines. This search is independent of engine, but, radial search is supported only for FAISS engine out of all native engines. Signed-off-by: Vijayan Balasubramanian <[email protected]> --------- Signed-off-by: Vijayan Balasubramanian <[email protected]> (cherry picked from commit 5a56829) * Fix compilation issue due to package error Signed-off-by: Vijayan Balasubramanian <[email protected]> --------- Signed-off-by: Vijayan Balasubramanian <[email protected]> Co-authored-by: Vijayan Balasubramanian <[email protected]>

VijayanB requested review from heemin32, navneet1v, vamshin, jmazanec15, naveentatikonda, junqiu-lei, martin-gaievski, ryanbogan and luyuncheng as code owners October 1, 2024 20:40

VijayanB force-pushed the support-exact-search-for-radius branch from cbd28c4 to a2936f3 Compare October 1, 2024 20:41

VijayanB added the skip-changelog label Oct 1, 2024

VijayanB marked this pull request as draft October 1, 2024 21:13

VijayanB force-pushed the support-exact-search-for-radius branch 3 times, most recently from e0ab310 to 0b33a1b Compare October 1, 2024 21:54

VijayanB marked this pull request as ready for review October 1, 2024 21:56

junqiu-lei reviewed Oct 1, 2024

View reviewed changes

VijayanB requested a review from junqiu-lei October 1, 2024 23:10

VijayanB force-pushed the support-exact-search-for-radius branch 3 times, most recently from dc81e8f to 230f286 Compare October 2, 2024 01:29

shatejas reviewed Oct 2, 2024

View reviewed changes

src/main/java/org/opensearch/knn/index/query/ExactSearcher.java Outdated Show resolved Hide resolved

src/main/java/org/opensearch/knn/index/query/ExactSearcher.java Outdated Show resolved Hide resolved

src/main/java/org/opensearch/knn/index/query/ExactSearcher.java Show resolved Hide resolved

VijayanB requested a review from shatejas October 2, 2024 23:35

VijayanB force-pushed the support-exact-search-for-radius branch 4 times, most recently from 6c47860 to abc6e69 Compare October 3, 2024 05:55

navneet1v reviewed Oct 3, 2024

View reviewed changes

Fix code review comments

54cdfe4

Signed-off-by: Vijayan Balasubramanian <[email protected]>

VijayanB force-pushed the support-exact-search-for-radius branch 2 times, most recently from 7466091 to efa7cb3 Compare October 3, 2024 19:07

Refactor test

e51ae11

Signed-off-by: Vijayan Balasubramanian <[email protected]>

VijayanB force-pushed the support-exact-search-for-radius branch from efa7cb3 to e51ae11 Compare October 3, 2024 19:11

shatejas reviewed Oct 3, 2024

View reviewed changes

src/test/java/org/opensearch/knn/index/query/ExactSearcherTests.java Show resolved Hide resolved

shatejas approved these changes Oct 3, 2024

View reviewed changes

VijayanB requested a review from navneet1v October 3, 2024 20:07

junqiu-lei approved these changes Oct 3, 2024

View reviewed changes

vibrantvarun reviewed Oct 3, 2024

View reviewed changes

vibrantvarun approved these changes Oct 4, 2024

View reviewed changes

jmazanec15 approved these changes Oct 4, 2024

View reviewed changes

VijayanB merged commit 96663fd into opensearch-project:feature/build-vector-ds-greedily Oct 4, 2024
30 checks passed

VijayanB deleted the support-exact-search-for-radius branch October 4, 2024 19:14

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for radial search in exact search #2174

Add support for radial search in exact search #2174

VijayanB commented Oct 1, 2024 •

edited

Loading

VijayanB commented Oct 1, 2024

VijayanB commented Oct 1, 2024

junqiu-lei Oct 1, 2024

VijayanB Oct 1, 2024

shatejas left a comment

VijayanB commented Oct 3, 2024

navneet1v Oct 3, 2024

VijayanB Oct 3, 2024 •

edited

Loading

navneet1v Oct 3, 2024

VijayanB Oct 3, 2024

VijayanB Oct 3, 2024

vibrantvarun Oct 3, 2024

VijayanB Oct 3, 2024 •

edited

Loading

		@SneakyThrows
		public void testRadialSearch_whenNoEngineFiles_thenPerformExactSearch() {

Add support for radial search in exact search #2174

Add support for radial search in exact search #2174

Conversation

VijayanB commented Oct 1, 2024 • edited Loading

Description

Related Issues

Check List

VijayanB commented Oct 1, 2024

VijayanB commented Oct 1, 2024

junqiu-lei Oct 1, 2024

Choose a reason for hiding this comment

VijayanB Oct 1, 2024

Choose a reason for hiding this comment

shatejas left a comment

Choose a reason for hiding this comment

VijayanB commented Oct 3, 2024

navneet1v Oct 3, 2024

Choose a reason for hiding this comment

VijayanB Oct 3, 2024 • edited Loading

Choose a reason for hiding this comment

navneet1v Oct 3, 2024

Choose a reason for hiding this comment

VijayanB Oct 3, 2024

Choose a reason for hiding this comment

VijayanB Oct 3, 2024

Choose a reason for hiding this comment

vibrantvarun Oct 3, 2024

Choose a reason for hiding this comment

VijayanB Oct 3, 2024 • edited Loading

Choose a reason for hiding this comment

VijayanB commented Oct 1, 2024 •

edited

Loading

VijayanB Oct 3, 2024 •

edited

Loading

VijayanB Oct 3, 2024 •

edited

Loading