Update knn index default to always do approximate search in test (#2332…

…) (#2350) * Update knn index default to always do approximate search * Update custom mappings * update bwc --------- Signed-off-by: Vijayan Balasubramanian <[email protected]>
opensearch-project · Dec 23, 2024 · 5f4ee5a · 5f4ee5a
1 parent b01edd5
commit 5f4ee5a
Show file tree

Hide file tree

Showing 13 changed files with 68 additions and 18 deletions.
diff --git a/qa/restart-upgrade/src/test/java/org/opensearch/knn/bwc/AbstractRestartUpgradeTestCase.java b/qa/restart-upgrade/src/test/java/org/opensearch/knn/bwc/AbstractRestartUpgradeTestCase.java
@@ -61,4 +61,13 @@ protected static final boolean isRunningAgainstOldCluster() {
     protected final Optional<String> getBWCVersion() {
         return Optional.ofNullable(System.getProperty(BWC_VERSION, null));
     }
+
+    @Override
+    protected Settings getKNNDefaultIndexSettings() {
+        if (isApproximateThresholdSupported(getBWCVersion())) {
+            return super.getKNNDefaultIndexSettings();
+        }
+        // for bwc will return old default setting without approximate value threshold setting
+        return getDefaultIndexSettings();
+    }
 }
diff --git a/qa/rolling-upgrade/src/test/java/org/opensearch/knn/bwc/AbstractRollingUpgradeTestCase.java b/qa/rolling-upgrade/src/test/java/org/opensearch/knn/bwc/AbstractRollingUpgradeTestCase.java
@@ -85,4 +85,13 @@ protected final Optional<String> getBWCVersion() {
         return Optional.ofNullable(System.getProperty(BWC_VERSION, null));
     }
 
+    @Override
+    protected Settings getKNNDefaultIndexSettings() {
+        if (isApproximateThresholdSupported(getBWCVersion())) {
+            return super.getKNNDefaultIndexSettings();
+        }
+        // for bwc will return old default setting without approximate value threshold setting
+        return getDefaultIndexSettings();
+    }
+
 }
