K-NN Plugin Upgrade with Lucene 10.0.1 #2429

Vikasht34 · 2025-01-23T20:02:07Z

Description

As per Lucene 10.0.1 upgrade, There are lots of breaking changes. This PR consumes Lucene 10.0.1 as part of Opensearch 3.0.0 snapshot and Fixes all breaking changes.

Note :- This PR Does not aim to refactor any part of code, It's just making compatible with Lucene 10.0.1

Resolves this issues

#2434
#2433

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.

martin-gaievski · 2025-01-24T21:41:31Z

it looks like opensearch-knn-test-fixtures needs to consume latest corre main with lucene 10

/home/runner/work/k-NN/k-NN/src/main/java/org/opensearch/knn/index/codec/KNNCodecVersion.java:16: error: package org.apache.lucene.backward_codecs.lucene912 does not exist
import org.apache.lucene.backward_codecs.lucene912.Lucene912Codec;

Vikasht34 · 2025-01-24T21:45:56Z

it looks like opensearch-knn-test-fixtures needs to consume latest corre main with lucene 10

/home/runner/work/k-NN/k-NN/src/main/java/org/opensearch/knn/index/codec/KNNCodecVersion.java:16: error: package org.apache.lucene.backward_codecs.lucene912 does not exist
import org.apache.lucene.backward_codecs.lucene912.Lucene912Codec;

Yes ...Agree ...We need to change to alpha !! lots of things started breaking ..This Pr would be updated with all latest changes.

.github/workflows/backwards_compatibility_tests_workflow.yml

src/main/java/org/opensearch/knn/index/KNNVectorDVLeafFieldData.java

src/main/java/org/opensearch/knn/index/codec/KNN80Codec/KNN80DocValuesProducer.java

src/main/java/org/opensearch/knn/index/codec/KNN990Codec/NativeEngines990KnnVectorsReader.java

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

src/main/java/org/opensearch/knn/index/query/common/DocAndScoreQuery.java

src/main/java/org/opensearch/knn/index/vectorvalues/KNNVectorValuesIterator.java

Signed-off-by: Vikasht34 <[email protected]>

navneet1v · 2025-02-05T16:42:33Z

src/main/java/org/opensearch/knn/index/KNNVectorScriptDocValues.java

