[Snapshot Interop] Add changes for overriding remote store and replic…

…ation settings during snapshot restore. Signed-off-by: Harish Bhakuni <[email protected]>
opensearch-project · Jan 22, 2024 · 310324a · 310324a
1 parent bea196f
commit 310324a
Show file tree

Hide file tree

Showing 4 changed files with 86 additions and 12 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -133,6 +133,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 - Bump OpenTelemetry from 1.32.0 to 1.34.1 ([#11891](https://github.com/opensearch-project/OpenSearch/pull/11891))
 - Support index level allocation filtering for searchable snapshot index ([#11563](https://github.com/opensearch-project/OpenSearch/pull/11563))
 - Add `org.opensearch.rest.MethodHandlers` and `RestController#getAllHandlers` ([11876](https://github.com/opensearch-project/OpenSearch/pull/11876))
+- Add changes for overriding remote store and replication settings during snapshot restore. ([#11868](https://github.com/opensearch-project/OpenSearch/pull/11868))
 
 ### Dependencies
 - Bumps jetty version to 9.4.52.v20230823 to fix GMS-2023-1857 ([#9822](https://github.com/opensearch-project/OpenSearch/pull/9822))

diff --git a/...r/src/internalClusterTest/java/org/opensearch/snapshots/SegmentReplicationSnapshotIT.java b/...r/src/internalClusterTest/java/org/opensearch/snapshots/SegmentReplicationSnapshotIT.java
@@ -112,12 +112,16 @@ public void createSnapshot() {
     }
 
     public RestoreSnapshotResponse restoreSnapshotWithSettings(Settings indexSettings) {
+        return restoreSnapshotWithSettings(indexSettings, RESTORED_INDEX_NAME);
+    }
+
+    public RestoreSnapshotResponse restoreSnapshotWithSettings(Settings indexSettings, String restoredIndexName) {
         RestoreSnapshotRequestBuilder builder = client().admin()
             .cluster()
             .prepareRestoreSnapshot(REPOSITORY_NAME, SNAPSHOT_NAME)
             .setWaitForCompletion(false)
             .setRenamePattern(INDEX_NAME)
-            .setRenameReplacement(RESTORED_INDEX_NAME);
+            .setRenameReplacement(restoredIndexName);
         if (indexSettings != null) {
             builder.setIndexSettings(indexSettings);
         }
@@ -311,7 +315,8 @@ public void testSnapshotRestoreOnIndexWithSegRepClusterSetting() throws Exceptio
     * 2. Snapshot index
     * 3. Add new set of nodes with `cluster.indices.replication.strategy` set to SEGMENT and `cluster.index.restrict.replication.type`
     *    set to true.
-    * 4. Perform restore on new set of nodes to validate restored index has `DOCUMENT` replication.
+    * 4. Perform restore on new set of nodes to validate restored index has `SEGMENT` replication.
+    * 5. Validate that if replication type is passed as DOCUMENT as request parameter, restore operation fails
     */
     public void testSnapshotRestoreOnRestrictReplicationSetting() throws Exception {
         final int documentCount = scaledRandomIntBetween(1, 10);
@@ -361,7 +366,22 @@ public void testSnapshotRestoreOnRestrictReplicationSetting() throws Exception {
         // Perform snapshot restore
         logger.info("--> Performing snapshot restore to target index");
 
-        IllegalArgumentException exception = expectThrows(IllegalArgumentException.class, () -> restoreSnapshotWithSettings(null));
+        RestoreSnapshotResponse restoreSnapshotResponse = restoreSnapshotWithSettings(null);
+
+        // Assertions
+        assertEquals(restoreSnapshotResponse.status(), RestStatus.ACCEPTED);
+        ensureGreen(RESTORED_INDEX_NAME);
+        GetSettingsResponse settingsResponse = client().admin()
+            .indices()
+            .getSettings(new GetSettingsRequest().indices(RESTORED_INDEX_NAME).includeDefaults(true))
+            .get();
+        assertEquals(settingsResponse.getSetting(RESTORED_INDEX_NAME, SETTING_REPLICATION_TYPE), ReplicationType.SEGMENT.toString());
+
+        // Perform Snapshot Restore with different index name
+        IllegalArgumentException exception = expectThrows(
+            IllegalArgumentException.class,
+            () -> restoreSnapshotWithSettings(restoreIndexDocRepSettings(), RESTORED_INDEX_NAME + "2")
+        );
         assertEquals(REPLICATION_MISMATCH_VALIDATION_ERROR, exception.getMessage());
     }
 }
diff --git a/server/src/main/java/org/opensearch/cluster/metadata/MetadataCreateIndexService.java b/server/src/main/java/org/opensearch/cluster/metadata/MetadataCreateIndexService.java
@@ -940,7 +940,7 @@ static Settings aggregateIndexSettings(
      * @param clusterSettings cluster level settings
      * @param combinedTemplateSettings combined template settings which satisfy the index
      */
-    private static void updateReplicationStrategy(
+    public static void updateReplicationStrategy(
         Settings.Builder settingsBuilder,
         Settings requestSettings,
         Settings clusterSettings,
@@ -970,20 +970,20 @@ private static void updateReplicationStrategy(
     /**
      * Updates index settings to enable remote store by default based on node attributes
      * @param settingsBuilder index settings builder to be updated with relevant settings
-     * @param clusterSettings cluster level settings
+     * @param nodeSettings node settings
      */
-    private static void updateRemoteStoreSettings(Settings.Builder settingsBuilder, Settings clusterSettings) {
-        if (isRemoteStoreAttributePresent(clusterSettings)) {
+    public static void updateRemoteStoreSettings(Settings.Builder settingsBuilder, Settings nodeSettings) {
+        if (isRemoteStoreAttributePresent(nodeSettings)) {
             settingsBuilder.put(SETTING_REMOTE_STORE_ENABLED, true)
                 .put(
                     SETTING_REMOTE_SEGMENT_STORE_REPOSITORY,
-                    clusterSettings.get(
+                    nodeSettings.get(
                         Node.NODE_ATTRIBUTES.getKey() + RemoteStoreNodeAttribute.REMOTE_STORE_SEGMENT_REPOSITORY_NAME_ATTRIBUTE_KEY
                     )
                 )
                 .put(
                     SETTING_REMOTE_TRANSLOG_STORE_REPOSITORY,
-                    clusterSettings.get(
+                    nodeSettings.get(
                         Node.NODE_ATTRIBUTES.getKey() + RemoteStoreNodeAttribute.REMOTE_STORE_TRANSLOG_REPOSITORY_NAME_ATTRIBUTE_KEY
                     )
                 );

diff --git a/server/src/main/java/org/opensearch/snapshots/RestoreService.java b/server/src/main/java/org/opensearch/snapshots/RestoreService.java
@@ -79,6 +79,7 @@
 import org.opensearch.common.settings.ClusterSettings;
 import org.opensearch.common.settings.Settings;
 import org.opensearch.common.unit.TimeValue;
+import org.opensearch.common.util.ArrayUtils;
 import org.opensearch.common.util.FeatureFlags;
 import org.opensearch.core.action.ActionListener;
 import org.opensearch.core.index.Index;
@@ -90,6 +91,8 @@
 import org.opensearch.index.store.remote.filecache.FileCacheStats;
 import org.opensearch.indices.IndicesService;
 import org.opensearch.indices.ShardLimitValidator;
+import org.opensearch.indices.replication.common.ReplicationType;
+import org.opensearch.node.remotestore.RemoteStoreNodeAttribute;
 import org.opensearch.repositories.IndexId;
 import org.opensearch.repositories.RepositoriesService;
 import org.opensearch.repositories.Repository;
@@ -110,20 +113,24 @@
 import java.util.stream.Collectors;
 
 import static java.util.Collections.unmodifiableSet;
+import static org.opensearch.cluster.metadata.IndexMetadata.INDEX_REPLICATION_TYPE_SETTING;
 import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_AUTO_EXPAND_REPLICAS;
 import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_CREATION_DATE;
 import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_HISTORY_UUID;
 import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_INDEX_UUID;
 import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_NUMBER_OF_REPLICAS;
 import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_NUMBER_OF_SHARDS;
 import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_REMOTE_STORE_ENABLED;
+import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_REPLICATION_TYPE;
 import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_VERSION_CREATED;
 import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_VERSION_UPGRADED;
 import static org.opensearch.common.util.FeatureFlags.SEARCHABLE_SNAPSHOT_EXTENDED_COMPATIBILITY;
 import static org.opensearch.common.util.set.Sets.newHashSet;
 import static org.opensearch.index.store.remote.directory.RemoteSnapshotDirectory.SEARCHABLE_SNAPSHOT_EXTENDED_COMPATIBILITY_MINIMUM_VERSION;
 import static org.opensearch.index.store.remote.filecache.FileCache.DATA_TO_FILE_CACHE_SIZE_RATIO_SETTING;
+import static org.opensearch.indices.IndicesService.CLUSTER_REPLICATION_TYPE_SETTING;
 import static org.opensearch.node.Node.NODE_SEARCH_CACHE_SIZE_SETTING;
+import static org.opensearch.node.remotestore.RemoteStoreNodeAttribute.isRemoteStoreAttributePresent;
 import static org.opensearch.snapshots.SnapshotUtils.filterIndices;
 
 /**
@@ -360,10 +367,14 @@ public ClusterState execute(ClusterState currentState) {
                                 boolean partial = checkPartial(index);
 
                                 IndexId snapshotIndexId = repositoryData.resolveIndexId(index);
+
+                                final Settings updatedRequestSettings = getUpdatedRequestSettings();
+                                final String[] updatedIgnoredSettings = getUpdatedIgnoredSettings();
+
                                 IndexMetadata snapshotIndexMetadata = updateIndexSettings(
                                     metadata.index(index),
-                                    request.indexSettings(),
-                                    request.ignoreIndexSettings()
+                                    updatedRequestSettings,
+                                    updatedIgnoredSettings
                                 );
                                 if (isRemoteSnapshot) {
                                     snapshotIndexMetadata = addSnapshotToIndexSettings(snapshotIndexMetadata, snapshot, snapshotIndexId);
@@ -424,7 +435,7 @@ public ClusterState execute(ClusterState currentState) {
                                 final Index renamedIndex;
                                 if (currentIndexMetadata == null) {
                                     // Index doesn't exist - create it and start recovery
-                                    // Make sure that the index we are about to create has a validate name
+                                    // Make sure that the index we are about to create has valid name
                                     boolean isHidden = IndexMetadata.INDEX_HIDDEN_SETTING.get(snapshotIndexMetadata.getSettings());
                                     createIndexService.validateIndexName(renamedIndexName, currentState);
                                     createIndexService.validateDotIndex(renamedIndexName, isHidden);
@@ -629,6 +640,48 @@ private void checkAliasNameConflicts(Map<String, String> renamedIndices, Set<Str
                         }
                     }
 
+                    private String[] getUpdatedIgnoredSettings() {
+                        // for non-remote store enabled domain, we will remove all the remote store
+                        // related index settings present in the snapshot.
+                        String[] indexSettingsToBeIgnored = request.ignoreIndexSettings();
+                        if (false == RemoteStoreNodeAttribute.isRemoteStoreAttributePresent(clusterService.getSettings())) {
+                            indexSettingsToBeIgnored = ArrayUtils.concat(indexSettingsToBeIgnored, new String[] { "index.remote_store.*" });
+                        }
+                        return indexSettingsToBeIgnored;
+                    }
+
+                    private Settings getUpdatedRequestSettings() {
+                        final Settings.Builder settingsBuilder = Settings.builder().put(request.indexSettings());
+                        updateReplicationStrategy(
+                            settingsBuilder,
+                            request.indexSettings(),
+                            clusterService.getSettings(),
+                            clusterService.getClusterSettings()
+                        );
+                        // remote store settings needs to be overridden if the remote store feature is enabled in the
+                        // cluster where snapshot is being restored.
+                        MetadataCreateIndexService.updateRemoteStoreSettings(settingsBuilder, clusterService.getSettings());
+                        return settingsBuilder.build();
+                    }
+
+                    private void updateReplicationStrategy(
+                        Settings.Builder settingsBuilder,
+                        Settings requestSettings,
+                        Settings nodeSettings,
+                        ClusterSettings clusterSettings
+                    ) {
+                        // if remote store feature is enabled, override replication strategy to segment.
+                        // if `cluster.index.restrict.replication.type` is enabled and user have not provided
+                        // replication strategy setting with restore request, override replication strategy
+                        // setting to cluster default.
+                        if (isRemoteStoreAttributePresent(nodeSettings)) {
+                            settingsBuilder.put(SETTING_REPLICATION_TYPE, ReplicationType.SEGMENT);
+                        } else if (!INDEX_REPLICATION_TYPE_SETTING.exists(requestSettings)
+                            && clusterSettings.get(IndicesService.CLUSTER_INDEX_RESTRICT_REPLICATION_TYPE_SETTING)) {
+                                settingsBuilder.put(SETTING_REPLICATION_TYPE, clusterSettings.get(CLUSTER_REPLICATION_TYPE_SETTING).name());
+                            }
+                    }
+
                     private void populateIgnoredShards(String index, final Set<Integer> ignoreShards) {
                         for (SnapshotShardFailure failure : snapshotInfo.shardFailures()) {
                             if (index.equals(failure.index())) {