Skip to content

Commit c882d97

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 d142366 commit c882d97

File tree

4 files changed

+145
-1
lines changed

4 files changed

+145
-1
lines changed

‎CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
4343
* 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)
4444
* 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]
4545
* Fixing bug where mapping accepts both dimension and model-id (#2410)[https://github.com/opensearch-project/k-NN/pull/2410]
46+
* 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]
4647
### Infrastructure
4748
* Updated C++ version in JNI from c++11 to c++17 [#2259](https://github.com/opensearch-project/k-NN/pull/2259)
4849
* Upgrade bytebuddy and objenesis version to match OpenSearch core and, update github ci runner for macos [#2279](https://github.com/opensearch-project/k-NN/pull/2279)

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

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@
4444
import static org.opensearch.common.settings.Setting.Property.IndexScope;
4545
import static org.opensearch.common.settings.Setting.Property.NodeScope;
4646
import static org.opensearch.common.settings.Setting.Property.Final;
47+
import static org.opensearch.common.settings.Setting.Property.UnmodifiableOnRestore;
4748
import static org.opensearch.common.unit.MemorySizeValue.parseBytesSizeValueOrHeapRatio;
4849
import static org.opensearch.core.common.unit.ByteSizeValue.parseBytesSizeValue;
4950
import static org.opensearch.knn.common.featureflags.KNNFeatureFlags.getFeatureFlags;
@@ -271,7 +272,13 @@ public class KNNSettings {
271272
/**
272273
* This setting identifies KNN index.
273274
*/
274-
public static final Setting<Boolean> IS_KNN_INDEX_SETTING = Setting.boolSetting(KNN_INDEX, false, IndexScope, Final);
275+
public static final Setting<Boolean> IS_KNN_INDEX_SETTING = Setting.boolSetting(
276+
KNN_INDEX,
277+
false,
278+
IndexScope,
279+
Final,
280+
UnmodifiableOnRestore
281+
);
275282

276283
/**
277284
* 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
@@ -17,6 +17,7 @@
1717
import org.apache.hc.core5.http.io.entity.EntityUtils;
1818
import org.apache.hc.core5.net.URIBuilder;
1919
import org.opensearch.Version;
20+
import org.opensearch.common.xcontent.support.XContentMapValues;
2021
import org.opensearch.core.common.bytes.BytesReference;
2122
import org.opensearch.common.xcontent.XContentHelper;
2223
import org.opensearch.core.xcontent.DeprecationHandler;
@@ -73,6 +74,8 @@
7374
import java.util.concurrent.TimeUnit;
7475
import java.util.stream.Collectors;
7576

77+
import static org.hamcrest.Matchers.hasSize;
78+
import static org.hamcrest.Matchers.notNullValue;
7679
import static org.opensearch.knn.common.KNNConstants.DIMENSION;
7780
import static org.opensearch.knn.common.KNNConstants.ENCODER_PARAMETER_PQ_CODE_SIZE;
7881
import static org.opensearch.knn.common.KNNConstants.ENCODER_PARAMETER_PQ_M;
@@ -1980,4 +1983,29 @@ protected boolean isApproximateThresholdSupported(final Optional<String> bwcVers
19801983
protected static String randomLowerCaseString() {
19811984
return randomAlphaOfLengthBetween(MIN_CODE_UNITS, MAX_CODE_UNITS).toLowerCase(Locale.ROOT);
19821985
}
1986+
1987+
@SneakyThrows
1988+
protected void setupSnapshotRestore(String index, String snapshot, String repository) {
1989+
Request clusterSettingsRequest = new Request("GET", "/_cluster/settings");
1990+
clusterSettingsRequest.addParameter("include_defaults", "true");
1991+
Response clusterSettingsResponse = client().performRequest(clusterSettingsRequest);
1992+
Map<String, Object> clusterSettings = entityAsMap(clusterSettingsResponse);
1993+
1994+
@SuppressWarnings("unchecked")
1995+
List<String> pathRepos = (List<String>) XContentMapValues.extractValue("defaults.path.repo", clusterSettings);
1996+
assertThat(pathRepos, notNullValue());
1997+
assertThat(pathRepos, hasSize(1));
1998+
1999+
final String pathRepo = pathRepos.get(0);
2000+
2001+
// create index
2002+
createIndex(index, getDefaultIndexSettings());
2003+
2004+
// create repo
2005+
Settings repoSettings = Settings.builder().put("compress", randomBoolean()).put("location", pathRepo).build();
2006+
registerRepository(repository, "fs", true, repoSettings);
2007+
2008+
// create snapshot
2009+
createSnapshot(repository, snapshot, true);
2010+
}
19832011
}

0 commit comments

Comments
 (0)