diff --git a/qa/rolling-upgrade/src/test/java/org/opensearch/knn/bwc/IndexingIT.java b/qa/rolling-upgrade/src/test/java/org/opensearch/knn/bwc/IndexingIT.java
@@ -21,7 +21,7 @@ public void testKNNDefaultIndexSettings() throws Exception {
         waitForClusterHealthGreen(NODES_BWC_CLUSTER);
         switch (getClusterType()) {
             case OLD:
-                createKnnIndex(testIndex, getKNNDefaultIndexSettings(), createKnnIndexMapping(TEST_FIELD, DIMENSIONS));
+                createKnnIndex(testIndex, getDefaultIndexSettings(), createKnnIndexMapping(TEST_FIELD, DIMENSIONS));
                 int docIdOld = 0;
                 addKNNDocs(testIndex, TEST_FIELD, DIMENSIONS, docIdOld, NUM_DOCS);
                 break;

diff --git a/src/test/java/org/opensearch/knn/KNNSingleNodeTestCase.java b/src/test/java/org/opensearch/knn/KNNSingleNodeTestCase.java
@@ -96,7 +96,7 @@ public void tearDown() throws Exception {
      * Create a k-NN index with default settings
      */
     protected IndexService createKNNIndex(String indexName) {
-        return createIndex(indexName, getKNNDefaultIndexSettings());
+        return createIndex(indexName, getKNNDefaultIndexSettingsBuildsGraphAlways());
     }
 
     /**
@@ -161,13 +161,6 @@ protected void createKnnNestedIndexMapping(String indexName, String fieldPath, I
         OpenSearchAssertions.assertAcked(client().admin().indices().putMapping(request).actionGet());
     }
 
-    /**
-     * Get default k-NN settings for test cases
-     */
-    protected Settings getKNNDefaultIndexSettings() {
-        return Settings.builder().put("number_of_shards", 1).put("number_of_replicas", 0).put("index.knn", true).build();
-    }
-
     /**
      * Get default k-NN settings for test cases with build graph always
      */

diff --git a/src/test/java/org/opensearch/knn/index/KNNESSettingsTestIT.java b/src/test/java/org/opensearch/knn/index/KNNESSettingsTestIT.java
@@ -106,7 +106,11 @@ public void testCreateIndexWithInvalidSpaceType() throws IOException {
     }
 
     public void testUpdateIndexSetting() throws IOException {
-        Settings settings = Settings.builder().put("index.knn", true).put(KNNSettings.KNN_ALGO_PARAM_EF_SEARCH, 512).build();
+        Settings settings = Settings.builder()
+            .put("index.knn", true)
+            .put(KNNSettings.KNN_ALGO_PARAM_EF_SEARCH, 512)
+            .put(KNNSettings.INDEX_KNN_ADVANCED_APPROXIMATE_THRESHOLD, 0)
+            .build();
         createKnnIndex(INDEX_NAME, settings, createKnnIndexMapping(FIELD_NAME, 2));
         assertEquals("512", getIndexSettingByName(INDEX_NAME, KNNSettings.KNN_ALGO_PARAM_EF_SEARCH));
 
@@ -122,7 +126,7 @@ public void testUpdateIndexSetting() throws IOException {
 
     @SuppressWarnings("unchecked")
     public void testCacheRebuiltAfterUpdateIndexSettings() throws Exception {
-        createKnnIndex(INDEX_NAME, buildKNNIndexSettings(ALWAYS_BUILD_GRAPH), createKnnIndexMapping(FIELD_NAME, 2));
+        createKnnIndex(INDEX_NAME, getKNNDefaultIndexSettings(), createKnnIndexMapping(FIELD_NAME, 2));
 
         Float[] vector = { 6.0f, 6.0f };
         addKnnDoc(INDEX_NAME, "1", FIELD_NAME, vector);

diff --git a/src/test/java/org/opensearch/knn/index/OpenSearchIT.java b/src/test/java/org/opensearch/knn/index/OpenSearchIT.java
@@ -640,7 +640,7 @@ public void testKNNIndex_whenBuildGraphThresholdIsPresent_thenGetThresholdValue(
     public void testKNNIndex_whenBuildThresholdIsNotProvided_thenShouldNotReturnSetting() throws Exception {
         final String knnIndexMapping = createKnnIndexMapping(FIELD_NAME, KNNEngine.getMaxDimensionByEngine(KNNEngine.DEFAULT));
         final String indexName = "test-index-with-build-graph-settings";
-        createKnnIndex(indexName, knnIndexMapping);
+        createKnnIndex(indexName, getDefaultIndexSettings(), knnIndexMapping);
         final String buildVectorDataStructureThresholdSetting = getIndexSettingByName(
             indexName,
             KNNSettings.INDEX_KNN_ADVANCED_APPROXIMATE_THRESHOLD
@@ -655,7 +655,7 @@ public void testKNNIndex_whenBuildThresholdIsNotProvided_thenShouldNotReturnSett
     public void testKNNIndex_whenGetIndexSettingWithDefaultIsCalled_thenReturnDefaultBuildGraphThresholdValue() throws Exception {
         final String knnIndexMapping = createKnnIndexMapping(FIELD_NAME, KNNEngine.getMaxDimensionByEngine(KNNEngine.DEFAULT));
         final String indexName = "test-index-with-build-vector-graph-settings";
-        createKnnIndex(indexName, knnIndexMapping);
+        createKnnIndex(indexName, getDefaultIndexSettings(), knnIndexMapping);
         final String buildVectorDataStructureThresholdSetting = getIndexSettingByName(
             indexName,
             KNNSettings.INDEX_KNN_ADVANCED_APPROXIMATE_THRESHOLD,

diff --git a/src/test/java/org/opensearch/knn/integ/ExpandNestedDocsIT.java b/src/test/java/org/opensearch/knn/integ/ExpandNestedDocsIT.java
@@ -19,6 +19,7 @@
 import org.opensearch.core.xcontent.XContentBuilder;
 import org.opensearch.knn.KNNRestTestCase;
 import org.opensearch.knn.NestedKnnDocBuilder;
+import org.opensearch.knn.index.KNNSettings;
 import org.opensearch.knn.index.VectorDataType;
 import org.opensearch.knn.index.engine.KNNEngine;
 import org.opensearch.knn.index.mapper.Mode;
@@ -329,6 +330,7 @@ private void createKnnIndex(
             .put("number_of_shards", numOfShards)
             .put("number_of_replicas", 0)
             .put("index.knn", true)
+            .put(KNNSettings.INDEX_KNN_ADVANCED_APPROXIMATE_THRESHOLD, 0)
             .build();
         createKnnIndex(INDEX_NAME, settings, mapping);
     }

diff --git a/src/test/java/org/opensearch/knn/integ/KNNScriptScoringIT.java b/src/test/java/org/opensearch/knn/integ/KNNScriptScoringIT.java
@@ -14,6 +14,7 @@
 import org.opensearch.knn.KNNRestTestCase;
 import org.opensearch.knn.KNNResult;
 import org.opensearch.knn.common.KNNConstants;
+import org.opensearch.knn.index.KNNSettings;
 import org.opensearch.knn.index.SpaceType;
 import org.apache.http.util.EntityUtils;
 import org.opensearch.client.Request;
@@ -831,7 +832,11 @@ private void createIndexAndAssertScriptScore(
         /*
          * Create knn index and populate data
          */
-        Settings settings = Settings.builder().put("number_of_shards", 1).put("number_of_replicas", 0).put("index.knn", enableKnn).build();
+        Settings.Builder builder = Settings.builder().put("number_of_shards", 1).put("number_of_replicas", 0).put("index.knn", enableKnn);
+        if (enableKnn) {
+            builder.put(KNNSettings.INDEX_KNN_ADVANCED_APPROXIMATE_THRESHOLD, 0);
+        }
+        Settings settings = builder.build();
         createKnnIndex(INDEX_NAME, settings, mapper);
         try {
             final int numDocsWithField = randomIntBetween(4, 10);

diff --git a/src/test/java/org/opensearch/knn/integ/PainlessScriptScoreIT.java b/src/test/java/org/opensearch/knn/integ/PainlessScriptScoreIT.java
@@ -9,6 +9,7 @@
 import org.opensearch.common.settings.Settings;
 import org.opensearch.knn.KNNRestTestCase;
 import org.opensearch.knn.KNNResult;
+import org.opensearch.knn.index.KNNSettings;
 import org.opensearch.knn.index.engine.KNNMethodContext;
 import org.opensearch.knn.index.engine.MethodComponentContext;
 import org.opensearch.knn.index.SpaceType;
@@ -622,7 +623,11 @@ private Response buildIndexAndRunPainlessScript(
         /*
          * Create knn index and populate data
          */
-        Settings settings = Settings.builder().put("number_of_shards", 1).put("number_of_replicas", 0).put("index.knn", enableKnn).build();
+        Settings.Builder builder = Settings.builder().put("number_of_shards", 1).put("number_of_replicas", 0).put("index.knn", enableKnn);
+        if (enableKnn) {
+            builder.put(KNNSettings.INDEX_KNN_ADVANCED_APPROXIMATE_THRESHOLD, 0);
+        }
+        Settings settings = builder.build();
         createKnnIndex(INDEX_NAME, settings, mapper);
         try {
             for (Map.Entry<String, Float[]> data : documents.entrySet()) {

diff --git a/src/test/java/org/opensearch/knn/plugin/action/RestKNNStatsHandlerIT.java b/src/test/java/org/opensearch/knn/plugin/action/RestKNNStatsHandlerIT.java
@@ -25,6 +25,8 @@
 import org.opensearch.index.query.QueryBuilder;
 import org.opensearch.index.query.QueryBuilders;
 import org.opensearch.knn.KNNRestTestCase;
+import org.opensearch.knn.index.KNNSettings;
+import org.opensearch.knn.index.query.KNNQueryBuilder;
 import org.opensearch.knn.index.SpaceType;
 import org.opensearch.knn.index.query.KNNQueryBuilder;
 import org.opensearch.knn.plugin.stats.KNNStats;
@@ -292,7 +294,12 @@ public void testScriptStats_multipleShards() throws Exception {
         // Create an index with a single vector
         createKnnIndex(
             INDEX_NAME,
-            Settings.builder().put("number_of_shards", 2).put("number_of_replicas", 0).put("index.knn", true).build(),
+            Settings.builder()
+                .put("number_of_shards", 2)
+                .put("number_of_replicas", 0)
+                .put("index.knn", true)
+                .put(KNNSettings.INDEX_KNN_ADVANCED_APPROXIMATE_THRESHOLD, 0)
+                .build(),
             createKnnIndexMapping(FIELD_NAME, 2)
         );
 

diff --git a/src/test/java/org/opensearch/knn/plugin/action/RestLegacyKNNStatsHandlerIT.java b/src/test/java/org/opensearch/knn/plugin/action/RestLegacyKNNStatsHandlerIT.java
@@ -12,6 +12,7 @@
 package org.opensearch.knn.plugin.action;
 
 import org.opensearch.knn.KNNRestTestCase;
+import org.opensearch.knn.index.KNNSettings;
 import org.opensearch.knn.index.SpaceType;
 import org.opensearch.knn.index.query.KNNQueryBuilder;
 import org.opensearch.knn.plugin.KNNPlugin;
@@ -266,7 +267,12 @@ public void testScriptStats_multipleShards() throws Exception {
         // Create an index with a single vector
         createKnnIndex(
             INDEX_NAME,
-            Settings.builder().put("number_of_shards", 2).put("number_of_replicas", 0).put("index.knn", true).build(),
+            Settings.builder()
+                .put("number_of_shards", 2)
+                .put("number_of_replicas", 0)
+                .put("index.knn", true)
+                .put(KNNSettings.INDEX_KNN_ADVANCED_APPROXIMATE_THRESHOLD, 0)
+                .build(),
             createKnnIndexMapping(FIELD_NAME, 2)
         );
 

diff --git a/src/test/java/org/opensearch/knn/recall/RecallTestsIT.java b/src/test/java/org/opensearch/knn/recall/RecallTestsIT.java
@@ -46,6 +46,7 @@
 import static org.opensearch.knn.common.KNNConstants.PARAMETERS;
 import static org.opensearch.knn.common.KNNConstants.TYPE;
 import static org.opensearch.knn.common.KNNConstants.TYPE_KNN_VECTOR;
+import static org.opensearch.knn.index.KNNSettings.INDEX_KNN_ADVANCED_APPROXIMATE_THRESHOLD;
 import static org.opensearch.knn.index.KNNSettings.KNN_ALGO_PARAM_EF_SEARCH;
 import static org.opensearch.knn.index.KNNSettings.KNN_ALGO_PARAM_INDEX_THREAD_QTY;
 import static org.opensearch.knn.index.KNNSettings.KNN_MEMORY_CIRCUIT_BREAKER_ENABLED;
@@ -152,6 +153,7 @@ public void testRecall_whenNmslibHnswFP32_thenRecallAbove75percent() {
                     .put("number_of_shards", SHARD_COUNT)
                     .put("number_of_replicas", REPLICA_COUNT)
                     .put("index.knn", true)
+                    .put(INDEX_KNN_ADVANCED_APPROXIMATE_THRESHOLD, 0)
                     .put(KNN_ALGO_PARAM_EF_SEARCH, HNSW_EF_SEARCH)
                     .build(),
                 builder.toString()
@@ -531,6 +533,7 @@ private Settings getSettings() {
             .put("number_of_shards", SHARD_COUNT)
             .put("number_of_replicas", REPLICA_COUNT)
             .put("index.knn", true)
+            .put(INDEX_KNN_ADVANCED_APPROXIMATE_THRESHOLD, 0)
             .build();
     }
 }
diff --git a/src/testFixtures/java/org/opensearch/knn/KNNRestTestCase.java b/src/testFixtures/java/org/opensearch/knn/KNNRestTestCase.java
@@ -797,7 +797,14 @@ protected void updateClusterSettings(String settingKey, Object value) throws Exc
      * Return default index settings for index creation
      */
     protected Settings getKNNDefaultIndexSettings() {
-        return Settings.builder().put("number_of_shards", 1).put("number_of_replicas", 0).put("index.knn", true).build();
+        return buildKNNIndexSettings(0);
+    }
+
+    /**
+     * Return default index settings for index creation
+     */
+    protected Settings getDefaultIndexSettings() {
+        return Settings.builder().put("number_of_shards", 1).put("number_of_replicas", 0).put(KNN_INDEX, true).build();
     }
 
     protected Settings getKNNSegmentReplicatedIndexSettings() {