Update approximate_threshold to 15K documents (opensearch-project#2229)

* Update threshold to 15K documents After comparing indexing and search performance, we are updating default value to be 15000. Signed-off-by: Vijayan Balasubramanian <[email protected]> * Fix bwc test Signed-off-by: Vijayan Balasubramanian <[email protected]> * Update test method Signed-off-by: Vijayan Balasubramanian <[email protected]> * Flush data after index Signed-off-by: Vijayan Balasubramanian <[email protected]> --------- Signed-off-by: Vijayan Balasubramanian <[email protected]>
VijayanB · Oct 23, 2024 · b30b971 · b30b971
1 parent be0837d
commit b30b971
Show file tree

Hide file tree

Showing 23 changed files with 103 additions and 44 deletions.
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
@@ -22,28 +22,19 @@ public void testClearCache() throws Exception {
         if (isRunningAgainstOldCluster()) {
             createKnnIndex(testIndex, getKNNDefaultIndexSettings(), createKnnIndexMapping(TEST_FIELD, DIMENSIONS));
             addKNNDocs(testIndex, TEST_FIELD, DIMENSIONS, docId, NUM_DOCS);
-        } else {
             queryCnt = NUM_DOCS;
-            validateClearCacheOnUpgrade(queryCnt);
-
-            docId = NUM_DOCS;
-            addKNNDocs(testIndex, TEST_FIELD, DIMENSIONS, docId, NUM_DOCS);
-
-            queryCnt = queryCnt + NUM_DOCS;
+            int graphCount = getTotalGraphsInCache();
+            knnWarmup(Collections.singletonList(testIndex));
+            assertTrue(getTotalGraphsInCache() > graphCount);
+        } else {
             validateClearCacheOnUpgrade(queryCnt);
             deleteKNNIndex(testIndex);
         }
     }
 
     // validation steps for Clear Cache API after upgrading node to new version
     private void validateClearCacheOnUpgrade(int queryCount) throws Exception {
-        int graphCount = getTotalGraphsInCache();
-        knnWarmup(Collections.singletonList(testIndex));
-        assertTrue(getTotalGraphsInCache() > graphCount);
-        validateKNNSearch(testIndex, TEST_FIELD, DIMENSIONS, queryCount, K);
-
         clearCache(Collections.singletonList(testIndex));
         assertEquals(0, getTotalGraphsInCache());
-        validateKNNSearch(testIndex, TEST_FIELD, DIMENSIONS, queryCount, K);
     }
 }
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
@@ -6,11 +6,14 @@
 package org.opensearch.knn.bwc;
 
 import org.junit.Assert;
+import org.opensearch.common.settings.Settings;
 import org.opensearch.common.xcontent.XContentFactory;
+import org.opensearch.knn.index.KNNSettings;
 import org.opensearch.knn.index.SpaceType;
 import org.opensearch.knn.index.VectorDataType;
 import org.opensearch.knn.index.engine.KNNEngine;
 
+import java.util.List;
 import java.util.Map;
 
 import static org.opensearch.knn.TestUtils.KNN_ALGO_PARAM_EF_CONSTRUCTION_MIN_VALUE;
@@ -52,6 +55,8 @@ public void testKNNIndexDefaultLegacyFieldMapping() throws Exception {
             createKnnIndex(testIndex, getKNNDefaultIndexSettings(), 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
+            updateIndexSettings(testIndex, Settings.builder().put(KNNSettings.INDEX_KNN_ADVANCED_APPROXIMATE_THRESHOLD, 0));
             validateKNNIndexingOnUpgrade(NUM_DOCS);
         }
     }
