Skip to content

Commit 8ee82b9

Browse files
VijayanBVijayanB
authored
Update engine for version 2.19 or above (opensearch-project#2501)
Signed-off-by: Balasubramanian <[email protected]>
1 parent 0df6e41 commit 8ee82b9

File tree

6 files changed

+82
-19
lines changed

6 files changed

+82
-19
lines changed

‎release-notes/opensearch-knn.release-notes-2.19.0.0.md

+1
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ Compatible with OpenSearch 2.19.0
3838
* Fix Faiss byte vector efficient filter bug [#2448](https://github.com/opensearch-project/k-NN/pull/2448)
3939
* Fixing bug where mapping accepts both dimension and model-id [#2410](https://github.com/opensearch-project/k-NN/pull/2410)
4040
* Add version check for full field name validation [#2477](https://github.com/opensearch-project/k-NN/pull/2477)
41+
* Update engine for version 2.19 or above [#2501](https://github.com/opensearch-project/k-NN/pull/2501)
4142
### Infrastructure
4243
* Updated C++ version in JNI from c++11 to c++17 [#2259](https://github.com/opensearch-project/k-NN/pull/2259)
4344
* Upgrade bytebuddy and objenesis version to match OpenSearch core and, update github ci runner for macos [#2279](https://github.com/opensearch-project/k-NN/pull/2279)

‎src/main/java/org/opensearch/knn/index/engine/EngineResolver.java

+52-14
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,10 @@
55

66
package org.opensearch.knn.index.engine;
77

8+
import com.google.common.annotations.VisibleForTesting;
89
import org.apache.logging.log4j.LogManager;
910
import org.apache.logging.log4j.Logger;
11+
import org.opensearch.Version;
1012
import org.opensearch.knn.index.mapper.CompressionLevel;
1113
import org.opensearch.knn.index.mapper.Mode;
1214

@@ -22,47 +24,83 @@ public final class EngineResolver {
2224

2325
private EngineResolver() {}
2426

27+
@VisibleForTesting
28+
KNNEngine resolveEngine(KNNMethodConfigContext knnMethodConfigContext, KNNMethodContext knnMethodContext, boolean requiresTraining) {
29+
return logAndReturnEngine(resolveKNNEngine(knnMethodConfigContext, knnMethodContext, requiresTraining, Version.CURRENT));
30+
}
31+
2532
/**
2633
* Based on the provided {@link Mode} and {@link CompressionLevel}, resolve to a {@link KNNEngine}.
2734
*
2835
* @param knnMethodConfigContext configuration context
2936
* @param knnMethodContext KNNMethodContext
3037
* @param requiresTraining whether config requires training
38+
* @param version opensearch index version
3139
* @return {@link KNNEngine}
3240
*/
3341
public KNNEngine resolveEngine(
3442
KNNMethodConfigContext knnMethodConfigContext,
3543
KNNMethodContext knnMethodContext,
36-
boolean requiresTraining
44+
boolean requiresTraining,
45+
Version version
3746
) {
38-
// User configuration gets precedence
39-
if (knnMethodContext != null && knnMethodContext.isEngineConfigured()) {
40-
return logAndReturnEngine(knnMethodContext.getKnnEngine());
47+
return logAndReturnEngine(resolveKNNEngine(knnMethodConfigContext, knnMethodContext, requiresTraining, version));
48+
}
49+
50+
/**
51+
* Based on the provided {@link Mode} and {@link CompressionLevel}, resolve to a {@link KNNEngine}.
52+
*
53+
* @param knnMethodConfigContext configuration context
54+
* @param knnMethodContext KNNMethodContext
55+
* @param requiresTraining whether config requires training
56+
* @param version opensearch index version
57+
* @return {@link KNNEngine}
58+
*/
59+
private KNNEngine resolveKNNEngine(
60+
KNNMethodConfigContext knnMethodConfigContext,
61+
KNNMethodContext knnMethodContext,
62+
boolean requiresTraining,
63+
Version version
64+
) {
65+
// Check user configuration first
66+
if (hasUserConfiguredEngine(knnMethodContext)) {
67+
return knnMethodContext.getKnnEngine();
4168
}
4269

43-
// Faiss is the only engine that supports training, so we default to faiss here for now
70+
// Handle training case
4471
if (requiresTraining) {
45-
return logAndReturnEngine(KNNEngine.FAISS);
72+
// Faiss is the only engine that supports training, so we default to faiss here for now
73+
return KNNEngine.FAISS;
4674
}
4775

4876
Mode mode = knnMethodConfigContext.getMode();
4977
CompressionLevel compressionLevel = knnMethodConfigContext.getCompressionLevel();
78+
5079
// If both mode and compression are not specified, we can just default
5180
if (Mode.isConfigured(mode) == false && CompressionLevel.isConfigured(compressionLevel) == false) {
52-
return logAndReturnEngine(KNNEngine.DEFAULT);
81+
return KNNEngine.DEFAULT;
5382
}
5483

55-
// For 1x, we need to default to faiss if mode is provided and use nmslib otherwise
84+
if (compressionLevel == CompressionLevel.x4) {
85+
// Lucene is only engine that supports 4x - so we have to default to it here.
86+
return KNNEngine.LUCENE;
87+
}
5688
if (CompressionLevel.isConfigured(compressionLevel) == false || compressionLevel == CompressionLevel.x1) {
57-
return logAndReturnEngine(mode == Mode.ON_DISK ? KNNEngine.FAISS : KNNEngine.NMSLIB);
89+
// For 1x or no compression, we need to default to faiss if mode is provided and use nmslib otherwise based on version check
90+
return resolveEngineForX1OrNoCompression(mode, version);
5891
}
92+
return KNNEngine.FAISS;
93+
}
5994

60-
// Lucene is only engine that supports 4x - so we have to default to it here.
61-
if (compressionLevel == CompressionLevel.x4) {
62-
return logAndReturnEngine(KNNEngine.LUCENE);
63-
}
95+
private boolean hasUserConfiguredEngine(KNNMethodContext knnMethodContext) {
96+
return knnMethodContext != null && knnMethodContext.isEngineConfigured();
97+
}
6498

65-
return logAndReturnEngine(KNNEngine.FAISS);
99+
private KNNEngine resolveEngineForX1OrNoCompression(Mode mode, Version version) {
100+
if (version != null && version.onOrAfter(Version.V_2_19_0)) {
101+
return KNNEngine.FAISS;
102+
}
103+
return mode == Mode.ON_DISK ? KNNEngine.FAISS : KNNEngine.NMSLIB;
66104
}
67105

68106
private KNNEngine logAndReturnEngine(KNNEngine knnEngine) {

‎src/main/java/org/opensearch/knn/index/mapper/KNNVectorFieldMapper.java

+2-1
Original file line numberDiff line numberDiff line change
@@ -541,7 +541,8 @@ private void resolveKNNMethodComponents(
541541
KNNEngine resolvedKNNEngine = EngineResolver.INSTANCE.resolveEngine(
542542
builder.knnMethodConfigContext,
543543
builder.originalParameters.getResolvedKnnMethodContext(),
544-
false
544+
false,
545+
builder.indexCreatedVersion
545546
);
546547
setEngine(builder.originalParameters.getResolvedKnnMethodContext(), resolvedKNNEngine);
547548

‎src/main/java/org/opensearch/knn/plugin/transport/TrainingModelRequest.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -138,7 +138,7 @@ public TrainingModelRequest(
138138
.mode(mode)
139139
.build();
140140

141-
KNNEngine knnEngine = EngineResolver.INSTANCE.resolveEngine(knnMethodConfigContext, knnMethodContext, true);
141+
KNNEngine knnEngine = EngineResolver.INSTANCE.resolveEngine(knnMethodConfigContext, knnMethodContext, true, Version.CURRENT);
142142
ResolvedMethodContext resolvedMethodContext = knnEngine.resolveMethod(knnMethodContext, knnMethodConfigContext, true, spaceType);
143143
this.knnMethodContext = resolvedMethodContext.getKnnMethodContext();
144144
this.compressionLevel = resolvedMethodContext.getCompressionLevel();

‎src/test/java/org/opensearch/knn/index/engine/EngineResolverTests.java

+25-2
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55

66
package org.opensearch.knn.index.engine;
77

8+
import org.opensearch.Version;
89
import org.opensearch.knn.KNNTestCase;
910
import org.opensearch.knn.index.SpaceType;
1011
import org.opensearch.knn.index.mapper.CompressionLevel;
@@ -41,10 +42,23 @@ public void testResolveEngine_whenModeAndCompressionAreFalse_thenDefault() {
4142
);
4243
}
4344

44-
public void testResolveEngine_whenModeSpecifiedAndCompressionIsNotSpecified_thenNMSLIB() {
45+
public void testResolveEngine_whenModeSpecifiedAndCompressionIsNotSpecified_whenVersionBefore2_19_thenNMSLIB() {
4546
assertEquals(KNNEngine.DEFAULT, ENGINE_RESOLVER.resolveEngine(KNNMethodConfigContext.builder().build(), null, false));
4647
assertEquals(
4748
KNNEngine.NMSLIB,
49+
ENGINE_RESOLVER.resolveEngine(
50+
KNNMethodConfigContext.builder().mode(Mode.IN_MEMORY).build(),
51+
new KNNMethodContext(KNNEngine.DEFAULT, SpaceType.UNDEFINED, MethodComponentContext.EMPTY, false),
52+
false,
53+
Version.V_2_18_0
54+
)
55+
);
56+
}
57+
58+
public void testResolveEngine_whenModeSpecifiedAndCompressionIsNotSpecified_thenFAISS() {
59+
assertEquals(KNNEngine.DEFAULT, ENGINE_RESOLVER.resolveEngine(KNNMethodConfigContext.builder().build(), null, false));
60+
assertEquals(
61+
KNNEngine.FAISS,
4862
ENGINE_RESOLVER.resolveEngine(
4963
KNNMethodConfigContext.builder().mode(Mode.IN_MEMORY).build(),
5064
new KNNMethodContext(KNNEngine.DEFAULT, SpaceType.UNDEFINED, MethodComponentContext.EMPTY, false),
@@ -63,9 +77,18 @@ public void testResolveEngine_whenCompressionIs1x_thenEngineBasedOnMode() {
6377
)
6478
);
6579
assertEquals(
66-
KNNEngine.NMSLIB,
80+
KNNEngine.FAISS,
6781
ENGINE_RESOLVER.resolveEngine(KNNMethodConfigContext.builder().compressionLevel(CompressionLevel.x1).build(), null, false)
6882
);
83+
assertEquals(
84+
KNNEngine.NMSLIB,
85+
ENGINE_RESOLVER.resolveEngine(
86+
KNNMethodConfigContext.builder().compressionLevel(CompressionLevel.x1).build(),
87+
null,
88+
false,
89+
Version.V_2_18_0
90+
)
91+
);
6992
}
7093

7194
public void testResolveEngine_whenCompressionIs4x_thenEngineIsLucene() {

‎src/test/java/org/opensearch/knn/index/mapper/KNNVectorFieldMapperTests.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -1782,7 +1782,7 @@ public void testTypeParser_whenModeAndCompressionAreSet_thenHandle() throws IOEx
17821782
assertFalse(builder.getOriginalParameters().isLegacyMapping());
17831783
validateBuilderAfterParsing(
17841784
builder,
1785-
KNNEngine.NMSLIB,
1785+
KNNEngine.FAISS,
17861786
SpaceType.L2,
17871787
VectorDataType.FLOAT,
17881788
CompressionLevel.x1,

0 commit comments

Comments
 (0)