Skip to content

Commit

Permalink
Fixed a bug to prevent updating index.knn setting after index creation (
Browse files Browse the repository at this point in the history 
opensearch-project#2348)

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

Signed-off-by: AnnTian Shao <[email protected]>

* Add to ChangeLog the description of bug fix

Signed-off-by: AnnTian Shao <[email protected]>

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

Signed-off-by: Tommy Shao <[email protected]>

---------

Signed-off-by: AnnTian Shao <[email protected]>
Signed-off-by: Tommy Shao <[email protected]>
Signed-off-by: Tommy Shao <[email protected]>
Co-authored-by: AnnTian Shao <[email protected]>
(cherry picked from commit a875eb8)
  • Loading branch information
@anntians
anntians authored and AnnTian Shao committed Jan 10, 2025
1 parent cc72442 commit 0c0332e
Show file tree
Hide file tree
Showing 4 changed files with 132 additions and 1 deletion.
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,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]
Expand Down
109 changes: 109 additions & 0 deletions qa/restart-upgrade/src/test/java/org/opensearch/knn/bwc/IndexingIT.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,17 +5,23 @@

package org.opensearch.knn.bwc;

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;
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 org.opensearch.knn.index.query.KNNQueryBuilder;

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;
Expand All @@ -32,6 +38,8 @@
import static org.opensearch.knn.common.KNNConstants.METHOD_PARAMETER_EF_SEARCH;
import static org.opensearch.knn.common.KNNConstants.METHOD_PARAMETER_M;
import static org.opensearch.knn.common.KNNConstants.METHOD_PARAMETER_SPACE_TYPE;
import static org.opensearch.knn.common.KNNConstants.METHOD_ENCODER_PARAMETER;
import static org.opensearch.knn.common.KNNConstants.ENCODER_SQ;
import static org.opensearch.knn.common.KNNConstants.NAME;
import static org.opensearch.knn.common.KNNConstants.PARAMETERS;

Expand Down Expand Up @@ -126,6 +134,107 @@ 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);

if (isRunningAgainstOldCluster()) {
createKnnIndex(
testIndex,
getKNNDefaultIndexSettings(),
createKnnIndexMapping(TEST_FIELD, DIMENSIONS, METHOD_HNSW, LUCENE_NAME, SpaceType.L2.getValue(), true, VectorDataType.BYTE)
);
addKNNByteDocs(testIndex, TEST_FIELD, DIMENSIONS, DOC_ID, 50);
// Flush to ensure that index is not re-indexed when node comes back up
flush(testIndex, true);
validateKNNSearch(testIndex, TEST_FIELD, DIMENSIONS, 50, 5);
} else {
validateKNNSearch(testIndex, TEST_FIELD, DIMENSIONS, 50, 5);
addKNNDocs(testIndex, TEST_FIELD, DIMENSIONS, 50, 25);
validateKNNSearch(testIndex, TEST_FIELD, DIMENSIONS, 75, 5);
deleteKNNIndex(testIndex);
}
}

public void testKNNIndexLuceneQuantization() throws Exception {
waitForClusterHealthGreen(NODES_BWC_CLUSTER);
int k = 4;
int dimension = 2;

if (isRunningAgainstOldCluster()) {
String mapping = XContentFactory.jsonBuilder()
.startObject()
.startObject("properties")
.startObject(TEST_FIELD)
.field(VECTOR_TYPE, KNN_VECTOR)
.field(DIMENSION, dimension)
.startObject(KNN_METHOD)
.field(NAME, METHOD_HNSW)
.field(METHOD_PARAMETER_SPACE_TYPE, SpaceType.INNER_PRODUCT.getValue())
.field(KNN_ENGINE, LUCENE_NAME)
.startObject(PARAMETERS)
.startObject(METHOD_ENCODER_PARAMETER)
.field(NAME, ENCODER_SQ)
.endObject()
.field(METHOD_PARAMETER_EF_CONSTRUCTION, 256)
.field(METHOD_PARAMETER_M, 16)
.endObject()
.endObject()
.endObject()
.endObject()
.endObject()
.toString();
createKnnIndex(testIndex, getKNNDefaultIndexSettings(), mapping);

Float[] vector1 = { -10.6f, 25.48f };
Float[] vector2 = { -10.8f, 25.48f };
Float[] vector3 = { -11.0f, 25.48f };
Float[] vector4 = { -11.2f, 25.48f };
addKnnDoc(testIndex, "1", TEST_FIELD, vector1);
addKnnDoc(testIndex, "2", TEST_FIELD, vector2);
addKnnDoc(testIndex, "3", TEST_FIELD, vector3);
addKnnDoc(testIndex, "4", TEST_FIELD, vector4);

float[] queryVector = { -10.5f, 25.48f };
Response searchResponse = searchKNNIndex(testIndex, new KNNQueryBuilder(TEST_FIELD, queryVector, k), k);
List<KNNResult> results = parseSearchResponse(EntityUtils.toString(searchResponse.getEntity()), TEST_FIELD);
assertEquals(k, results.size());
for (int i = 0; i < k; i++) {
assertEquals(k - i, Integer.parseInt(results.get(i).getDocId()));
}
} else {
float[] queryVector = { -10.5f, 25.48f };
Response searchResponse = searchKNNIndex(testIndex, new KNNQueryBuilder(TEST_FIELD, queryVector, k), k);
List<KNNResult> results = parseSearchResponse(EntityUtils.toString(searchResponse.getEntity()), TEST_FIELD);
assertEquals(k, results.size());
for (int i = 0; i < k; i++) {
assertEquals(k - i, Integer.parseInt(results.get(i).getDocId()));
}
deleteKNNIndex(testIndex);
}
}

// Ensure bwc works for binary force merge
public void testKNNIndexBinaryForceMerge() throws Exception {
int dimension = 40;
Expand Down
3 changes: 2 additions & 1 deletion src/main/java/org/opensearch/knn/index/KNNSettings.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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.
Expand Down
20 changes: 20 additions & 0 deletions src/test/java/org/opensearch/knn/index/KNNESSettingsTestIT.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down

0 comments on commit 0c0332e

Please sign in to comment.