Skip to content

Commit dd403a1

Browse files
anntiansAnnTian Shao
anntians
and
AnnTian Shao
committed
Add UnmodifiableOnRestore property to index.knn setting (opensearch-project#2445)
* Add UnmodifiableOnRestore property to index.knn setting Signed-off-by: AnnTian Shao <[email protected]> * Update tests with helper methods Signed-off-by: AnnTian Shao <[email protected]> * Update test names Signed-off-by: AnnTian Shao <[email protected]> * Add to changeLog and fixes to tests Signed-off-by: AnnTian Shao <[email protected]> --------- Signed-off-by: AnnTian Shao <[email protected]> Co-authored-by: AnnTian Shao <[email protected]> (cherry picked from commit 1d2c4c0)
1 parent c7ac05c commit dd403a1

File tree

4 files changed

+155
-1
lines changed

4 files changed

+155
-1
lines changed

‎CHANGELOG.md

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,18 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
2222
### Features
2323
### Enhancements
2424
### Bug Fixes
25+
* 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]
26+
* Fixing the bug where search fails with "fields" parameter for an index with a knn_vector field (#2314)[https://github.com/opensearch-project/k-NN/pull/2314]
27+
* Fix for NPE while merging segments after all the vector fields docs are deleted (#2365)[https://github.com/opensearch-project/k-NN/pull/2365]
28+
* Allow validation for non knn index only after 2.17.0 (#2315)[https://github.com/opensearch-project/k-NN/pull/2315]
29+
* Fixing the bug to prevent updating the index.knn setting after index creation(#2348)[https://github.com/opensearch-project/k-NN/pull/2348]
30+
* Release query vector memory after execution (#2346)[https://github.com/opensearch-project/k-NN/pull/2346]
31+
* Fix shard level rescoring disabled setting flag (#2352)[https://github.com/opensearch-project/k-NN/pull/2352]
32+
* 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]
33+
* Fixing it to retrieve space_type from index setting when both method and top level don't have the value. [#2374](https://github.com/opensearch-project/k-NN/pull/2374)
34+
* Fixing the bug where setting rescore as false for on_disk knn_vector query is a no-op (#2399)[https://github.com/opensearch-project/k-NN/pull/2399]
35+
* Fixing bug where mapping accepts both dimension and model-id (#2410)[https://github.com/opensearch-project/k-NN/pull/2410]
36+
* Fixing the bug to prevent index.knn setting from being modified or removed on restore snapshot (#2445)[https://github.com/opensearch-project/k-NN/pull/2445]
2537
* Fix derived source for binary and byte vectors [#2533](https://github.com/opensearch-project/k-NN/pull/2533/)
2638
* Fix the put mapping issue for already created index with flat mapper [#2542](https://github.com/opensearch-project/k-NN/pull/2542)
2739
### Infrastructure

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

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -283,7 +283,13 @@ public class KNNSettings {
283283
/**
284284
* This setting identifies KNN index.
285285
*/
286-
public static final Setting<Boolean> IS_KNN_INDEX_SETTING = Setting.boolSetting(KNN_INDEX, false, IndexScope, Final);
286+
public static final Setting<Boolean> IS_KNN_INDEX_SETTING = Setting.boolSetting(
287+
KNN_INDEX,
288+
false,
289+
IndexScope,
290+
Final,
291+
UnmodifiableOnRestore
292+
);
287293

288294
/**
289295
* 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/RestoreSnapshotIT.java

Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,108 @@
1+
/*
2+
* Copyright OpenSearch Contributors
3+
* SPDX-License-Identifier: Apache-2.0
4+
*/
5+
6+
package org.opensearch.knn.index;
7+
8+
import org.opensearch.client.Request;
9+
import org.opensearch.client.Response;
10+
import org.opensearch.client.ResponseException;
11+
import org.opensearch.common.xcontent.json.JsonXContent;
12+
import org.opensearch.core.xcontent.XContentBuilder;
13+
import org.opensearch.knn.KNNRestTestCase;
14+
import org.junit.Before;
15+
import org.junit.Test;
16+
import lombok.SneakyThrows;
17+
import static org.hamcrest.Matchers.containsString;
18+
19+
public class RestoreSnapshotIT extends KNNRestTestCase {
20+
21+
private String index = "test-index";;
22+
private String snapshot = "snapshot-" + index;
23+
private String repository = "repo";
24+
25+
@Before
26+
@SneakyThrows
27+
public void setUp() {
28+
super.setUp();
29+
setupSnapshotRestore(index, snapshot, repository);
30+
}
31+
32+
@Test
33+
@SneakyThrows
34+
public void testKnnSettingIsModifiable_whenRestore_thenSuccess() {
35+
// valid restore
36+
XContentBuilder restoreCommand = JsonXContent.contentBuilder().startObject();
37+
restoreCommand.field("indices", index);
38+
restoreCommand.field("rename_pattern", index);
39+
restoreCommand.field("rename_replacement", "restored-" + index);
40+
restoreCommand.startObject("index_settings");
41+
{
42+
restoreCommand.field("knn.model.index.number_of_shards", 1);
43+
}
44+
restoreCommand.endObject();
45+
restoreCommand.endObject();
46+
Request restoreRequest = new Request("POST", "/_snapshot/" + repository + "/" + snapshot + "/_restore");
47+
restoreRequest.addParameter("wait_for_completion", "true");
48+
restoreRequest.setJsonEntity(restoreCommand.toString());
49+
50+
final Response restoreResponse = client().performRequest(restoreRequest);
51+
assertEquals(200, restoreResponse.getStatusLine().getStatusCode());
52+
}
53+
54+
@Test
55+
@SneakyThrows
56+
public void testKnnSettingIsUnmodifiable_whenRestore_thenFailure() {
57+
// invalid restore
58+
XContentBuilder restoreCommand = JsonXContent.contentBuilder().startObject();
59+
restoreCommand.field("indices", index);
60+
restoreCommand.field("rename_pattern", index);
61+
restoreCommand.field("rename_replacement", "restored-" + index);
62+
restoreCommand.startObject("index_settings");
63+
{
64+
restoreCommand.field("index.knn", false);
65+
}
66+
restoreCommand.endObject();
67+
restoreCommand.endObject();
68+
Request restoreRequest = new Request("POST", "/_snapshot/" + repository + "/" + snapshot + "/_restore");
69+
restoreRequest.addParameter("wait_for_completion", "true");
70+
restoreRequest.setJsonEntity(restoreCommand.toString());
71+
final ResponseException error = expectThrows(ResponseException.class, () -> client().performRequest(restoreRequest));
72+
assertThat(error.getMessage(), containsString("cannot modify UnmodifiableOnRestore setting [index.knn]" + " on restore"));
73+
}
74+
75+
@Test
76+
@SneakyThrows
77+
public void testKnnSettingCanBeIgnored_whenRestore_thenSuccess() {
78+
// valid restore
79+
XContentBuilder restoreCommand = JsonXContent.contentBuilder().startObject();
80+
restoreCommand.field("indices", index);
81+
restoreCommand.field("rename_pattern", index);
82+
restoreCommand.field("rename_replacement", "restored-" + index);
83+
restoreCommand.field("ignore_index_settings", "knn.model.index.number_of_shards");
84+
restoreCommand.endObject();
85+
Request restoreRequest = new Request("POST", "/_snapshot/" + repository + "/" + snapshot + "/_restore");
86+
restoreRequest.addParameter("wait_for_completion", "true");
87+
restoreRequest.setJsonEntity(restoreCommand.toString());
88+
final Response restoreResponse = client().performRequest(restoreRequest);
89+
assertEquals(200, restoreResponse.getStatusLine().getStatusCode());
90+
}
91+
92+
@Test
93+
@SneakyThrows
94+
public void testKnnSettingCannotBeIgnored_whenRestore_thenFailure() {
95+
// invalid restore
96+
XContentBuilder restoreCommand = JsonXContent.contentBuilder().startObject();
97+
restoreCommand.field("indices", index);
98+
restoreCommand.field("rename_pattern", index);
99+
restoreCommand.field("rename_replacement", "restored-" + index);
100+
restoreCommand.field("ignore_index_settings", "index.knn");
101+
restoreCommand.endObject();
102+
Request restoreRequest = new Request("POST", "/_snapshot/" + repository + "/" + snapshot + "/_restore");
103+
restoreRequest.addParameter("wait_for_completion", "true");
104+
restoreRequest.setJsonEntity(restoreCommand.toString());
105+
final ResponseException error = expectThrows(ResponseException.class, () -> client().performRequest(restoreRequest));
106+
assertThat(error.getMessage(), containsString("cannot remove UnmodifiableOnRestore setting [index.knn] on restore"));
107+
}
108+
}

‎src/testFixtures/java/org/opensearch/knn/KNNRestTestCase.java

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
import org.junit.AfterClass;
2020
import org.junit.Before;
2121
import org.opensearch.Version;
22+
import org.opensearch.common.xcontent.support.XContentMapValues;
2223
import org.opensearch.client.Request;
2324
import org.opensearch.client.Response;
2425
import org.opensearch.common.settings.Settings;
@@ -86,6 +87,8 @@
8687
import static org.opensearch.knn.TestUtils.VECTOR_TYPE;
8788
import static org.opensearch.knn.TestUtils.computeGroundTruthValues;
8889
import static org.opensearch.knn.common.KNNConstants.CLEAR_CACHE;
90+
import static org.hamcrest.Matchers.hasSize;
91+
import static org.hamcrest.Matchers.notNullValue;
8992
import static org.opensearch.knn.common.KNNConstants.DIMENSION;
9093
import static org.opensearch.knn.common.KNNConstants.ENCODER_PARAMETER_PQ_CODE_SIZE;
9194
import static org.opensearch.knn.common.KNNConstants.ENCODER_PARAMETER_PQ_M;
@@ -2370,4 +2373,29 @@ protected boolean isRemoteIndexBuildSupported(final Optional<String> bwcVersion)
23702373
protected static String randomLowerCaseString() {
23712374
return randomAlphaOfLengthBetween(MIN_CODE_UNITS, MAX_CODE_UNITS).toLowerCase(Locale.ROOT);
23722375
}
2376+
2377+
@SneakyThrows
2378+
protected void setupSnapshotRestore(String index, String snapshot, String repository) {
2379+
Request clusterSettingsRequest = new Request("GET", "/_cluster/settings");
2380+
clusterSettingsRequest.addParameter("include_defaults", "true");
2381+
Response clusterSettingsResponse = client().performRequest(clusterSettingsRequest);
2382+
Map<String, Object> clusterSettings = entityAsMap(clusterSettingsResponse);
2383+
2384+
@SuppressWarnings("unchecked")
2385+
List<String> pathRepos = (List<String>) XContentMapValues.extractValue("defaults.path.repo", clusterSettings);
2386+
assertThat(pathRepos, notNullValue());
2387+
assertThat(pathRepos, hasSize(1));
2388+
2389+
final String pathRepo = pathRepos.get(0);
2390+
2391+
// create index
2392+
createIndex(index, getDefaultIndexSettings());
2393+
2394+
// create repo
2395+
Settings repoSettings = Settings.builder().put("compress", randomBoolean()).put("location", pathRepo).build();
2396+
registerRepository(repository, "fs", true, repoSettings);
2397+
2398+
// create snapshot
2399+
createSnapshot(repository, snapshot, true);
2400+
}
23732401
}

0 commit comments

Comments
 (0)