Changed the default values for ef_search and ef_constrction for enabl…

…ing better indexing and search latency for vector search

Signed-off-by: Navneet Verma <[email protected]>

qa/restart-upgrade/src/test/java/org/opensearch/knn/bwc/IndexSettingIT.java

@@ -0,0 +1,34 @@
+/*
+ * SPDX-License-Identifier: Apache-2.0
+ *
+ * The OpenSearch Contributors require contributions made to
+ * this file be licensed under the Apache-2.0 license or a
+ * compatible open source license.
+ *
+ * Modifications Copyright OpenSearch Contributors. See
+ * GitHub history for details.
+ */
+
+package org.opensearch.knn.bwc;
+
+import org.junit.Assert;
+import org.opensearch.knn.index.KNNSettings;
+
+public class IndexSettingIT extends AbstractRestartUpgradeTestCase {
+    private static final String TEST_FIELD = "test-field";
+    private static final int DIMENSIONS = 5;
+
+    public void testOldIndexSettingsPersistedAfterUpgrade() throws Exception {
+        if(isRunningAgainstOldCluster()) {
+            // create index with Old Values
+            createKnnIndex(testIndex, getKNNDefaultIndexSettings(), createKnnIndexMapping(TEST_FIELD, DIMENSIONS));
+            int old_ef_search = Integer.parseInt(getIndexSettingByName(testIndex,
+                    KNNSettings.KNN_ALGO_PARAM_EF_SEARCH));
+            Assert.assertEquals(KNNSettings.INDEX_KNN_DEFAULT_ALGO_PARAM_EF_SEARCH.intValue(), old_ef_search);
+        } else {
+            int old_ef_search = Integer.parseInt(getIndexSettingByName(testIndex,
+                    KNNSettings.KNN_ALGO_PARAM_EF_SEARCH));
+            Assert.assertEquals(KNNSettings.INDEX_KNN_DEFAULT_ALGO_PARAM_EF_SEARCH.intValue(), old_ef_search);
+        }
+    }
+}

src/main/java/org/opensearch/knn/index/KNNIndexSettingProvider.java

@@ -0,0 +1,47 @@
+/*
+ * SPDX-License-Identifier: Apache-2.0
+ *
+ * The OpenSearch Contributors require contributions made to
+ * this file be licensed under the Apache-2.0 license or a
+ * compatible open source license.
+ *
+ * Modifications Copyright OpenSearch Contributors. See
+ * GitHub history for details.
+ */
+
+package org.opensearch.knn.index;
+
+import lombok.NoArgsConstructor;
+import org.opensearch.common.settings.Settings;
+import org.opensearch.index.shard.IndexSettingProvider;
+
+@NoArgsConstructor
+public class KNNIndexSettingProvider implements IndexSettingProvider {
+
+    private static final Settings EF_SEARCH_SETTING = Settings.builder().put(KNNSettings.KNN_ALGO_PARAM_EF_SEARCH,
+            KNNSettings.INDEX_KNN_NEW_DEFAULT_ALGO_PARAM_EF_SEARCH).build();
+
+    /**
+     * Returns explicitly set default index {@link Settings} for the given index. This should not
+     * return null.
+     *
+     * @param indexName {@link String} name of the index
+     * @param isDataStreamIndex boolean: index is a datastream index or not
+     * @param templateAndRequestSettings {@link Settings}
+     */
+    @Override
+    public Settings getAdditionalIndexSettings(String indexName, boolean isDataStreamIndex, Settings templateAndRequestSettings) {
+        if(isKNNIndex(templateAndRequestSettings) && isEfSearchNotSetByUser(templateAndRequestSettings)) {
+            return EF_SEARCH_SETTING;
+        }
+        return IndexSettingProvider.super.getAdditionalIndexSettings(indexName, isDataStreamIndex, templateAndRequestSettings);
+    }
+
+    private boolean isKNNIndex(Settings templateAndRequestSettings) {
+        return templateAndRequestSettings.getAsBoolean(KNNSettings.KNN_INDEX, false);
+    }
+
+    private boolean isEfSearchNotSetByUser(Settings templateAndRequestSettings) {
+        return templateAndRequestSettings.hasValue(KNNSettings.KNN_ALGO_PARAM_EF_SEARCH) == false;
+    }
+}

src/main/java/org/opensearch/knn/index/KNNSettings.java

