Skip to content
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

Add Lucene 9.5 codec and make it new default #700

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion DEVELOPER_GUIDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down
Original file line number Diff line number Diff line change
@@ -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;
}
}
Original file line number Diff line number Diff line change
@@ -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> mapperService) {
super(
mapperService,
Lucene95HnswVectorsFormat.DEFAULT_MAX_CONN,
Lucene95HnswVectorsFormat.DEFAULT_BEAM_WIDTH,
() -> new Lucene95HnswVectorsFormat(),
(maxConnm, beamWidth) -> new Lucene95HnswVectorsFormat(maxConnm, beamWidth)
);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
import org.apache.lucene.backward_codecs.lucene92.Lucene92Codec;
import org.apache.lucene.backward_codecs.lucene94.Lucene94Codec;
import org.apache.lucene.codecs.Codec;
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;
Expand All @@ -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;
Expand Down Expand Up @@ -74,9 +77,24 @@ public enum KNNCodecVersion {
.knnVectorsFormat(new KNN940PerFieldKnnVectorsFormat(Optional.ofNullable(mapperService)))
.build(),
KNN940Codec::new
),

V_9_5_0(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Question]: The other enums which are present in the like V_9_4_0, are they getting used somewhere? if not lets remove them.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

they are referenced from codec classes, like here. So if we keep older codecs we also need that other enum members.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For clarification - in enum we do have only codec versions starting from 9.1 (9.1.0, 9.2.0, 9.4.0). There are no references for older codecs, so it's not a problem if we decide to drop them in future to be consistent with what core OS supports.

"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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
org.opensearch.knn.index.codec.KNN940Codec.KNN940Codec
org.opensearch.knn.index.codec.KNN950Codec.KNN950Codec
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
/*
* 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()));
}

@SneakyThrows
public void testKnnVectorIndex() {
Function<MapperService, PerFieldKnnVectorsFormat> perFieldKnnVectorsFormatProvider = (
mapperService) -> new KNN950PerFieldKnnVectorsFormat(Optional.of(mapperService));

Function<PerFieldKnnVectorsFormat, Codec> knnCodecProvider = (knnVectorFormat) -> KNN950Codec.builder()
.delegate(V_9_5_0.getDefaultCodecDelegate())
.knnVectorsFormat(knnVectorFormat)
.build();

testKnnVectorIndex(knnCodecProvider, perFieldKnnVectorsFormatProvider);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,13 @@
import org.apache.lucene.codecs.Codec;
import org.apache.lucene.backward_codecs.lucene91.Lucene91Codec;
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 {

Expand All @@ -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);
Expand Down