Fixed a bug to prevent updating index.knn setting after index creation (#2348)

* Change index.knn setting to FINAL, immutable after index creation

Signed-off-by: AnnTian Shao <anntians@amazon.com>

* Add to ChangeLog the description of bug fix

Signed-off-by: AnnTian Shao <anntians@amazon.com>

* Add restart upgrade test for checking immutability of knn.index setting after version upgrade

Signed-off-by: Tommy Shao <anntians@amazon.com>

---------

Signed-off-by: AnnTian Shao <anntians@amazon.com>
Signed-off-by: Tommy Shao <69884021+anntians@users.noreply.github.com>
Signed-off-by: Tommy Shao <anntians@amazon.com>
Co-authored-by: AnnTian Shao <anntians@amazon.com>

Author: anntians

CHANGELOG.md

@@ -29,6 +29,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 * Fixing the bug when a segment has no vector field present for disk based vector search (#2282)[https://github.com/opensearch-project/k-NN/pull/2282]
 * Fix for NPE while merging segments after all the vector fields docs are deleted (#2365)[https://github.com/opensearch-project/k-NN/pull/2365]
 * Allow validation for non knn index only after 2.17.0 (#2315)[https://github.com/opensearch-project/k-NN/pull/2315]
+* Fixing the bug to prevent updating the index.knn setting after index creation(#2348)[https://github.com/opensearch-project/k-NN/pull/2348]
 * Release query vector memory after execution (#2346)[https://github.com/opensearch-project/k-NN/pull/2346]
 * Fix shard level rescoring disabled setting flag (#2352)[https://github.com/opensearch-project/k-NN/pull/2352]
 * Fix filter rewrite logic which was resulting in getting inconsistent / incorrect results for cases where filter was getting rewritten for shards (#2359)[https://github.com/opensearch-project/k-NN/pull/2359]

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

@@ -8,6 +8,7 @@
 import org.apache.hc.core5.http.io.entity.EntityUtils;
 import org.junit.Assert;
 import org.opensearch.client.Response;
+import org.opensearch.client.ResponseException;
 import org.opensearch.common.settings.Settings;
 import org.opensearch.common.xcontent.XContentFactory;
 import org.opensearch.knn.KNNResult;
@@ -20,6 +21,7 @@
 import java.util.List;
 import java.util.Map;
 
+import static org.hamcrest.Matchers.containsString;
 import static org.opensearch.knn.TestUtils.KNN_ALGO_PARAM_EF_CONSTRUCTION_MIN_VALUE;
 import static org.opensearch.knn.TestUtils.KNN_ALGO_PARAM_M_MIN_VALUE;
 import static org.opensearch.knn.TestUtils.KNN_VECTOR;
@@ -132,6 +134,28 @@ public void testKNNIndexLuceneForceMerge() throws Exception {
         }
     }
 
+    public void testKNNIndexSettingImmutableAfterUpgrade() throws Exception {
+        waitForClusterHealthGreen(NODES_BWC_CLUSTER);
+
+        if (isRunningAgainstOldCluster()) {
+            createKnnIndex(testIndex, getKNNDefaultIndexSettings(), createKnnIndexMapping(TEST_FIELD, DIMENSIONS));
+        } else {
+            Exception ex = expectThrows(
+                ResponseException.class,
+                () -> updateIndexSettings(testIndex, Settings.builder().put(KNNSettings.KNN_INDEX, false))
+            );
+            assertThat(ex.getMessage(), containsString("Can't update non dynamic settings [[index.knn]] for open indices"));
+
+            closeIndex(testIndex);
+
+            ex = expectThrows(
+                ResponseException.class,
+                () -> updateIndexSettings(testIndex, Settings.builder().put(KNNSettings.KNN_INDEX, false))
+            );
+            assertThat(ex.getMessage(), containsString(String.format("final %s setting [index.knn], not updateable", testIndex)));
+        }
+    }
+
     public void testKNNIndexLuceneByteVector() throws Exception {
         waitForClusterHealthGreen(NODES_BWC_CLUSTER);
 

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

@@ -43,6 +43,7 @@
 import static org.opensearch.common.settings.Setting.Property.Dynamic;
 import static org.opensearch.common.settings.Setting.Property.IndexScope;
 import static org.opensearch.common.settings.Setting.Property.NodeScope;
+import static org.opensearch.common.settings.Setting.Property.Final;
 import static org.opensearch.common.unit.MemorySizeValue.parseBytesSizeValueOrHeapRatio;
 import static org.opensearch.core.common.unit.ByteSizeValue.parseBytesSizeValue;
 import static org.opensearch.knn.common.featureflags.KNNFeatureFlags.getFeatureFlags;
@@ -268,7 +269,7 @@ public class KNNSettings {
     /**
      * This setting identifies KNN index.
      */
-    public static final Setting<Boolean> IS_KNN_INDEX_SETTING = Setting.boolSetting(KNN_INDEX, false, IndexScope);
+    public static final Setting<Boolean> IS_KNN_INDEX_SETTING = Setting.boolSetting(KNN_INDEX, false, IndexScope, Final);
 
     /**
      * index_thread_quantity - the parameter specifies how many threads the nms library should use to create the graph.

src/test/java/org/opensearch/knn/index/KNNESSettingsTestIT.java

@@ -124,6 +124,26 @@ public void testUpdateIndexSetting() throws IOException {
         assertThat(ex.getMessage(), containsString("Failed to parse value [1] for setting [index.knn.algo_param.ef_search] must be >= 2"));
     }
 
+    public void testUpdateIndexSettingKnnFlagImmutable() throws IOException {
+        Settings settings = Settings.builder().put(KNNSettings.KNN_INDEX, true).build();
+        createKnnIndex(INDEX_NAME, settings, createKnnIndexMapping(FIELD_NAME, 2));
+
+        Exception ex = expectThrows(
+            ResponseException.class,
+            () -> updateIndexSettings(INDEX_NAME, Settings.builder().put(KNNSettings.KNN_INDEX, false))
+        );
+        assertThat(ex.getMessage(), containsString("Can't update non dynamic settings [[index.knn]] for open indices"));
+
+        closeIndex(INDEX_NAME);
+
+        ex = expectThrows(
+            ResponseException.class,
+            () -> updateIndexSettings(INDEX_NAME, Settings.builder().put(KNNSettings.KNN_INDEX, false))
+        );
+        assertThat(ex.getMessage(), containsString(String.format("final %s setting [index.knn], not updateable", INDEX_NAME)));
+
+    }
+
     @SuppressWarnings("unchecked")
     public void testCacheRebuiltAfterUpdateIndexSettings() throws Exception {
         createKnnIndex(INDEX_NAME, getKNNDefaultIndexSettings(), createKnnIndexMapping(FIELD_NAME, 2));