@@ -81,7 +81,8 @@ public class KNNSettings {
     public static final String INDEX_KNN_DEFAULT_SPACE_TYPE = "l2";
     public static final Integer INDEX_KNN_DEFAULT_ALGO_PARAM_M = 16;
     public static final Integer INDEX_KNN_DEFAULT_ALGO_PARAM_EF_SEARCH = 512;
-    public static final Integer INDEX_KNN_DEFAULT_ALGO_PARAM_EF_CONSTRUCTION = 512;
+    public static final Integer INDEX_KNN_NEW_DEFAULT_ALGO_PARAM_EF_SEARCH = 100;
+    public static final Integer INDEX_KNN_DEFAULT_ALGO_PARAM_EF_CONSTRUCTION = 100;
     public static final Integer KNN_DEFAULT_ALGO_PARAM_INDEX_THREAD_QTY = 1;
     public static final Integer KNN_DEFAULT_CIRCUIT_BREAKER_UNSET_PERCENTAGE = 75;
     public static final Integer KNN_DEFAULT_MODEL_CACHE_SIZE_LIMIT_PERCENTAGE = 10; // By default, set aside 10% of the JVM for the limit
@@ -451,7 +452,7 @@ public static int getEfSearchParam(String index) {
             .getMetadata()
             .index(index)
             .getSettings()
-            .getAsInt(KNNSettings.KNN_ALGO_PARAM_EF_SEARCH, 512);
+            .getAsInt(KNNSettings.KNN_ALGO_PARAM_EF_SEARCH, INDEX_KNN_DEFAULT_ALGO_PARAM_EF_SEARCH);
     }
 
     public void setClusterService(ClusterService clusterService) {

src/main/java/org/opensearch/knn/jni/JNIService.java

@@ -11,6 +11,7 @@
 
 package org.opensearch.knn.jni;
 
+import lombok.extern.log4j.Log4j2;
 import org.apache.commons.lang.ArrayUtils;
 import org.opensearch.knn.index.query.KNNQueryResult;
 import org.opensearch.knn.index.util.KNNEngine;
@@ -20,6 +21,7 @@
 /**
  * Service to distribute requests to the proper engine jni service
  */
+@Log4j2
 public class JNIService {
 
     /**
@@ -32,6 +34,7 @@ public class JNIService {
      * @param engineName name of engine to build index for
      */
     public static void createIndex(int[] ids, float[][] data, String indexPath, Map<String, Object> parameters, String engineName) {
+        log.error("index path {}, has the parameters : {}, engine: {}", indexPath, parameters, engineName);
         if (KNNEngine.NMSLIB.getName().equals(engineName)) {
             NmslibService.createIndex(ids, data, indexPath, parameters);
             return;

src/main/java/org/opensearch/knn/plugin/KNNPlugin.java

@@ -11,9 +11,11 @@
 import org.opensearch.core.action.ActionResponse;
 import org.opensearch.index.codec.CodecServiceFactory;
 import org.opensearch.index.engine.EngineFactory;
+import org.opensearch.index.shard.IndexSettingProvider;
 import org.opensearch.indices.SystemIndexDescriptor;
 import org.opensearch.knn.index.KNNCircuitBreaker;
 import org.opensearch.knn.index.KNNClusterUtil;
+import org.opensearch.knn.index.KNNIndexSettingProvider;
 import org.opensearch.knn.index.query.KNNQueryBuilder;
 import org.opensearch.knn.index.KNNSettings;
 import org.opensearch.knn.index.mapper.KNNVectorFieldMapper;
@@ -361,4 +363,14 @@ public Settings additionalSettings() {
             .collect(Collectors.toList());
         return Settings.builder().putList(IndexModule.INDEX_STORE_HYBRID_NIO_EXTENSIONS.getKey(), finalSettings).build();
     }
+
+    /**
+     * An {@link IndexSettingProvider} allows hooking in to parts of an index
+     * lifecycle to provide explicit default settings for newly created indices. Rather than changing
+     * the default values for an index-level setting, these act as though the setting has been set
+     * explicitly, but still allow the setting to be overridden by a template or creation request body.
+     */
+    public Collection<IndexSettingProvider> getAdditionalIndexSettingProviders() {
+        return List.of(new KNNIndexSettingProvider());
+    }
 }