Skip to content

Commit d5a4355

Browse files
author
AnnTian Shao
committed
Add to changeLog and fixes to tests
Signed-off-by: AnnTian Shao <[email protected]>
1 parent 7e5f720 commit d5a4355

File tree

3 files changed

+84
-43
lines changed

3 files changed

+84
-43
lines changed

‎CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
4040
* 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]
4141
* 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)
4242
* 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]
43+
* 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]
4344
### Infrastructure
4445
* Updated C++ version in JNI from c++11 to c++17 [#2259](https://github.com/opensearch-project/k-NN/pull/2259)
4546
* 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/test/java/org/opensearch/knn/index/RestoreSnapshotIT.java

Lines changed: 55 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -8,57 +8,52 @@
88
import org.opensearch.client.Request;
99
import org.opensearch.client.Response;
1010
import org.opensearch.client.ResponseException;
11-
import org.opensearch.common.settings.Settings;
1211
import org.opensearch.common.xcontent.json.JsonXContent;
13-
import org.opensearch.common.xcontent.support.XContentMapValues;
1412
import org.opensearch.core.xcontent.XContentBuilder;
15-
import org.opensearch.test.rest.OpenSearchRestTestCase;
16-
17-
import java.util.List;
18-
import java.util.Map;
19-
13+
import org.opensearch.knn.KNNRestTestCase;
14+
import org.junit.Before;
15+
import org.junit.Test;
16+
import lombok.SneakyThrows;
2017
import static org.hamcrest.Matchers.containsString;
21-
import static org.hamcrest.Matchers.notNullValue;
22-
import static org.hamcrest.Matchers.hasSize;
23-
24-
public class RestoreSnapshotIT extends OpenSearchRestTestCase {
25-
26-
private String index;
27-
private String snapshot;
28-
private String repository;
2918

30-
private void setupSnapshotRestore() throws Exception {
31-
Request clusterSettingsRequest = new Request("GET", "/_cluster/settings");
32-
clusterSettingsRequest.addParameter("include_defaults", "true");
33-
Response clusterSettingsResponse = client().performRequest(clusterSettingsRequest);
34-
Map<String, Object> clusterSettings = entityAsMap(clusterSettingsResponse);
19+
public class RestoreSnapshotIT extends KNNRestTestCase {
3520

36-
@SuppressWarnings("unchecked")
37-
List<String> pathRepos = (List<String>) XContentMapValues.extractValue("defaults.path.repo", clusterSettings);
38-
assertThat(pathRepos, notNullValue());
39-
assertThat(pathRepos, hasSize(1));
21+
private String index = "test-index";;
22+
private String snapshot = "snapshot-" + index;
23+
private String repository = "repo";
4024

41-
final String pathRepo = pathRepos.get(0);
42-
43-
index = "test-index";
44-
snapshot = "snapshot-" + index;
45-
repository = "repo";
46-
47-
// create index
48-
Settings indexSettings = Settings.builder().put("number_of_shards", 1).put("number_of_replicas", 1).put("index.knn", true).build();
49-
createIndex(index, indexSettings);
25+
@Before
26+
@SneakyThrows
27+
public void setUp() {
28+
super.setUp();
29+
setupSnapshotRestore(index, snapshot, repository);
30+
}
5031

51-
// create repo
52-
Settings repoSettings = Settings.builder().put("compress", randomBoolean()).put("location", pathRepo).build();
53-
registerRepository(repository, "fs", true, repoSettings);
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());
5449

55-
// create snapshot
56-
createSnapshot(repository, snapshot, true);
50+
final Response restoreResponse = client().performRequest(restoreRequest);
51+
assertEquals(200, restoreResponse.getStatusLine().getStatusCode());
5752
}
5853

59-
public void testKnnSettingIsUnmodifiableOnRestore() throws Exception {
60-
setupSnapshotRestore();
61-
54+
@Test
55+
@SneakyThrows
56+
public void testKnnSettingIsUnmodifiable_whenRestore_thenFailure() {
6257
// invalid restore
6358
XContentBuilder restoreCommand = JsonXContent.contentBuilder().startObject();
6459
restoreCommand.field("indices", index);
@@ -77,9 +72,26 @@ public void testKnnSettingIsUnmodifiableOnRestore() throws Exception {
7772
assertThat(error.getMessage(), containsString("cannot modify UnmodifiableOnRestore setting [index.knn]" + " on restore"));
7873
}
7974

80-
public void testKnnSettingCannotBeIgnoredDuringRestore() throws Exception {
81-
setupSnapshotRestore();
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+
}
8291

92+
@Test
93+
@SneakyThrows
94+
public void testKnnSettingCannotBeIgnored_whenRestore_thenFailure() {
8395
// invalid restore
8496
XContentBuilder restoreCommand = JsonXContent.contentBuilder().startObject();
8597
restoreCommand.field("indices", index);

‎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.http.client.utils.URIBuilder;
1818
import org.apache.http.util.EntityUtils;
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)