From 2a40da4d5a386e44014901e93de28d0f83e8595e Mon Sep 17 00:00:00 2001 From: Naveen Tatikonda Date: Thu, 2 Feb 2023 19:09:07 -0600 Subject: [PATCH] Update codec version to 9.5 (#747) * Update lucene94 package Signed-off-by: Naveen Tatikonda * Add Lucene 9.5 codec and make it new default (#700) * Add Lucene 9.5 codec and make it new default Signed-off-by: Martin Gaievski * Update tests for backwards codecs (#710) Updates tests for backwards codecs to prevent backwards codecs from writing with a read only codec. Signed-off-by: John Mazanec --------- Signed-off-by: Naveen Tatikonda Signed-off-by: Martin Gaievski Signed-off-by: John Mazanec Co-authored-by: Martin Gaievski Co-authored-by: John Mazanec --- DEVELOPER_GUIDE.md | 4 +- .../KNN940PerFieldKnnVectorsFormat.java | 2 +- .../index/codec/KNN950Codec/KNN950Codec.java | 58 +++++++++++++++++++ .../KNN950PerFieldKnnVectorsFormat.java | 28 +++++++++ .../knn/index/codec/KNNCodecVersion.java | 22 ++++++- .../services/org.apache.lucene.codecs.Codec | 3 +- .../codec/KNN940Codec/KNN940CodecTests.java | 18 ++---- .../codec/KNN950Codec/KNN950CodecTests.java | 51 ++++++++++++++++ .../knn/index/codec/KNNCodecFactoryTests.java | 10 +++- 9 files changed, 176 insertions(+), 20 deletions(-) create mode 100644 src/main/java/org/opensearch/knn/index/codec/KNN950Codec/KNN950Codec.java create mode 100644 src/main/java/org/opensearch/knn/index/codec/KNN950Codec/KNN950PerFieldKnnVectorsFormat.java create mode 100644 src/test/java/org/opensearch/knn/index/codec/KNN950Codec/KNN950CodecTests.java diff --git a/DEVELOPER_GUIDE.md b/DEVELOPER_GUIDE.md index 1a47abef8..4a6f360b5 100644 --- a/DEVELOPER_GUIDE.md +++ b/DEVELOPER_GUIDE.md @@ -286,10 +286,12 @@ Before adding any new tests to Backward Compatibility Tests, we should be aware Starting from 2.0 release the new versioning for codec has been introduced. Two positions will be used to define the version, in format 'X.Y', where 'X' corresponds to underlying version of Lucene and 'Y' is the version of the format. +Please note that Lucene version along with corresponding Lucene codec is part of the core OpenSearch. KNN codec should be in sync with Lucene codec version from core OpenSearch. Codec version is used in following classes and methods: - org.opensearch.knn.index.codec.KNNXYCodec.KNNXYCodec -- org.opensearch.knn.index.codec.KNNFormatFactory.createKNNXYFormat +- org.opensearch.knn.index.codec.KNNXYCodec.KNNXYPerFieldKnnVectorsFormat +- org.opensearch.knn.index.codec.KNNCodecVersion These classes and methods are tied directly to Lucene version represented by 'X' part. Other classes use the delegate pattern so no direct tie to Lucene version are related to format and represented by 'Y' diff --git a/src/main/java/org/opensearch/knn/index/codec/KNN940Codec/KNN940PerFieldKnnVectorsFormat.java b/src/main/java/org/opensearch/knn/index/codec/KNN940Codec/KNN940PerFieldKnnVectorsFormat.java index d80c757c9..d9a1a9251 100644 --- a/src/main/java/org/opensearch/knn/index/codec/KNN940Codec/KNN940PerFieldKnnVectorsFormat.java +++ b/src/main/java/org/opensearch/knn/index/codec/KNN940Codec/KNN940PerFieldKnnVectorsFormat.java @@ -5,7 +5,7 @@ package org.opensearch.knn.index.codec.KNN940Codec; -import org.apache.lucene.codecs.lucene94.Lucene94HnswVectorsFormat; +import org.apache.lucene.backward_codecs.lucene94.Lucene94HnswVectorsFormat; import org.opensearch.index.mapper.MapperService; import org.opensearch.knn.index.codec.BasePerFieldKnnVectorsFormat; diff --git a/src/main/java/org/opensearch/knn/index/codec/KNN950Codec/KNN950Codec.java b/src/main/java/org/opensearch/knn/index/codec/KNN950Codec/KNN950Codec.java new file mode 100644 index 000000000..338e54451 --- /dev/null +++ b/src/main/java/org/opensearch/knn/index/codec/KNN950Codec/KNN950Codec.java @@ -0,0 +1,58 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +package org.opensearch.knn.index.codec.KNN950Codec; + +import lombok.Builder; +import org.apache.lucene.codecs.Codec; +import org.apache.lucene.codecs.CompoundFormat; +import org.apache.lucene.codecs.DocValuesFormat; +import org.apache.lucene.codecs.FilterCodec; +import org.apache.lucene.codecs.KnnVectorsFormat; +import org.apache.lucene.codecs.perfield.PerFieldKnnVectorsFormat; +import org.opensearch.knn.index.codec.KNNCodecVersion; +import org.opensearch.knn.index.codec.KNNFormatFacade; + +public class KNN950Codec extends FilterCodec { + private static final KNNCodecVersion VERSION = KNNCodecVersion.V_9_5_0; + private final KNNFormatFacade knnFormatFacade; + private final PerFieldKnnVectorsFormat perFieldKnnVectorsFormat; + + /** + * No arg constructor that uses Lucene95 as the delegate + */ + public KNN950Codec() { + this(VERSION.getDefaultCodecDelegate(), VERSION.getPerFieldKnnVectorsFormat()); + } + + /** + * Sole constructor. When subclassing this codec, create a no-arg ctor and pass the delegate codec + * and a unique name to this ctor. + * + * @param delegate codec that will perform all operations this codec does not override + * @param knnVectorsFormat per field format for KnnVector + */ + @Builder + protected KNN950Codec(Codec delegate, PerFieldKnnVectorsFormat knnVectorsFormat) { + super(VERSION.getCodecName(), delegate); + knnFormatFacade = VERSION.getKnnFormatFacadeSupplier().apply(delegate); + perFieldKnnVectorsFormat = knnVectorsFormat; + } + + @Override + public DocValuesFormat docValuesFormat() { + return knnFormatFacade.docValuesFormat(); + } + + @Override + public CompoundFormat compoundFormat() { + return knnFormatFacade.compoundFormat(); + } + + @Override + public KnnVectorsFormat knnVectorsFormat() { + return perFieldKnnVectorsFormat; + } +} diff --git a/src/main/java/org/opensearch/knn/index/codec/KNN950Codec/KNN950PerFieldKnnVectorsFormat.java b/src/main/java/org/opensearch/knn/index/codec/KNN950Codec/KNN950PerFieldKnnVectorsFormat.java new file mode 100644 index 000000000..66dfcd46e --- /dev/null +++ b/src/main/java/org/opensearch/knn/index/codec/KNN950Codec/KNN950PerFieldKnnVectorsFormat.java @@ -0,0 +1,28 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +package org.opensearch.knn.index.codec.KNN950Codec; + +import org.apache.lucene.codecs.lucene95.Lucene95HnswVectorsFormat; +import org.opensearch.index.mapper.MapperService; +import org.opensearch.knn.index.codec.BasePerFieldKnnVectorsFormat; + +import java.util.Optional; + +/** + * Class provides per field format implementation for Lucene Knn vector type + */ +public class KNN950PerFieldKnnVectorsFormat extends BasePerFieldKnnVectorsFormat { + + public KNN950PerFieldKnnVectorsFormat(final Optional mapperService) { + super( + mapperService, + Lucene95HnswVectorsFormat.DEFAULT_MAX_CONN, + Lucene95HnswVectorsFormat.DEFAULT_BEAM_WIDTH, + () -> new Lucene95HnswVectorsFormat(), + (maxConnm, beamWidth) -> new Lucene95HnswVectorsFormat(maxConnm, beamWidth) + ); + } +} diff --git a/src/main/java/org/opensearch/knn/index/codec/KNNCodecVersion.java b/src/main/java/org/opensearch/knn/index/codec/KNNCodecVersion.java index a6d699484..cbf6680f7 100644 --- a/src/main/java/org/opensearch/knn/index/codec/KNNCodecVersion.java +++ b/src/main/java/org/opensearch/knn/index/codec/KNNCodecVersion.java @@ -10,7 +10,8 @@ import org.apache.lucene.backward_codecs.lucene91.Lucene91Codec; import org.apache.lucene.backward_codecs.lucene92.Lucene92Codec; import org.apache.lucene.codecs.Codec; -import org.apache.lucene.codecs.lucene94.Lucene94Codec; +import org.apache.lucene.backward_codecs.lucene94.Lucene94Codec; +import org.apache.lucene.codecs.lucene95.Lucene95Codec; import org.apache.lucene.codecs.perfield.PerFieldKnnVectorsFormat; import org.opensearch.index.mapper.MapperService; import org.opensearch.knn.index.codec.KNN80Codec.KNN80CompoundFormat; @@ -20,6 +21,8 @@ import org.opensearch.knn.index.codec.KNN920Codec.KNN920PerFieldKnnVectorsFormat; import org.opensearch.knn.index.codec.KNN940Codec.KNN940Codec; import org.opensearch.knn.index.codec.KNN940Codec.KNN940PerFieldKnnVectorsFormat; +import org.opensearch.knn.index.codec.KNN950Codec.KNN950Codec; +import org.opensearch.knn.index.codec.KNN950Codec.KNN950PerFieldKnnVectorsFormat; import java.util.Optional; import java.util.function.BiFunction; @@ -74,9 +77,24 @@ public enum KNNCodecVersion { .knnVectorsFormat(new KNN940PerFieldKnnVectorsFormat(Optional.ofNullable(mapperService))) .build(), KNN940Codec::new + ), + + V_9_5_0( + "KNN950Codec", + new Lucene95Codec(), + new KNN950PerFieldKnnVectorsFormat(Optional.empty()), + (delegate) -> new KNNFormatFacade( + new KNN80DocValuesFormat(delegate.docValuesFormat()), + new KNN80CompoundFormat(delegate.compoundFormat()) + ), + (userCodec, mapperService) -> KNN950Codec.builder() + .delegate(userCodec) + .knnVectorsFormat(new KNN950PerFieldKnnVectorsFormat(Optional.ofNullable(mapperService))) + .build(), + KNN950Codec::new ); - private static final KNNCodecVersion CURRENT = V_9_4_0; + private static final KNNCodecVersion CURRENT = V_9_5_0; private final String codecName; private final Codec defaultCodecDelegate; diff --git a/src/main/resources/META-INF/services/org.apache.lucene.codecs.Codec b/src/main/resources/META-INF/services/org.apache.lucene.codecs.Codec index 8185e7858..5c44d5756 100644 --- a/src/main/resources/META-INF/services/org.apache.lucene.codecs.Codec +++ b/src/main/resources/META-INF/services/org.apache.lucene.codecs.Codec @@ -4,4 +4,5 @@ org.opensearch.knn.index.codec.KNN86Codec.KNN86Codec org.opensearch.knn.index.codec.KNN87Codec.KNN87Codec org.opensearch.knn.index.codec.KNN910Codec.KNN910Codec org.opensearch.knn.index.codec.KNN920Codec.KNN920Codec -org.opensearch.knn.index.codec.KNN940Codec.KNN940Codec \ No newline at end of file +org.opensearch.knn.index.codec.KNN940Codec.KNN940Codec +org.opensearch.knn.index.codec.KNN950Codec.KNN950Codec \ No newline at end of file diff --git a/src/test/java/org/opensearch/knn/index/codec/KNN940Codec/KNN940CodecTests.java b/src/test/java/org/opensearch/knn/index/codec/KNN940Codec/KNN940CodecTests.java index 1101d93bb..805edac9d 100644 --- a/src/test/java/org/opensearch/knn/index/codec/KNN940Codec/KNN940CodecTests.java +++ b/src/test/java/org/opensearch/knn/index/codec/KNN940Codec/KNN940CodecTests.java @@ -6,13 +6,9 @@ package org.opensearch.knn.index.codec.KNN940Codec; import org.apache.lucene.codecs.Codec; -import org.apache.lucene.codecs.perfield.PerFieldKnnVectorsFormat; -import org.opensearch.index.mapper.MapperService; import org.opensearch.knn.index.codec.KNNCodecTestCase; import java.io.IOException; -import java.util.Optional; import java.util.concurrent.ExecutionException; -import java.util.function.Function; import static org.opensearch.knn.index.codec.KNNCodecVersion.V_9_4_0; @@ -26,15 +22,9 @@ public void testBuildFromModelTemplate() throws InterruptedException, ExecutionE testBuildFromModelTemplate((KNN940Codec.builder().delegate(V_9_4_0.getDefaultCodecDelegate()).build())); } - public void testKnnVectorIndex() throws Exception { - Function perFieldKnnVectorsFormatProvider = ( - mapperService) -> new KNN940PerFieldKnnVectorsFormat(Optional.of(mapperService)); - - Function knnCodecProvider = (knnVectorFormat) -> KNN940Codec.builder() - .delegate(V_9_4_0.getDefaultCodecDelegate()) - .knnVectorsFormat(knnVectorFormat) - .build(); - - testKnnVectorIndex(knnCodecProvider, perFieldKnnVectorsFormatProvider); + // Ensure that the codec is able to return the correct per field knn vectors format for codec + public void testCodecSetsCustomPerFieldKnnVectorsFormat() { + final Codec codec = new KNN940Codec(); + assertTrue(codec.knnVectorsFormat() instanceof KNN940PerFieldKnnVectorsFormat); } } diff --git a/src/test/java/org/opensearch/knn/index/codec/KNN950Codec/KNN950CodecTests.java b/src/test/java/org/opensearch/knn/index/codec/KNN950Codec/KNN950CodecTests.java new file mode 100644 index 000000000..8eafb6a4a --- /dev/null +++ b/src/test/java/org/opensearch/knn/index/codec/KNN950Codec/KNN950CodecTests.java @@ -0,0 +1,51 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +package org.opensearch.knn.index.codec.KNN950Codec; + +import lombok.SneakyThrows; +import org.apache.lucene.codecs.Codec; +import org.apache.lucene.codecs.perfield.PerFieldKnnVectorsFormat; +import org.opensearch.index.mapper.MapperService; +import org.opensearch.knn.index.codec.KNNCodecTestCase; + +import java.util.Optional; +import java.util.function.Function; + +import static org.opensearch.knn.index.codec.KNNCodecVersion.V_9_5_0; + +public class KNN950CodecTests extends KNNCodecTestCase { + + @SneakyThrows + public void testMultiFieldsKnnIndex() { + testMultiFieldsKnnIndex(KNN950Codec.builder().delegate(V_9_5_0.getDefaultCodecDelegate()).build()); + } + + @SneakyThrows + public void testBuildFromModelTemplate() { + testBuildFromModelTemplate((KNN950Codec.builder().delegate(V_9_5_0.getDefaultCodecDelegate()).build())); + } + + // Ensure that the codec is able to return the correct per field knn vectors format for codec + public void testCodecSetsCustomPerFieldKnnVectorsFormat() { + final Codec codec = new KNN950Codec(); + assertTrue(codec.knnVectorsFormat() instanceof KNN950PerFieldKnnVectorsFormat); + } + + // IMPORTANT: When this Codec is moved to a backwards Codec, this test needs to be removed, because it attempts to + // write with a read only codec, which will fail + @SneakyThrows + public void testKnnVectorIndex() { + Function perFieldKnnVectorsFormatProvider = ( + mapperService) -> new KNN950PerFieldKnnVectorsFormat(Optional.of(mapperService)); + + Function knnCodecProvider = (knnVectorFormat) -> KNN950Codec.builder() + .delegate(V_9_5_0.getDefaultCodecDelegate()) + .knnVectorsFormat(knnVectorFormat) + .build(); + + testKnnVectorIndex(knnCodecProvider, perFieldKnnVectorsFormatProvider); + } +} diff --git a/src/test/java/org/opensearch/knn/index/codec/KNNCodecFactoryTests.java b/src/test/java/org/opensearch/knn/index/codec/KNNCodecFactoryTests.java index d918f5439..2ec953b18 100644 --- a/src/test/java/org/opensearch/knn/index/codec/KNNCodecFactoryTests.java +++ b/src/test/java/org/opensearch/knn/index/codec/KNNCodecFactoryTests.java @@ -8,12 +8,14 @@ import org.apache.lucene.backward_codecs.lucene92.Lucene92Codec; import org.apache.lucene.codecs.Codec; import org.apache.lucene.backward_codecs.lucene91.Lucene91Codec; -import org.apache.lucene.codecs.lucene94.Lucene94Codec; +import org.apache.lucene.backward_codecs.lucene94.Lucene94Codec; +import org.apache.lucene.codecs.lucene95.Lucene95Codec; import org.opensearch.knn.KNNTestCase; import static org.opensearch.knn.index.codec.KNNCodecVersion.V_9_1_0; import static org.opensearch.knn.index.codec.KNNCodecVersion.V_9_2_0; import static org.opensearch.knn.index.codec.KNNCodecVersion.V_9_4_0; +import static org.opensearch.knn.index.codec.KNNCodecVersion.V_9_5_0; public class KNNCodecFactoryTests extends KNNTestCase { @@ -35,6 +37,12 @@ public void testKNN940Codec() { assertNotNull(V_9_4_0.getKnnFormatFacadeSupplier().apply(V_9_4_0.getDefaultCodecDelegate())); } + public void testKNN950Codec() { + assertDelegateForVersion(V_9_5_0, Lucene95Codec.class); + assertNotNull(V_9_5_0.getPerFieldKnnVectorsFormat()); + assertNotNull(V_9_5_0.getKnnFormatFacadeSupplier().apply(V_9_5_0.getDefaultCodecDelegate())); + } + private void assertDelegateForVersion(final KNNCodecVersion codecVersion, final Class expectedCodecClass) { final Codec defaultDelegate = codecVersion.getDefaultCodecDelegate(); assertNotNull(defaultDelegate);