@@ -275,13 +280,14 @@ public void testNoParametersOnUpgrade() throws Exception {
 
     // KNN indexing tests when the cluster is upgraded to latest version
     public void validateKNNIndexingOnUpgrade(int numOfDocs) throws Exception {
+        updateIndexSettings(testIndex, Settings.builder().put(KNNSettings.INDEX_KNN_ADVANCED_APPROXIMATE_THRESHOLD, 0));
+        forceMergeKnnIndex(testIndex);
         QUERY_COUNT = numOfDocs;
         validateKNNSearch(testIndex, TEST_FIELD, DIMENSIONS, QUERY_COUNT, K);
-        cleanUpCache();
+        clearCache(List.of(testIndex));
         DOC_ID = numOfDocs;
         addKNNDocs(testIndex, TEST_FIELD, DIMENSIONS, DOC_ID, NUM_DOCS);
         QUERY_COUNT = QUERY_COUNT + NUM_DOCS;
-        validateKNNSearch(testIndex, TEST_FIELD, DIMENSIONS, QUERY_COUNT, K);
         forceMergeKnnIndex(testIndex);
         validateKNNSearch(testIndex, TEST_FIELD, DIMENSIONS, QUERY_COUNT, K);
         deleteKNNIndex(testIndex);

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
@@ -6,6 +6,7 @@
 package org.opensearch.knn.bwc;
 
 import org.opensearch.common.settings.Settings;
+import org.opensearch.knn.index.KNNSettings;
 import org.opensearch.knn.index.SpaceType;
 
 import java.util.Collections;
@@ -34,6 +35,8 @@ public void testKNNWarmupDefaultLegacyFieldMapping() throws Exception {
             createKnnIndex(testIndex, getKNNDefaultIndexSettings(), 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
+            updateIndexSettings(testIndex, Settings.builder().put(KNNSettings.INDEX_KNN_ADVANCED_APPROXIMATE_THRESHOLD, 0));
             validateKNNWarmupOnUpgrade();
         }
     }
@@ -65,6 +68,8 @@ public void testKNNWarmupDefaultMethodFieldMapping() throws Exception {
             createKnnIndex(testIndex, getKNNDefaultIndexSettings(), 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
+            updateIndexSettings(testIndex, Settings.builder().put(KNNSettings.INDEX_KNN_ADVANCED_APPROXIMATE_THRESHOLD, 0));
             validateKNNWarmupOnUpgrade();
         }
     }
@@ -85,21 +90,26 @@ public void testKNNWarmupCustomMethodFieldMapping() throws Exception {
     }
 
     public void validateKNNWarmupOnUpgrade() throws Exception {
+        // update index setting to allow build graph always since we test graph count that are loaded into memory
+        updateIndexSettings(testIndex, Settings.builder().put(KNNSettings.INDEX_KNN_ADVANCED_APPROXIMATE_THRESHOLD, 0));
         int graphCount = getTotalGraphsInCache();
         knnWarmup(Collections.singletonList(testIndex));
-        assertTrue(getTotalGraphsInCache() > graphCount);
+        int totalGraph = getTotalGraphsInCache();
+        assertTrue(totalGraph > graphCount);
 
         QUERY_COUNT = NUM_DOCS;
         validateKNNSearch(testIndex, TEST_FIELD, DIMENSIONS, QUERY_COUNT, K);
 
         DOC_ID = NUM_DOCS;
         addKNNDocs(testIndex, TEST_FIELD, DIMENSIONS, DOC_ID, NUM_DOCS);
+        forceMergeKnnIndex(testIndex);
 
         int updatedGraphCount = getTotalGraphsInCache();
         knnWarmup(Collections.singletonList(testIndex));
         assertTrue(getTotalGraphsInCache() > updatedGraphCount);
 
         QUERY_COUNT = QUERY_COUNT + NUM_DOCS;
+
         validateKNNSearch(testIndex, TEST_FIELD, DIMENSIONS, QUERY_COUNT, K);
         deleteKNNIndex(testIndex);
     }

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
@@ -25,6 +25,9 @@ public void testClearCache() throws Exception {
                 createKnnIndex(testIndex, getKNNDefaultIndexSettings(), createKnnIndexMapping(TEST_FIELD, DIMENSIONS));
                 int docIdOld = 0;
                 addKNNDocs(testIndex, TEST_FIELD, DIMENSIONS, docIdOld, NUM_DOCS);
+                int graphCount = getTotalGraphsInCache();
+                knnWarmup(Collections.singletonList(testIndex));
+                assertTrue(getTotalGraphsInCache() > graphCount);
                 break;
             case UPGRADED:
                 queryCnt = NUM_DOCS;
@@ -42,14 +45,8 @@ public void testClearCache() throws Exception {
 
     // validation steps for Clear Cache API after upgrading all nodes from old version to new version
     public void validateClearCacheOnUpgrade(int queryCount) throws Exception {
-        int graphCount = getTotalGraphsInCache();
-        knnWarmup(Collections.singletonList(testIndex));
-        assertTrue(getTotalGraphsInCache() > graphCount);
-        validateKNNSearch(testIndex, TEST_FIELD, DIMENSIONS, queryCount, K);
-
         clearCache(Collections.singletonList(testIndex));
         assertEquals(0, getTotalGraphsInCache());
-        validateKNNSearch(testIndex, TEST_FIELD, DIMENSIONS, queryCount, K);
     }
 
 }
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
@@ -5,6 +5,9 @@
 
 package org.opensearch.knn.bwc;
 
+import org.opensearch.common.settings.Settings;
+import org.opensearch.knn.index.KNNSettings;
+
 import java.util.Collections;
 
 import static org.opensearch.knn.TestUtils.NODES_BWC_CLUSTER;
@@ -33,6 +36,7 @@ public void testKNNWarmup() throws Exception {
                     docIdMixed = 2 * NUM_DOCS;
                     totalDocsCountMixed = 3 * NUM_DOCS;
                 }
+                updateIndexSettings(testIndex, Settings.builder().put(KNNSettings.INDEX_KNN_ADVANCED_APPROXIMATE_THRESHOLD, 0));
                 validateKNNWarmupOnUpgrade(totalDocsCountMixed, docIdMixed);
                 break;
             case UPGRADED:

diff --git a/release-notes/opensearch-knn.release-notes-2.18.0.0.md b/release-notes/opensearch-knn.release-notes-2.18.0.0.md
@@ -14,6 +14,7 @@ Compatible with OpenSearch 2.18.0
 * Add support to build vector data structures greedily and perform exact search when there are no engine files [#1942](https://github.com/opensearch-project/k-NN/issues/1942)
 * Add CompressionLevel Calculation for PQ [#2200](https://github.com/opensearch-project/k-NN/pull/2200)
 * Remove FSDirectory dependency from native engine constructing side and deprecated FileWatcher [#2182](https://github.com/opensearch-project/k-NN/pull/2182)
+* Update approximate_threshold to 15K documents [#2229](https://github.com/opensearch-project/k-NN/pull/2229)
 ### Bug Fixes
 * Add DocValuesProducers for releasing memory when close index [#1946](https://github.com/opensearch-project/k-NN/pull/1946)
 * KNN80DocValues should only be considered for BinaryDocValues fields [#2147](https://github.com/opensearch-project/k-NN/pull/2147)

diff --git a/src/main/java/org/opensearch/knn/index/KNNSettings.java b/src/main/java/org/opensearch/knn/index/KNNSettings.java
@@ -98,7 +98,7 @@ public class KNNSettings {
     public static final boolean KNN_DEFAULT_FAISS_AVX2_DISABLED_VALUE = false;
     public static final boolean KNN_DEFAULT_FAISS_AVX512_DISABLED_VALUE = false;
     public static final String INDEX_KNN_DEFAULT_SPACE_TYPE = "l2";
-    public static final Integer INDEX_KNN_ADVANCED_APPROXIMATE_THRESHOLD_DEFAULT_VALUE = 0;
+    public static final Integer INDEX_KNN_ADVANCED_APPROXIMATE_THRESHOLD_DEFAULT_VALUE = 15_000;
     public static final Integer INDEX_KNN_BUILD_VECTOR_DATA_STRUCTURE_THRESHOLD_MIN = -1;
     public static final Integer INDEX_KNN_BUILD_VECTOR_DATA_STRUCTURE_THRESHOLD_MAX = Integer.MAX_VALUE - 2;
     public static final String INDEX_KNN_DEFAULT_SPACE_TYPE_FOR_BINARY = "hamming";

diff --git a/...ain/java/org/opensearch/knn/index/codec/KNN990Codec/NativeEngines990KnnVectorsFormat.java b/...ain/java/org/opensearch/knn/index/codec/KNN990Codec/NativeEngines990KnnVectorsFormat.java
@@ -37,6 +37,10 @@ public NativeEngines990KnnVectorsFormat() {
         this(new Lucene99FlatVectorsFormat(new DefaultFlatVectorScorer()));
     }
 
+    public NativeEngines990KnnVectorsFormat(int approximateThreshold) {
+        this(new Lucene99FlatVectorsFormat(new DefaultFlatVectorScorer()), approximateThreshold);
+    }
+
     public NativeEngines990KnnVectorsFormat(final FlatVectorsFormat flatVectorsFormat) {
         this(flatVectorsFormat, KNNSettings.INDEX_KNN_ADVANCED_APPROXIMATE_THRESHOLD_DEFAULT_VALUE);
     }

diff --git a/src/main/java/org/opensearch/knn/index/codec/KNN990Codec/UnitTestCodec.java b/src/main/java/org/opensearch/knn/index/codec/KNN990Codec/UnitTestCodec.java
@@ -21,6 +21,8 @@
  * able to pick this class if its in test folder. Don't use this codec outside of testing
  */
 public class UnitTestCodec extends FilterCodec {
+    private static final Integer BUILD_GRAPH_ALWAYS = 0;
+
     public UnitTestCodec() {
         super("UnitTestCodec", KNNCodecVersion.current().getDefaultKnnCodecSupplier().get());
     }
@@ -30,7 +32,7 @@ public KnnVectorsFormat knnVectorsFormat() {
         return new PerFieldKnnVectorsFormat() {
             @Override
             public KnnVectorsFormat getKnnVectorsFormatForField(String field) {
-                return new NativeEngines990KnnVectorsFormat();
+                return new NativeEngines990KnnVectorsFormat(BUILD_GRAPH_ALWAYS);
             }
         };
     }

diff --git a/src/test/java/org/opensearch/knn/KNNSingleNodeTestCase.java b/src/test/java/org/opensearch/knn/KNNSingleNodeTestCase.java
@@ -5,6 +5,7 @@
 
 package org.opensearch.knn;
 
+import org.opensearch.action.admin.indices.settings.put.UpdateSettingsRequest;
 import org.opensearch.core.action.ActionListener;
 import org.opensearch.core.common.bytes.BytesReference;
 import org.opensearch.cluster.ClusterName;
@@ -14,6 +15,7 @@
 import org.opensearch.cluster.block.ClusterBlocks;
 import org.opensearch.cluster.service.ClusterService;
 import org.opensearch.common.xcontent.XContentHelper;
+import org.opensearch.knn.index.KNNSettings;
 import org.opensearch.knn.index.query.KNNQueryBuilder;
 import org.opensearch.knn.index.memory.NativeMemoryCacheManager;
 import org.opensearch.knn.index.memory.NativeMemoryLoadStrategy;
@@ -104,6 +106,14 @@ protected void createKnnIndexMapping(String indexName, String fieldName, Integer
         OpenSearchAssertions.assertAcked(client().admin().indices().putMapping(request).actionGet());
     }
 
+    /**
+     * Create simple k-NN mapping with engine
+     */
+    protected void updateIndexSetting(String indexName, Settings setting) {
+        UpdateSettingsRequest request = new UpdateSettingsRequest(setting, indexName);
+        OpenSearchAssertions.assertAcked(client().admin().indices().updateSettings(request).actionGet());
+    }
+
     /**
      * Create simple k-NN mapping which can be nested.
      * e.g. fieldPath = "a.b.c" will create mapping for "c" as knn_vector
@@ -140,6 +150,18 @@ 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
+     */
+    protected Settings getKNNDefaultIndexSettingsBuildsGraphAlways() {
+        return Settings.builder()
+            .put("number_of_shards", 1)
+            .put("number_of_replicas", 0)
+            .put("index.knn", true)
+            .put(KNNSettings.INDEX_KNN_ADVANCED_APPROXIMATE_THRESHOLD, 0)
+            .build();
+    }
+
     /**
      * Add a k-NN doc to an index
      */

diff --git a/src/test/java/org/opensearch/knn/index/KNNCircuitBreakerIT.java b/src/test/java/org/opensearch/knn/index/KNNCircuitBreakerIT.java
@@ -20,6 +20,8 @@
  * Integration tests to test Circuit Breaker functionality
  */
 public class KNNCircuitBreakerIT extends KNNRestTestCase {
+    private static final Integer ALWAYS_BUILD_GRAPH = 0;
+
     /**
      * To trip the circuit breaker, we will create two indices and index documents. Each index will be small enough so
      * that individually they fit into the cache, but together they do not. To prevent Lucene conditions where
@@ -39,6 +41,7 @@ private void tripCb() throws Exception {
             .put("number_of_shards", 1)
             .put("number_of_replicas", numNodes - 1)
             .put("index.knn", true)
+            .put(KNNSettings.INDEX_KNN_ADVANCED_APPROXIMATE_THRESHOLD, ALWAYS_BUILD_GRAPH)
             .build();
 
         String indexName1 = INDEX_NAME + "1";

diff --git a/src/test/java/org/opensearch/knn/index/KNNESSettingsTestIT.java b/src/test/java/org/opensearch/knn/index/KNNESSettingsTestIT.java
@@ -21,6 +21,9 @@
 import static org.hamcrest.Matchers.containsString;
 
 public class KNNESSettingsTestIT extends KNNRestTestCase {
+
+    public static final int ALWAYS_BUILD_GRAPH = 0;
+
     /**
      * KNN Index writes should be blocked when the plugin disabled
      * @throws Exception Exception from test

diff --git a/src/test/java/org/opensearch/knn/index/KNNIndexShardTests.java b/src/test/java/org/opensearch/knn/index/KNNIndexShardTests.java
@@ -14,6 +14,7 @@
 import org.apache.lucene.util.StringHelper;
 import org.apache.lucene.util.Version;
 import org.mockito.Mockito;
+import org.opensearch.common.settings.Settings;
 import org.opensearch.knn.KNNSingleNodeTestCase;
 import org.opensearch.index.IndexService;
 import org.opensearch.index.engine.Engine;
@@ -71,6 +72,7 @@ public void testWarmup_emptyIndex() throws IOException {
     public void testWarmup_shardPresentInCache() throws InterruptedException, ExecutionException, IOException {
         IndexService indexService = createKNNIndex(testIndexName);
         createKnnIndexMapping(testIndexName, testFieldName, dimensions);
+        updateIndexSetting(testIndexName, Settings.builder().put(KNNSettings.INDEX_KNN_ADVANCED_APPROXIMATE_THRESHOLD, 0).build());
         addKnnDoc(testIndexName, "1", testFieldName, new Float[] { 2.5F, 3.5F });
 
         searchKNNIndex(testIndexName, testFieldName, new float[] { 1.0f, 2.0f }, 1);
@@ -85,6 +87,7 @@ public void testWarmup_shardPresentInCache() throws InterruptedException, Execut
     public void testWarmup_shardNotPresentInCache() throws InterruptedException, ExecutionException, IOException {
         IndexService indexService = createKNNIndex(testIndexName);
         createKnnIndexMapping(testIndexName, testFieldName, dimensions);
+        updateIndexSetting(testIndexName, Settings.builder().put(KNNSettings.INDEX_KNN_ADVANCED_APPROXIMATE_THRESHOLD, 0).build());
         IndexShard indexShard;
         KNNIndexShard knnIndexShard;
 
@@ -106,6 +109,7 @@ public void testWarmup_shardNotPresentInCache() throws InterruptedException, Exe
     public void testGetAllEngineFileContexts() throws IOException, ExecutionException, InterruptedException {
         IndexService indexService = createKNNIndex(testIndexName);
         createKnnIndexMapping(testIndexName, testFieldName, dimensions);
+        updateIndexSetting(testIndexName, Settings.builder().put(KNNSettings.INDEX_KNN_ADVANCED_APPROXIMATE_THRESHOLD, 0).build());
 
         IndexShard indexShard = indexService.iterator().next();
         KNNIndexShard knnIndexShard = new KNNIndexShard(indexShard);
@@ -204,6 +208,7 @@ public void testClearCache_emptyIndex() {
     public void testClearCache_shardPresentInCache() {
         IndexService indexService = createKNNIndex(testIndexName);
         createKnnIndexMapping(testIndexName, testFieldName, dimensions);
+        updateIndexSetting(testIndexName, Settings.builder().put(KNNSettings.INDEX_KNN_ADVANCED_APPROXIMATE_THRESHOLD, 0).build());
         addKnnDoc(testIndexName, String.valueOf(randomInt()), testFieldName, new Float[] { randomFloat(), randomFloat() });
 
         IndexShard indexShard = indexService.iterator().next();

diff --git a/src/test/java/org/opensearch/knn/index/NmslibIT.java b/src/test/java/org/opensearch/knn/index/NmslibIT.java
@@ -155,7 +155,7 @@ public void testEndToEnd() throws Exception {
         Map<String, Object> mappingMap = xContentBuilderToMap(builder);
         String mapping = builder.toString();
 
-        createKnnIndex(indexName, mapping);
+        createKnnIndex(indexName, buildKNNIndexSettings(0), mapping);
         assertEquals(new TreeMap<>(mappingMap), new TreeMap<>(getIndexMappingAsMap(indexName)));
 
         // Index the test data

diff --git a/src/test/java/org/opensearch/knn/index/OpenSearchIT.java b/src/test/java/org/opensearch/knn/index/OpenSearchIT.java
@@ -113,7 +113,7 @@ public void testEndToEnd() throws Exception {
 
         Map<String, Object> mappingMap = xContentBuilderToMap(builder);
         String mapping = builder.toString();
-        createKnnIndex(indexName, mapping);
+        createKnnIndex(indexName, buildKNNIndexSettings(0), mapping);
         assertEquals(new TreeMap<>(mappingMap), new TreeMap<>(getIndexMappingAsMap(indexName)));
 
         // Index the test data