@@ -81,12 +80,13 @@ public float[] get(int i) {
     * @return A KNNVectorScriptDocValues object based on the type of the values.
     * @throws IllegalArgumentException If the type of values is unsupported.
     */
-    public static KNNVectorScriptDocValues create(DocIdSetIterator values, String fieldName, VectorDataType vectorDataType) {
+    public static KNNVectorScriptDocValues create(Object values, String fieldName, VectorDataType vectorDataType) {


can we avoid passing the Object here? Can we use KNNVectorValues abstraction we have created in our k-NN plugin.

I can give a try in next PR. I was thinking to compplety remove all the abstraction we have created. I will discuss with you.

In this case lets cut a GH issue and put it as a TODO as part of 3.0 release that we need to pick that item

navneet1v · 2025-02-06T08:07:55Z

src/main/java/org/opensearch/knn/index/vectorvalues/KNNVectorValuesIterator.java

+        private int lastOrd = -1;
+        @Getter
+        @Setter
+        private Object lastAccessedVector = null;


same as the old comment, lets try to avoid Object data type. Please have a GH issue on this and should be taken as part of next PR.

navneet1v · 2025-02-06T08:12:17Z

src/main/java/org/opensearch/knn/index/vectorvalues/KNNVectorValuesIterator.java

-        protected DocIdSetIterator docIdSetIterator;
-        private static final List<Function<DocIdSetIterator, Boolean>> VALID_ITERATOR_INSTANCE = List.of(
-            (itr) -> itr instanceof BinaryDocValues,
-            (itr) -> itr instanceof FloatVectorValues,
-            (itr) -> itr instanceof ByteVectorValues
-        );
-
-        DocIdsIteratorValues(@NonNull final DocIdSetIterator docIdSetIterator) {
-            validateIteratorType(docIdSetIterator);
+        private final DocIdSetIterator docIdSetIterator;


I think this class will also need a bit of refactoring, for now to unblock main this is fine. Lets have a GH issue for this to fix the abstraction. Because this class otherwise is not going to be maintainable in future

navneet1v

Approving the code. Lets have the follow up GH issues and PR on the refactoring. Since k-NN plugin is dependency for other plugins I am approving this PR so that we can unblock other plugin main branches

Vikasht34 force-pushed the CodecFix branch 3 times, most recently from 988dcd2 to a4e06cd Compare January 23, 2025 20:09

Vikasht34 marked this pull request as ready for review January 24, 2025 20:20

Vikasht34 requested review from heemin32, navneet1v, VijayanB, vamshin, jmazanec15, naveentatikonda, junqiu-lei, martin-gaievski, ryanbogan, luyuncheng, shatejas and 0ctopus13prime as code owners January 24, 2025 20:20

Vikasht34 marked this pull request as draft January 24, 2025 21:49

This was referenced Jan 30, 2025

[RELEASE] Release version 3.0.0 opensearch-project/opensearch-build#3747

Open

[Release 3.0] Update gradle to 8.10.2 to support JDK23 #2460

Merged

Vikasht34 force-pushed the CodecFix branch 8 times, most recently from 934094b to e0f283e Compare February 1, 2025 23:19

Vikasht34 changed the title ~~Fixing Lucene912Codec Issue with BWC for Lucene 10.0.1 upgrade~~ K-NN Plugin Upgrade with Lucene 10.0.1 Feb 2, 2025

Vikasht34 force-pushed the CodecFix branch 9 times, most recently from 1a823aa to f46aae1 Compare February 4, 2025 19:33

Vikasht34 commented Feb 4, 2025

View reviewed changes

.github/workflows/backwards_compatibility_tests_workflow.yml Show resolved Hide resolved

navneet1v reviewed Feb 4, 2025

View reviewed changes

Vikasht34 force-pushed the CodecFix branch from f46aae1 to 538d993 Compare February 4, 2025 23:37

Vikasht34 marked this pull request as ready for review February 4, 2025 23:42

This was referenced Feb 4, 2025

Refactor KNNWeight and DocsAndScoreQuery to Use ScorerSupplier instead of Score for Compatibility with Lucene 10 #2434

Closed

Refactor KNNVectorScriptDocValues for Compatibility with Lucene 10 #2433

Open

KnnPlugin Upgrage with Lucene 10.0.1

bf0d878

Signed-off-by: Vikasht34 <[email protected]>

Vikasht34 force-pushed the CodecFix branch from 538d993 to bf0d878 Compare February 5, 2025 00:08

Vikasht34 requested a review from navneet1v February 5, 2025 01:01

navneet1v reviewed Feb 5, 2025

View reviewed changes

Vikasht34 requested a review from navneet1v February 5, 2025 17:34

navneet1v reviewed Feb 6, 2025

View reviewed changes

navneet1v approved these changes Feb 6, 2025

View reviewed changes

naveentatikonda approved these changes Feb 6, 2025

View reviewed changes

Vikasht34 merged commit c22ba17 into opensearch-project:main Feb 6, 2025
38 checks passed

navneet1v mentioned this pull request Feb 6, 2025

[ANNOUNCEMENT] For features going out in 2.19, please raise PRs to 2.x and then backport to main #2438

Closed

Vikasht34 deleted the CodecFix branch February 6, 2025 18:21

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

K-NN Plugin Upgrade with Lucene 10.0.1 #2429

K-NN Plugin Upgrade with Lucene 10.0.1 #2429

Vikasht34 commented Jan 23, 2025 •

edited

Loading

martin-gaievski commented Jan 24, 2025

Vikasht34 commented Jan 24, 2025

navneet1v Feb 5, 2025 •

edited

Loading

Vikasht34 Feb 5, 2025

navneet1v Feb 6, 2025

navneet1v Feb 6, 2025

navneet1v Feb 6, 2025

navneet1v left a comment

K-NN Plugin Upgrade with Lucene 10.0.1 #2429

K-NN Plugin Upgrade with Lucene 10.0.1 #2429

Conversation

Vikasht34 commented Jan 23, 2025 • edited Loading

Description

Resolves this issues

Check List

martin-gaievski commented Jan 24, 2025

Vikasht34 commented Jan 24, 2025

navneet1v Feb 5, 2025 • edited Loading

Choose a reason for hiding this comment

Vikasht34 Feb 5, 2025

Choose a reason for hiding this comment

navneet1v Feb 6, 2025

Choose a reason for hiding this comment

navneet1v Feb 6, 2025

Choose a reason for hiding this comment

navneet1v Feb 6, 2025

Choose a reason for hiding this comment

navneet1v left a comment

Choose a reason for hiding this comment

Vikasht34 commented Jan 23, 2025 •

edited

Loading

navneet1v Feb 5, 2025 •

edited

Loading