From 36d9d7ff4d799dc34c98c336b10578068112aa89 Mon Sep 17 00:00:00 2001 From: Vijayan Balasubramanian Date: Thu, 24 Oct 2024 14:52:47 -0700 Subject: [PATCH] Update bwc test to update index setting Signed-off-by: Vijayan Balasubramanian --- CHANGELOG.md | 1 + .../org/opensearch/knn/bwc/ClearCacheIT.java | 9 +++++- .../org/opensearch/knn/bwc/IndexingIT.java | 16 +++++------ .../java/org/opensearch/knn/bwc/WarmupIT.java | 22 +++++++++++---- .../org/opensearch/knn/bwc/ClearCacheIT.java | 7 ++++- .../java/org/opensearch/knn/bwc/WarmupIT.java | 5 +++- .../org/opensearch/knn/KNNRestTestCase.java | 28 +++++++++++++++++-- 7 files changed, 69 insertions(+), 19 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a5fcadee3..811dc9856 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,4 +21,5 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), ### Infrastructure ### Documentation ### Maintenance +* Select index settings based on cluster version[2236](https://github.com/opensearch-project/k-NN/pull/2236) ### Refactoring diff --git a/qa/restart-upgrade/src/test/java/org/opensearch/knn/bwc/ClearCacheIT.java b/qa/restart-upgrade/src/test/java/org/opensearch/knn/bwc/ClearCacheIT.java index f93f2f883..e4ac25000 100644 --- a/qa/restart-upgrade/src/test/java/org/opensearch/knn/bwc/ClearCacheIT.java +++ b/qa/restart-upgrade/src/test/java/org/opensearch/knn/bwc/ClearCacheIT.java @@ -5,7 +5,10 @@ package org.opensearch.knn.bwc; +import org.opensearch.common.settings.Settings; + import java.util.Collections; + import static org.opensearch.knn.TestUtils.NODES_BWC_CLUSTER; public class ClearCacheIT extends AbstractRestartUpgradeTestCase { @@ -20,7 +23,11 @@ public class ClearCacheIT extends AbstractRestartUpgradeTestCase { public void testClearCache() throws Exception { waitForClusterHealthGreen(NODES_BWC_CLUSTER); if (isRunningAgainstOldCluster()) { - createKnnIndex(testIndex, getKNNDefaultIndexSettings(), createKnnIndexMapping(TEST_FIELD, DIMENSIONS)); + // if approximate threshold is supported, set value to 0, to build graph always + Settings indexSettings = isApproximateThresholdSupported(getBWCVersion()) + ? buildKNNIndexSettings(0) + : getKNNDefaultIndexSettings(); + createKnnIndex(testIndex, indexSettings, createKnnIndexMapping(TEST_FIELD, DIMENSIONS)); addKNNDocs(testIndex, TEST_FIELD, DIMENSIONS, docId, NUM_DOCS); queryCnt = NUM_DOCS; int graphCount = getTotalGraphsInCache(); diff --git a/qa/restart-upgrade/src/test/java/org/opensearch/knn/bwc/IndexingIT.java b/qa/restart-upgrade/src/test/java/org/opensearch/knn/bwc/IndexingIT.java index bf4c8f7ec..7d2b3807a 100644 --- a/qa/restart-upgrade/src/test/java/org/opensearch/knn/bwc/IndexingIT.java +++ b/qa/restart-upgrade/src/test/java/org/opensearch/knn/bwc/IndexingIT.java @@ -143,15 +143,15 @@ public void testKNNIndexCustomLegacyFieldMapping() throws Exception { // When the cluster is in old version, create a KNN index with custom legacy field mapping settings // and add documents into that index if (isRunningAgainstOldCluster()) { - createKnnIndex( - testIndex, - createKNNIndexCustomLegacyFieldMappingSettings( - SpaceType.LINF, - KNN_ALGO_PARAM_M_MIN_VALUE, - KNN_ALGO_PARAM_EF_CONSTRUCTION_MIN_VALUE - ), - createKnnIndexMapping(TEST_FIELD, DIMENSIONS) + Settings.Builder indexMappingSettings = createKNNIndexCustomLegacyFieldMappingIndexSettingsBuilder( + SpaceType.LINF, + KNN_ALGO_PARAM_M_MIN_VALUE, + KNN_ALGO_PARAM_EF_CONSTRUCTION_MIN_VALUE ); + if (isApproximateThresholdSupported(getBWCVersion())) { + indexMappingSettings.put(KNNSettings.INDEX_KNN_ADVANCED_APPROXIMATE_THRESHOLD, 0); + } + createKnnIndex(testIndex, indexMappingSettings.build(), createKnnIndexMapping(TEST_FIELD, DIMENSIONS)); addKNNDocs(testIndex, TEST_FIELD, DIMENSIONS, DOC_ID, NUM_DOCS); } else { validateKNNIndexingOnUpgrade(NUM_DOCS); diff --git a/qa/restart-upgrade/src/test/java/org/opensearch/knn/bwc/WarmupIT.java b/qa/restart-upgrade/src/test/java/org/opensearch/knn/bwc/WarmupIT.java index db3555a8d..64c55f6fd 100644 --- a/qa/restart-upgrade/src/test/java/org/opensearch/knn/bwc/WarmupIT.java +++ b/qa/restart-upgrade/src/test/java/org/opensearch/knn/bwc/WarmupIT.java @@ -32,7 +32,10 @@ public void testKNNWarmupDefaultLegacyFieldMapping() throws Exception { waitForClusterHealthGreen(NODES_BWC_CLUSTER); if (isRunningAgainstOldCluster()) { - createKnnIndex(testIndex, getKNNDefaultIndexSettings(), createKnnIndexMapping(TEST_FIELD, DIMENSIONS)); + Settings indexSettings = isApproximateThresholdSupported(getBWCVersion()) + ? buildKNNIndexSettings(0) + : getKNNDefaultIndexSettings(); + createKnnIndex(testIndex, indexSettings, createKnnIndexMapping(TEST_FIELD, DIMENSIONS)); addKNNDocs(testIndex, TEST_FIELD, DIMENSIONS, DOC_ID, NUM_DOCS); } else { // update index setting to allow build graph always since we test graph count that are loaded into memory @@ -48,13 +51,16 @@ public void testKNNWarmupCustomLegacyFieldMapping() throws Exception { // When the cluster is in old version, create a KNN index with custom legacy field mapping settings // and add documents into that index if (isRunningAgainstOldCluster()) { - Settings indexMappingSettings = createKNNIndexCustomLegacyFieldMappingSettings( + Settings.Builder indexMappingSettings = createKNNIndexCustomLegacyFieldMappingIndexSettingsBuilder( SpaceType.LINF, KNN_ALGO_PARAM_M_MIN_VALUE, KNN_ALGO_PARAM_EF_CONSTRUCTION_MIN_VALUE ); + if (isApproximateThresholdSupported(getBWCVersion())) { + indexMappingSettings.put(KNNSettings.INDEX_KNN_ADVANCED_APPROXIMATE_THRESHOLD, 0); + } String indexMapping = createKnnIndexMapping(TEST_FIELD, DIMENSIONS); - createKnnIndex(testIndex, indexMappingSettings, indexMapping); + createKnnIndex(testIndex, indexMappingSettings.build(), indexMapping); addKNNDocs(testIndex, TEST_FIELD, DIMENSIONS, DOC_ID, NUM_DOCS); } else { validateKNNWarmupOnUpgrade(); @@ -65,7 +71,10 @@ public void testKNNWarmupCustomLegacyFieldMapping() throws Exception { // space_type : "l2", engine : "nmslib", m : 16, ef_construction : 512 public void testKNNWarmupDefaultMethodFieldMapping() throws Exception { if (isRunningAgainstOldCluster()) { - createKnnIndex(testIndex, getKNNDefaultIndexSettings(), createKNNIndexMethodFieldMapping(TEST_FIELD, DIMENSIONS)); + Settings indexSettings = isApproximateThresholdSupported(getBWCVersion()) + ? buildKNNIndexSettings(0) + : getKNNDefaultIndexSettings(); + createKnnIndex(testIndex, indexSettings, createKNNIndexMethodFieldMapping(TEST_FIELD, DIMENSIONS)); addKNNDocs(testIndex, TEST_FIELD, DIMENSIONS, DOC_ID, NUM_DOCS); } else { // update index setting to allow build graph always since we test graph count that are loaded into memory @@ -78,9 +87,12 @@ public void testKNNWarmupDefaultMethodFieldMapping() throws Exception { // space_type : "innerproduct", engine : "faiss", m : 50, ef_construction : 1024 public void testKNNWarmupCustomMethodFieldMapping() throws Exception { if (isRunningAgainstOldCluster()) { + Settings indexSettings = isApproximateThresholdSupported(getBWCVersion()) + ? buildKNNIndexSettings(0) + : getKNNDefaultIndexSettings(); createKnnIndex( testIndex, - getKNNDefaultIndexSettings(), + indexSettings, createKNNIndexCustomMethodFieldMapping(TEST_FIELD, DIMENSIONS, SpaceType.INNER_PRODUCT, FAISS_NAME, M, EF_CONSTRUCTION) ); addKNNDocs(testIndex, TEST_FIELD, DIMENSIONS, DOC_ID, NUM_DOCS); diff --git a/qa/rolling-upgrade/src/test/java/org/opensearch/knn/bwc/ClearCacheIT.java b/qa/rolling-upgrade/src/test/java/org/opensearch/knn/bwc/ClearCacheIT.java index 5ef3b1d97..0e8748eb6 100644 --- a/qa/rolling-upgrade/src/test/java/org/opensearch/knn/bwc/ClearCacheIT.java +++ b/qa/rolling-upgrade/src/test/java/org/opensearch/knn/bwc/ClearCacheIT.java @@ -5,6 +5,8 @@ package org.opensearch.knn.bwc; +import org.opensearch.common.settings.Settings; + import java.util.Collections; import static org.opensearch.knn.TestUtils.NODES_BWC_CLUSTER; @@ -22,7 +24,10 @@ public void testClearCache() throws Exception { waitForClusterHealthGreen(NODES_BWC_CLUSTER); switch (getClusterType()) { case OLD: - createKnnIndex(testIndex, getKNNDefaultIndexSettings(), createKnnIndexMapping(TEST_FIELD, DIMENSIONS)); + Settings indexSettings = isApproximateThresholdSupported(getBWCVersion()) + ? buildKNNIndexSettings(0) + : getKNNDefaultIndexSettings(); + createKnnIndex(testIndex, indexSettings, createKnnIndexMapping(TEST_FIELD, DIMENSIONS)); int docIdOld = 0; addKNNDocs(testIndex, TEST_FIELD, DIMENSIONS, docIdOld, NUM_DOCS); int graphCount = getTotalGraphsInCache(); diff --git a/qa/rolling-upgrade/src/test/java/org/opensearch/knn/bwc/WarmupIT.java b/qa/rolling-upgrade/src/test/java/org/opensearch/knn/bwc/WarmupIT.java index c8f9ad6e3..c665c3e86 100644 --- a/qa/rolling-upgrade/src/test/java/org/opensearch/knn/bwc/WarmupIT.java +++ b/qa/rolling-upgrade/src/test/java/org/opensearch/knn/bwc/WarmupIT.java @@ -22,7 +22,10 @@ public void testKNNWarmup() throws Exception { waitForClusterHealthGreen(NODES_BWC_CLUSTER); switch (getClusterType()) { case OLD: - createKnnIndex(testIndex, getKNNDefaultIndexSettings(), createKnnIndexMapping(TEST_FIELD, DIMENSIONS)); + Settings indexSettings = isApproximateThresholdSupported(getBWCVersion()) + ? buildKNNIndexSettings(0) + : getKNNDefaultIndexSettings(); + createKnnIndex(testIndex, indexSettings, createKnnIndexMapping(TEST_FIELD, DIMENSIONS)); addKNNDocs(testIndex, TEST_FIELD, DIMENSIONS, 0, NUM_DOCS); break; case MIXED: diff --git a/src/testFixtures/java/org/opensearch/knn/KNNRestTestCase.java b/src/testFixtures/java/org/opensearch/knn/KNNRestTestCase.java index 93f6b0792..8a4885cfe 100644 --- a/src/testFixtures/java/org/opensearch/knn/KNNRestTestCase.java +++ b/src/testFixtures/java/org/opensearch/knn/KNNRestTestCase.java @@ -13,6 +13,7 @@ import org.apache.commons.lang.StringUtils; import org.apache.hc.core5.http.io.entity.EntityUtils; import org.apache.hc.core5.net.URIBuilder; +import org.opensearch.Version; import org.opensearch.core.common.bytes.BytesReference; import org.opensearch.common.xcontent.XContentHelper; import org.opensearch.core.xcontent.DeprecationHandler; @@ -64,6 +65,7 @@ import java.util.Locale; import java.util.Map; import java.util.Objects; +import java.util.Optional; import java.util.PriorityQueue; import java.util.Set; import java.util.concurrent.TimeUnit; @@ -1345,15 +1347,22 @@ public void validateKNNSearch(String testIndex, String testField, int dimension, } } - protected Settings createKNNIndexCustomLegacyFieldMappingSettings(SpaceType spaceType, Integer m, Integer ef_construction) { + protected Settings.Builder createKNNIndexCustomLegacyFieldMappingIndexSettingsBuilder( + SpaceType spaceType, + Integer m, + Integer ef_construction + ) { return Settings.builder() .put(NUMBER_OF_SHARDS, 1) .put(NUMBER_OF_REPLICAS, 0) .put(INDEX_KNN, true) .put(KNNSettings.KNN_SPACE_TYPE, spaceType.getValue()) .put(KNNSettings.KNN_ALGO_PARAM_M, m) - .put(KNNSettings.KNN_ALGO_PARAM_EF_CONSTRUCTION, ef_construction) - .build(); + .put(KNNSettings.KNN_ALGO_PARAM_EF_CONSTRUCTION, ef_construction); + } + + protected Settings createKNNIndexCustomLegacyFieldMappingIndexSettings(SpaceType spaceType, Integer m, Integer ef_construction) { + return createKNNIndexCustomLegacyFieldMappingIndexSettingsBuilder(spaceType, m, ef_construction).build(); } public String createKNNIndexMethodFieldMapping(String fieldName, Integer dimensions) throws IOException { @@ -1859,4 +1868,17 @@ protected XContentBuilder buildSearchQuery(String fieldName, int k, float[] vect builder.endObject().endObject().endObject().endObject(); return builder; } + + // approximate threshold parameter is only supported on or after V_2_18_0 + protected boolean isApproximateThresholdSupported(final Optional bwcVersion) { + if (bwcVersion.isEmpty()) { + return false; + } + String versionString = bwcVersion.get(); + if (versionString.endsWith("-SNAPSHOT")) { + versionString = versionString.substring(0, versionString.length() - 9); + } + final Version version = Version.fromString(versionString); + return version.onOrAfter(Version.V_2_18_0); + } }