Fix index store type settings ordering for searchable snapshots (#7297)

Signed-off-by: Kunal Kotwani <[email protected]> (cherry picked from commit 72ed13b) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
opensearch-project · Apr 25, 2023 · 1ffc73b · 1ffc73b
1 parent e7f72dd
commit 1ffc73b
Show file tree

Hide file tree

Showing 2 changed files with 29 additions and 1 deletion.
diff --git a/server/src/internalClusterTest/java/org/opensearch/snapshots/SearchableSnapshotIT.java b/server/src/internalClusterTest/java/org/opensearch/snapshots/SearchableSnapshotIT.java
@@ -14,6 +14,8 @@
 import org.opensearch.action.admin.cluster.snapshots.delete.DeleteSnapshotRequest;
 import org.opensearch.action.admin.cluster.snapshots.get.GetSnapshotsResponse;
 import org.opensearch.action.admin.cluster.snapshots.restore.RestoreSnapshotRequest;
+import org.opensearch.action.admin.indices.settings.get.GetSettingsRequest;
+import org.opensearch.action.admin.indices.settings.get.GetSettingsResponse;
 import org.opensearch.action.admin.indices.settings.put.UpdateSettingsRequestBuilder;
 import org.opensearch.action.index.IndexRequestBuilder;
 import org.opensearch.action.support.master.AcknowledgedResponse;
@@ -28,6 +30,7 @@
 import org.opensearch.common.settings.Settings;
 import org.opensearch.common.unit.ByteSizeUnit;
 import org.opensearch.index.Index;
+import org.opensearch.index.IndexModule;
 import org.opensearch.index.IndexNotFoundException;
 import org.opensearch.index.store.remote.file.CleanerDaemonThreadLeakFilter;
 import org.opensearch.index.store.remote.filecache.FileCacheStats;
@@ -101,6 +104,7 @@ public void testCreateSearchableSnapshot() throws Exception {
 
         internalCluster().ensureAtLeastNumSearchNodes(Math.max(numReplicasIndex1, numReplicasIndex2) + 1);
         restoreSnapshotAndEnsureGreen(client, snapshotName, repoName);
+        assertRemoteSnapshotIndexSettings(client, restoredIndexName1, restoredIndexName2);
 
         assertDocCount(restoredIndexName1, 100L);
         assertDocCount(restoredIndexName2, 100L);
@@ -196,6 +200,7 @@ public void testCreateSearchableSnapshotWithChunks() throws Exception {
 
         deleteIndicesAndEnsureGreen(client, indexName);
         restoreSnapshotAndEnsureGreen(client, snapshotName, repoName);
+        assertRemoteSnapshotIndexSettings(client, restoredIndexName);
 
         assertDocCount(restoredIndexName, 1000L);
     }
@@ -219,6 +224,7 @@ public void testSearchableSnapshotAllocationForLocalAndRemoteShardsOnSameNode()
         takeSnapshot(client, snapshotName, repoName, indexName);
 
         restoreSnapshotAndEnsureGreen(client, snapshotName, repoName);
+        assertRemoteSnapshotIndexSettings(client, restoredIndexName);
 
         assertDocCount(restoredIndexName, 100L);
         assertDocCount(indexName, 100L);
@@ -246,6 +252,7 @@ public void testSearchableSnapshotAllocationForFailoverAndRecovery() throws Exce
 
         internalCluster().ensureAtLeastNumSearchNodes(numReplicasIndex + 1);
         restoreSnapshotAndEnsureGreen(client, snapshotName, repoName);
+        assertRemoteSnapshotIndexSettings(client, restoredIndexName);
         assertDocCount(restoredIndexName, 100L);
 
         logger.info("--> stop a random search node");
@@ -285,6 +292,7 @@ public void testSearchableSnapshotIndexIsReadOnly() throws Exception {
 
         internalCluster().ensureAtLeastNumSearchNodes(1);
         restoreSnapshotAndEnsureGreen(client, snapshotName, repoName);
+        assertRemoteSnapshotIndexSettings(client, restoredIndexName);
 
         assertIndexingBlocked(restoredIndexName);
         assertTrue(client.admin().indices().prepareDelete(restoredIndexName).get().isAcknowledged());
@@ -334,6 +342,7 @@ private void createIndexWithDocsAndEnsureGreen(int numReplicasIndex, int numOfDo
             Settings.builder()
                 .put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, Integer.toString(numReplicasIndex))
                 .put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, "1")
+                .put(IndexModule.INDEX_STORE_TYPE_SETTING.getKey(), IndexModule.Type.FS.getSettingsKey())
                 .build()
         );
         ensureGreen();
@@ -386,6 +395,20 @@ private void restoreSnapshotAndEnsureGreen(Client client, String snapshotName, S
         ensureGreen();
     }
 
+    private void assertRemoteSnapshotIndexSettings(Client client, String... snapshotIndexNames) {
+        GetSettingsResponse settingsResponse = client.admin()
+            .indices()
+            .getSettings(new GetSettingsRequest().indices(snapshotIndexNames))
+            .actionGet();
+        assertEquals(snapshotIndexNames.length, settingsResponse.getIndexToSettings().keys().size());
+        for (String snapshotIndexName : snapshotIndexNames) {
+            assertEquals(
+                IndexModule.Type.REMOTE_SNAPSHOT.getSettingsKey(),
+                settingsResponse.getSetting(snapshotIndexName, IndexModule.INDEX_STORE_TYPE_SETTING.getKey())
+            );
+        }
+    }
+
     private void assertIndexingBlocked(String index) {
         try {
             final IndexRequestBuilder builder = client().prepareIndex(index);
@@ -411,6 +434,7 @@ public void testUpdateIndexSettings() throws InterruptedException {
 
         internalCluster().ensureAtLeastNumSearchNodes(1);
         restoreSnapshotAndEnsureGreen(client, snapshotName, repoName);
+        assertRemoteSnapshotIndexSettings(client, restoredIndexName);
 
         testUpdateIndexSettingsOnlyNotAllowedSettings(restoredIndexName);
         testUpdateIndexSettingsOnlyAllowedSettings(restoredIndexName);
@@ -491,6 +515,8 @@ public void testFileCacheRestore() throws Exception {
 
         internalCluster().ensureAtLeastNumSearchNodes(numReplicasIndex + 1);
         restoreSnapshotAndEnsureGreen(client, snapshotName, repoName);
+        assertRemoteSnapshotIndexSettings(client, restoredIndexName);
+
         assertDocCount(restoredIndexName, 100L);
         assertIndexDirectoryDoesNotExist(restoredIndexName);
 
@@ -598,6 +624,7 @@ public void testPruneFileCacheOnIndexDeletion() throws Exception {
         deleteIndicesAndEnsureGreen(client, indexName1);
 
         restoreSnapshotAndEnsureGreen(client, snapshotName, repoName);
+        assertRemoteSnapshotIndexSettings(client, restoredIndexName1);
         assertNodesFileCacheNonEmpty(numNodes);
 
         deleteIndicesAndEnsureGreen(client, restoredIndexName1);
@@ -623,6 +650,7 @@ public void testCacheIndexFilesClearedOnDelete() throws Exception {
         takeSnapshot(client, snapshotName, repoName, indexName);
         restoreSnapshotAndEnsureGreen(client, snapshotName, repoName);
         assertDocCount(restoredIndexName, 100L);
+        assertRemoteSnapshotIndexSettings(client, restoredIndexName);
 
         // The index count will be 1 since there is only a single restored index "test-idx-copy"
         assertCacheDirectoryReplicaAndIndexCount(numShards, 1);

diff --git a/server/src/main/java/org/opensearch/snapshots/RestoreService.java b/server/src/main/java/org/opensearch/snapshots/RestoreService.java
@@ -1255,12 +1255,12 @@ public void applyClusterState(ClusterChangedEvent event) {
 
     private static IndexMetadata addSnapshotToIndexSettings(IndexMetadata metadata, Snapshot snapshot, IndexId indexId) {
         final Settings newSettings = Settings.builder()
+            .put(metadata.getSettings())
             .put(IndexModule.INDEX_STORE_TYPE_SETTING.getKey(), IndexModule.Type.REMOTE_SNAPSHOT.getSettingsKey())
             .put(IndexSettings.SEARCHABLE_SNAPSHOT_REPOSITORY.getKey(), snapshot.getRepository())
             .put(IndexSettings.SEARCHABLE_SNAPSHOT_ID_UUID.getKey(), snapshot.getSnapshotId().getUUID())
             .put(IndexSettings.SEARCHABLE_SNAPSHOT_ID_NAME.getKey(), snapshot.getSnapshotId().getName())
             .put(IndexSettings.SEARCHABLE_SNAPSHOT_INDEX_ID.getKey(), indexId.getId())
-            .put(metadata.getSettings())
             .build();
         return IndexMetadata.builder(metadata).settings(newSettings).build();
     }