diff --git a/server/src/internalClusterTest/java/org/opensearch/remotemigration/RemoteStoreMigrationAllocationIT.java b/server/src/internalClusterTest/java/org/opensearch/remotemigration/RemoteStoreMigrationAllocationIT.java index b8a70a5f10de7..3ab2cf1920549 100644 --- a/server/src/internalClusterTest/java/org/opensearch/remotemigration/RemoteStoreMigrationAllocationIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/remotemigration/RemoteStoreMigrationAllocationIT.java @@ -50,7 +50,7 @@ public class RemoteStoreMigrationAllocationIT extends MigrationBaseTestCase { // tests for primary shard copy allocation with MIXED mode and REMOTE_STORE direction - public void testDontAllocateNewPrimaryShardOnNonRemoteNodeForMixedModeAndRemoteStoreDirection() throws Exception { + public void testAllocateNewPrimaryShardForMixedModeAndRemoteStoreDirection() throws Exception { logger.info(" --> initialize cluster"); initializeCluster(false); @@ -71,59 +71,29 @@ public void testDontAllocateNewPrimaryShardOnNonRemoteNodeForMixedModeAndRemoteS setDirection(REMOTE_STORE.direction); Decision decision = getDecisionForTargetNode(nonRemoteNode, true, true, false); - Decision.Type type = Decision.Type.NO; - assertEquals(type, decision.type()); + assertEquals(Decision.Type.NO, decision.type()); assertEquals( "[remote_store migration_direction]: primary shard copy can not be allocated to a non-remote node", decision.getExplanation().toLowerCase(Locale.ROOT) ); - logger.info(" --> attempt allocation"); + logger.info(" --> attempt allocation on non-remote node"); attemptAllocation(nonRemoteNodeName); - logger.info(" --> verify non-allocation of primary shard"); + logger.info(" --> verify non-allocation of primary shard on non-remote node"); assertNonAllocation(true); - } - - public void testAllocateNewPrimaryShardOnRemoteNodeForMixedModeAndRemoteStoreDirection() throws Exception { - logger.info(" --> initialize cluster"); - initializeCluster(false); - - logger.info(" --> add remote and non-remote nodes"); - setClusterMode(MIXED.mode); - addRemote = true; - String remoteNodeName = internalCluster().startNode(); - addRemote = false; - String nonRemoteNodeName = internalCluster().startNode(); - internalCluster().validateClusterFormed(); - DiscoveryNode remoteNode = assertNodeInCluster(remoteNodeName); - DiscoveryNode nonRemoteNode = assertNodeInCluster(nonRemoteNodeName); logger.info(" --> verify expected decision for allocating a new primary shard on a remote node"); - prepareIndexWithoutReplica(Optional.empty()); - - logger.info(" --> set remote_store direction"); - setDirection(REMOTE_STORE.direction); - - Decision decision = getDecisionForTargetNode(remoteNode, true, true, false); + prepareDecisions(); + decision = getDecisionForTargetNode(remoteNode, true, true, false); assertEquals(Decision.Type.YES, decision.type()); assertEquals( "[remote_store migration_direction]: primary shard copy can be allocated to a remote node", decision.getExplanation().toLowerCase(Locale.ROOT) ); - logger.info(" --> attempt allocation"); - client.admin() - .indices() - .prepareUpdateSettings(TEST_INDEX) - .setSettings( - Settings.builder() - .put("index.routing.allocation.include._name", allNodesExcept(null)) - .put("index.routing.allocation.exclude._name", "") - ) - .execute() - .actionGet(); - + logger.info(" --> attempt allocation on remote node"); + attemptAllocation(remoteNodeName); ensureGreen(TEST_INDEX); logger.info(" --> verify allocation of primary shard"); @@ -236,11 +206,11 @@ public void testAllocateNewReplicaShardOnNonRemoteNodeIfPrimaryShardOnNonRemoteN logger.info(" --> verify expected decision for replica shard"); prepareDecisions(); Decision decision = getDecisionForTargetNode(nonRemoteNode2, false, true, false); - Decision.Type type = Decision.Type.YES; - String reason = "[remote_store migration_direction]: replica shard copy can be allocated to a non-remote node"; + Decision.Type expectedType = Decision.Type.YES; + String expectedReason = "[remote_store migration_direction]: replica shard copy can be allocated to a non-remote node"; - assertEquals(type, decision.type()); - assertEquals(reason, decision.getExplanation().toLowerCase(Locale.ROOT)); + assertEquals(expectedType, decision.type()); + assertEquals(expectedReason, decision.getExplanation().toLowerCase(Locale.ROOT)); logger.info(" --> allocate replica shard on the other non-remote node"); attemptAllocation(nonRemoteNodeName2); @@ -276,8 +246,8 @@ public void testAllocateNewReplicaShardOnNonRemoteNodeIfPrimaryShardOnRemoteNode prepareDecisions(); Decision decision = getDecisionForTargetNode(nonRemoteNode, false, true, false); - Decision.Type type = Decision.Type.YES; - assertEquals(type, decision.type()); + Decision.Type expectedType = Decision.Type.YES; + assertEquals(expectedType, decision.type()); assertEquals( "[remote_store migration_direction]: replica shard copy can be allocated to a non-remote node", decision.getExplanation().toLowerCase(Locale.ROOT) @@ -352,13 +322,13 @@ public void testAlwaysAllocateNewShardForStrictMode() throws Exception { prepareDecisions(); Decision decision = getDecisionForTargetNode(targetNode, !isReplicaAllocation, true, false); assertEquals(Decision.Type.YES, decision.type()); - String reason = String.format( + String expectedReason = String.format( Locale.ROOT, "[remote_store migration_direction]: %s shard copy can be allocated to a %s node for strict compatibility mode", (isReplicaAllocation ? "replica" : "primary"), (isRemoteCluster ? "remote" : "non-remote") ); - assertEquals(reason, decision.getExplanation().toLowerCase(Locale.ROOT)); + assertEquals(expectedReason, decision.getExplanation().toLowerCase(Locale.ROOT)); logger.info(" --> attempt allocation"); attemptAllocation(targetNode.getName()); @@ -410,12 +380,12 @@ public void testDontAllocateToNonRemoteNodeForRemoteStoreBackedIndex() throws Ex prepareDecisions(); Decision decision = getDecisionForTargetNode(nonRemoteNode, !isReplicaAllocation, false, false); assertEquals(Decision.Type.NO, decision.type()); - String reason = String.format( + String expectedReason = String.format( Locale.ROOT, "[remote_store migration_direction]: %s shard copy can not be allocated to a non-remote node because a remote store backed index's shard copy can only be allocated to a remote node", (isReplicaAllocation ? "replica" : "primary") ); - assertEquals(reason, decision.getExplanation().toLowerCase(Locale.ROOT)); + assertEquals(expectedReason, decision.getExplanation().toLowerCase(Locale.ROOT)); logger.info(" --> attempt allocation of shard on non-remote node"); attemptAllocation(nonRemoteNodeName); diff --git a/server/src/internalClusterTest/java/org/opensearch/remotemigration/RemoteStoreMigrationSettingsUpdateIT.java b/server/src/internalClusterTest/java/org/opensearch/remotemigration/RemoteStoreMigrationSettingsUpdateIT.java index b7a3eea09234f..c8dacd497dce5 100644 --- a/server/src/internalClusterTest/java/org/opensearch/remotemigration/RemoteStoreMigrationSettingsUpdateIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/remotemigration/RemoteStoreMigrationSettingsUpdateIT.java @@ -10,18 +10,14 @@ import org.opensearch.action.admin.cluster.snapshots.restore.RestoreSnapshotResponse; import org.opensearch.client.Client; -import org.opensearch.cluster.metadata.IndexMetadata; import org.opensearch.common.settings.Settings; import org.opensearch.common.settings.SettingsException; import org.opensearch.core.rest.RestStatus; import org.opensearch.index.IndexSettings; import org.opensearch.indices.replication.common.ReplicationType; -import org.opensearch.snapshots.SnapshotInfo; -import org.opensearch.snapshots.SnapshotState; import org.opensearch.test.InternalTestCluster; import org.opensearch.test.OpenSearchIntegTestCase; -import java.nio.file.Path; import java.util.Optional; import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_REMOTE_SEGMENT_STORE_REPOSITORY; @@ -38,7 +34,6 @@ import static org.opensearch.remotemigration.RemoteStoreMigrationAllocationIT.prepareIndexWithoutReplica; import static org.opensearch.remotemigration.RemoteStoreMigrationAllocationIT.setClusterMode; import static org.opensearch.remotemigration.RemoteStoreMigrationAllocationIT.setDirection; -import static org.opensearch.test.hamcrest.OpenSearchAssertions.assertAcked; @OpenSearchIntegTestCase.ClusterScope(scope = OpenSearchIntegTestCase.Scope.TEST, numDataNodes = 0, autoManageMasterNodes = false) public class RemoteStoreMigrationSettingsUpdateIT extends MigrationBaseTestCase { @@ -56,9 +51,9 @@ public void testNewIndexIsRemoteStoreBackedForRemoteStoreDirectionAndMixedMode() logger.info(" --> add non-remote node"); addRemote = false; - String remoteNodeName = internalCluster().startNode(); + String nonRemoteNodeName = internalCluster().startNode(); internalCluster().validateClusterFormed(); - assertNodeInCluster(remoteNodeName); + assertNodeInCluster(nonRemoteNodeName); logger.info(" --> create an index"); prepareIndexWithoutReplica(Optional.of(indexName1)); @@ -72,9 +67,9 @@ public void testNewIndexIsRemoteStoreBackedForRemoteStoreDirectionAndMixedMode() logger.info(" --> add remote node"); addRemote = true; - String nonRemoteNodeName = internalCluster().startNode(); + String remoteNodeName = internalCluster().startNode(); internalCluster().validateClusterFormed(); - assertNodeInCluster(nonRemoteNodeName); + assertNodeInCluster(remoteNodeName); logger.info(" --> create another index"); prepareIndexWithoutReplica(Optional.of(indexName2)); @@ -83,72 +78,6 @@ public void testNewIndexIsRemoteStoreBackedForRemoteStoreDirectionAndMixedMode() assertRemoteStoreBackedIndex(indexName2); } - public void testNewRestoredIndexIsRemoteStoreBackedForRemoteStoreDirectionAndMixedMode() throws Exception { - logger.info(" --> initialize cluster: gives non remote cluster manager"); - initializeCluster(false); - - logger.info(" --> add remote and non-remote nodes"); - setClusterMode(MIXED.mode); - addRemote = false; - String nonRemoteNodeName = internalCluster().startNode(); - addRemote = true; - String remoteNodeName = internalCluster().startNode(); - internalCluster().validateClusterFormed(); - assertNodeInCluster(nonRemoteNodeName); - assertNodeInCluster(remoteNodeName); - - logger.info(" --> create a non remote-backed index"); - client.admin() - .indices() - .prepareCreate(TEST_INDEX) - .setSettings( - Settings.builder().put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, 1).put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, 0).build() - ) - .get(); - - logger.info(" --> verify that non remote stored backed index is created"); - assertNonRemoteStoreBackedIndex(TEST_INDEX); - - logger.info(" --> create repository"); - String snapshotName = "test-snapshot"; - String snapshotRepoName = "test-restore-snapshot-repo"; - Path snapshotRepoNameAbsolutePath = randomRepoPath().toAbsolutePath(); - assertAcked( - clusterAdmin().preparePutRepository(snapshotRepoName) - .setType("fs") - .setSettings(Settings.builder().put("location", snapshotRepoNameAbsolutePath)) - ); - - logger.info(" --> create snapshot of non remote stored backed index"); - - SnapshotInfo snapshotInfo = client().admin() - .cluster() - .prepareCreateSnapshot(snapshotRepoName, snapshotName) - .setIndices(TEST_INDEX) - .setWaitForCompletion(true) - .get() - .getSnapshotInfo(); - - assertEquals(SnapshotState.SUCCESS, snapshotInfo.state()); - assertTrue(snapshotInfo.successfulShards() > 0); - assertEquals(0, snapshotInfo.failedShards()); - - logger.info(" --> restore index from snapshot under NONE direction"); - String restoredIndexName1 = TEST_INDEX + "-restored1"; - restoreSnapshot(snapshotRepoName, snapshotName, restoredIndexName1); - - logger.info(" --> verify that restored index is non remote-backed"); - assertNonRemoteStoreBackedIndex(restoredIndexName1); - - logger.info(" --> restore index from snapshot under REMOTE_STORE direction"); - setDirection(REMOTE_STORE.direction); - String restoredIndexName2 = TEST_INDEX + "-restored2"; - restoreSnapshot(snapshotRepoName, snapshotName, restoredIndexName2); - - logger.info(" --> verify that restored index is non remote-backed"); - assertRemoteStoreBackedIndex(restoredIndexName2); - } - // compatibility mode setting test public void testSwitchToStrictMode() throws Exception { diff --git a/server/src/main/java/org/opensearch/action/admin/cluster/settings/TransportClusterUpdateSettingsAction.java b/server/src/main/java/org/opensearch/action/admin/cluster/settings/TransportClusterUpdateSettingsAction.java index 618e16e085985..5ce83ff79758d 100644 --- a/server/src/main/java/org/opensearch/action/admin/cluster/settings/TransportClusterUpdateSettingsAction.java +++ b/server/src/main/java/org/opensearch/action/admin/cluster/settings/TransportClusterUpdateSettingsAction.java @@ -61,7 +61,9 @@ import org.opensearch.transport.TransportService; import java.io.IOException; -import java.util.Map; +import java.util.ArrayList; +import java.util.List; +import java.util.Optional; /** * Transport action for updating cluster settings @@ -268,21 +270,22 @@ public ClusterState execute(final ClusterState currentState) { ); } - private void validateCompatibilityModeSettingRequest(ClusterUpdateSettingsRequest request, ClusterState state) { + /** + * Verifies that while trying to switch to STRICT compatibility mode, all nodes must be of the + * same type (all remote or all non-remote). If not, it throws SettingsException error + * @param request cluster settings update request, for settings to be updated and new values + * @param clusterState current state of cluster, for information on nodes + */ + private void validateCompatibilityModeSettingRequest(ClusterUpdateSettingsRequest request, ClusterState clusterState) { if (RemoteStoreNodeService.REMOTE_STORE_COMPATIBILITY_MODE_SETTING.exists(request.persistentSettings())) { String value = request.persistentSettings().get(RemoteStoreNodeService.REMOTE_STORE_COMPATIBILITY_MODE_SETTING.getKey()); if (value.equals(RemoteStoreNodeService.CompatibilityMode.STRICT.mode)) { - boolean hasRemoteNode = false, hasNonRemoteNode = false; - Map nodes = state.nodes().getNodes(); - for (Map.Entry entry : nodes.entrySet()) { - DiscoveryNode node = entry.getValue(); - if (node.isRemoteStoreNode()) { - hasRemoteNode = true; - continue; - } - hasNonRemoteNode = true; - } - if (hasRemoteNode && hasNonRemoteNode) { + List discoveryNodeList = new ArrayList<>(clusterState.nodes().getNodes().values()); + Optional remoteNode = discoveryNodeList.stream().filter(DiscoveryNode::isRemoteStoreNode).findFirst(); + Optional nonRemoteNode = discoveryNodeList.stream() + .filter(dn -> dn.isRemoteStoreNode() == false) + .findFirst(); + if (remoteNode.isPresent() && nonRemoteNode.isPresent()) { throw new SettingsException( "can not switch to STRICT compatibility mode when the cluster contains both remote and non-remote nodes" ); diff --git a/server/src/main/java/org/opensearch/cluster/metadata/IndexMetadata.java b/server/src/main/java/org/opensearch/cluster/metadata/IndexMetadata.java index 03784df509ed6..d52310502aef0 100644 --- a/server/src/main/java/org/opensearch/cluster/metadata/IndexMetadata.java +++ b/server/src/main/java/org/opensearch/cluster/metadata/IndexMetadata.java @@ -319,6 +319,8 @@ public Iterator> settings() { Property.Final ); + public static final String SETTING_REMOTE_STORE_PREFIX = "index.remote_store."; + public static final String SETTING_REMOTE_STORE_ENABLED = "index.remote_store.enabled"; public static final String SETTING_REMOTE_SEGMENT_STORE_REPOSITORY = "index.remote_store.segment.repository"; diff --git a/server/src/main/java/org/opensearch/cluster/metadata/MetadataCreateIndexService.java b/server/src/main/java/org/opensearch/cluster/metadata/MetadataCreateIndexService.java index f645d95b69526..bdafed032157f 100644 --- a/server/src/main/java/org/opensearch/cluster/metadata/MetadataCreateIndexService.java +++ b/server/src/main/java/org/opensearch/cluster/metadata/MetadataCreateIndexService.java @@ -99,7 +99,6 @@ import org.opensearch.indices.replication.common.ReplicationType; import org.opensearch.node.Node; import org.opensearch.node.remotestore.RemoteStoreNodeAttribute; -import org.opensearch.node.remotestore.RemoteStoreNodeService; import org.opensearch.threadpool.ThreadPool; import java.io.IOException; @@ -141,6 +140,7 @@ import static org.opensearch.index.IndexModule.INDEX_STORE_TYPE_SETTING; import static org.opensearch.indices.IndicesService.CLUSTER_REPLICATION_TYPE_SETTING; import static org.opensearch.node.remotestore.RemoteStoreNodeAttribute.isRemoteDataAttributePresent; +import static org.opensearch.node.remotestore.RemoteStoreNodeService.isMigratingToRemoteStore; /** * Service responsible for submitting create index requests @@ -907,7 +907,7 @@ static Settings aggregateIndexSettings( updateReplicationStrategy(indexSettingsBuilder, request.settings(), settings, combinedTemplateSettings); updateRemoteStoreSettings(indexSettingsBuilder, settings); - updateRemoteStoreSettingsForMigration(indexSettingsBuilder, currentState, clusterSettings); + updateRemoteStoreSettingsForMigration(indexSettingsBuilder, currentState, clusterSettings, request.index()); if (sourceMetadata != null) { assert request.resizeType() != null; @@ -1014,38 +1014,40 @@ public static void updateRemoteStoreSettings(Settings.Builder settingsBuilder, S public static void updateRemoteStoreSettingsForMigration( Settings.Builder settingsBuilder, ClusterState clusterState, - ClusterSettings clusterSettings + ClusterSettings clusterSettings, + String indexName ) { String value = settingsBuilder.get(SETTING_REMOTE_STORE_ENABLED); if (value != null && value.toLowerCase(Locale.ROOT).equals("true")) { return; } - boolean isMixedMode = clusterSettings.get(RemoteStoreNodeService.REMOTE_STORE_COMPATIBILITY_MODE_SETTING) - .equals(RemoteStoreNodeService.CompatibilityMode.MIXED); - boolean isRemoteStoreMigrationDirection = clusterSettings.get(RemoteStoreNodeService.MIGRATION_DIRECTION_SETTING) - .equals(RemoteStoreNodeService.Direction.REMOTE_STORE); - - if (isMixedMode && isRemoteStoreMigrationDirection) { + if (isMigratingToRemoteStore(clusterSettings)) { String segmentRepo, translogRepo; - - Map nodes = clusterState.nodes().getNodes(); - for (Map.Entry entry : nodes.entrySet()) { - DiscoveryNode node = entry.getValue(); - if (node.isRemoteStoreNode()) { - translogRepo = node.getAttributes().get(RemoteStoreNodeAttribute.REMOTE_STORE_TRANSLOG_REPOSITORY_NAME_ATTRIBUTE_KEY); - segmentRepo = node.getAttributes().get(RemoteStoreNodeAttribute.REMOTE_STORE_SEGMENT_REPOSITORY_NAME_ATTRIBUTE_KEY); - if (segmentRepo != null && translogRepo != null) { - value = settingsBuilder.get(SETTING_REPLICATION_TYPE); - if (value == null || value.equals(ReplicationType.SEGMENT.toString()) == false) { - settingsBuilder.put(SETTING_REPLICATION_TYPE, ReplicationType.SEGMENT); - } - settingsBuilder.put(SETTING_REMOTE_STORE_ENABLED, true) - .put(SETTING_REMOTE_SEGMENT_STORE_REPOSITORY, segmentRepo) - .put(SETTING_REMOTE_TRANSLOG_STORE_REPOSITORY, translogRepo); - - break; - } + Optional remoteNode = clusterState.nodes() + .getNodes() + .values() + .stream() + .filter(DiscoveryNode::isRemoteStoreNode) + .findFirst(); + if (remoteNode.isPresent()) { + translogRepo = remoteNode.get() + .getAttributes() + .get(RemoteStoreNodeAttribute.REMOTE_STORE_TRANSLOG_REPOSITORY_NAME_ATTRIBUTE_KEY); + segmentRepo = remoteNode.get() + .getAttributes() + .get(RemoteStoreNodeAttribute.REMOTE_STORE_SEGMENT_REPOSITORY_NAME_ATTRIBUTE_KEY); + if (segmentRepo != null && translogRepo != null) { + settingsBuilder.put(SETTING_REPLICATION_TYPE, ReplicationType.SEGMENT) + .put(SETTING_REMOTE_STORE_ENABLED, true) + .put(SETTING_REMOTE_SEGMENT_STORE_REPOSITORY, segmentRepo) + .put(SETTING_REMOTE_TRANSLOG_STORE_REPOSITORY, translogRepo); + } else { + ValidationException validationException = new ValidationException(); + validationException.addValidationErrors( + Collections.singletonList("Cluster is migrating to remote store but no remote node found, failing index creation") + ); + throw new IndexCreationException(indexName, validationException); } } } diff --git a/server/src/main/java/org/opensearch/node/remotestore/RemoteStoreNodeService.java b/server/src/main/java/org/opensearch/node/remotestore/RemoteStoreNodeService.java index 33b182dd3cc97..b9b956e4870ca 100644 --- a/server/src/main/java/org/opensearch/node/remotestore/RemoteStoreNodeService.java +++ b/server/src/main/java/org/opensearch/node/remotestore/RemoteStoreNodeService.java @@ -14,6 +14,7 @@ import org.opensearch.cluster.metadata.RepositoriesMetadata; import org.opensearch.cluster.metadata.RepositoryMetadata; import org.opensearch.cluster.node.DiscoveryNode; +import org.opensearch.common.settings.ClusterSettings; import org.opensearch.common.settings.Setting; import org.opensearch.common.util.FeatureFlags; import org.opensearch.repositories.RepositoriesService; @@ -223,4 +224,18 @@ public RepositoriesMetadata updateRepositoriesMetadata(DiscoveryNode joiningNode return existingRepositories; } } + + /** + * To check if the cluster is undergoing remote store migration + * @param clusterSettings cluster level settings + * @return + * true For REMOTE_STORE migration direction and MIXED compatibility mode, + * false otherwise + */ + public static boolean isMigratingToRemoteStore(ClusterSettings clusterSettings) { + boolean isMixedMode = clusterSettings.get(REMOTE_STORE_COMPATIBILITY_MODE_SETTING).equals(CompatibilityMode.MIXED); + boolean isRemoteStoreMigrationDirection = clusterSettings.get(MIGRATION_DIRECTION_SETTING).equals(Direction.REMOTE_STORE); + + return (isMixedMode == true && isRemoteStoreMigrationDirection == true); + } } diff --git a/server/src/main/java/org/opensearch/snapshots/RestoreService.java b/server/src/main/java/org/opensearch/snapshots/RestoreService.java index cb518bbb5e380..bf2c7fc74be92 100644 --- a/server/src/main/java/org/opensearch/snapshots/RestoreService.java +++ b/server/src/main/java/org/opensearch/snapshots/RestoreService.java @@ -79,7 +79,6 @@ 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; @@ -91,8 +90,6 @@ 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; @@ -113,26 +110,21 @@ 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_SEGMENT_STORE_REPOSITORY; import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_REMOTE_STORE_ENABLED; -import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_REMOTE_TRANSLOG_STORE_REPOSITORY; -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.IndexModule.INDEX_STORE_TYPE_SETTING; 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; /** @@ -161,29 +153,20 @@ public class RestoreService implements ClusterStateApplier { private static final Logger logger = LogManager.getLogger(RestoreService.class); - private static final Set USER_UNMODIFIABLE_SETTINGS = unmodifiableSet( - newHashSet( - SETTING_NUMBER_OF_SHARDS, - SETTING_VERSION_CREATED, - SETTING_INDEX_UUID, - SETTING_CREATION_DATE, - SETTING_HISTORY_UUID, - SETTING_REMOTE_STORE_ENABLED, - SETTING_REMOTE_SEGMENT_STORE_REPOSITORY, - SETTING_REMOTE_TRANSLOG_STORE_REPOSITORY - ) + private static final Set UNMODIFIABLE_SETTINGS = unmodifiableSet( + newHashSet(SETTING_NUMBER_OF_SHARDS, SETTING_VERSION_CREATED, SETTING_INDEX_UUID, SETTING_CREATION_DATE, SETTING_HISTORY_UUID) ); // It's OK to change some settings, but we shouldn't allow simply removing them - private static final Set USER_UNREMOVABLE_SETTINGS; + private static final Set UNREMOVABLE_SETTINGS; static { - Set unremovable = new HashSet<>(USER_UNMODIFIABLE_SETTINGS.size() + 4); - unremovable.addAll(USER_UNMODIFIABLE_SETTINGS); + Set unremovable = new HashSet<>(UNMODIFIABLE_SETTINGS.size() + 4); + unremovable.addAll(UNMODIFIABLE_SETTINGS); unremovable.add(SETTING_NUMBER_OF_REPLICAS); unremovable.add(SETTING_AUTO_EXPAND_REPLICAS); unremovable.add(SETTING_VERSION_UPGRADED); - USER_UNREMOVABLE_SETTINGS = unmodifiableSet(unremovable); + UNREMOVABLE_SETTINGS = unmodifiableSet(unremovable); } private final ClusterService clusterService; @@ -244,6 +227,16 @@ public RestoreService( */ public void restoreSnapshot(final RestoreSnapshotRequest request, final ActionListener listener) { try { + // Setting INDEX_STORE_TYPE_SETTING as REMOTE_SNAPSHOT is intended to be a system-managed index setting that is configured when + // restoring a snapshot and should not be manually set by user. + String storeTypeSetting = request.indexSettings().get(INDEX_STORE_TYPE_SETTING.getKey()); + if (storeTypeSetting != null && storeTypeSetting.equals(RestoreSnapshotRequest.StorageType.REMOTE_SNAPSHOT.toString())) { + throw new SnapshotRestoreException( + request.repository(), + request.snapshot(), + "cannot restore remote snapshot with index settings \"index.store.type\" set to \"remote_snapshot\". Instead use \"storage_type\": \"remote_snapshot\" as argument to restore." + ); + } // Read snapshot info and metadata from the repository final String repositoryName = request.repository(); Repository repository = repositoriesService.repository(repositoryName); @@ -378,16 +371,10 @@ public ClusterState execute(ClusterState currentState) { boolean partial = checkPartial(index); IndexId snapshotIndexId = repositoryData.resolveIndexId(index); - - final Settings overrideSettingsInternal = getOverrideSettingsInternal(); - final String[] ignoreSettingsInternal = getIgnoreSettingsInternal(); - IndexMetadata snapshotIndexMetadata = updateIndexSettings( metadata.index(index), request.indexSettings(), - request.ignoreIndexSettings(), - overrideSettingsInternal, - ignoreSettingsInternal + request.ignoreIndexSettings() ); if (isRemoteSnapshot) { snapshotIndexMetadata = addSnapshotToIndexSettings(snapshotIndexMetadata, snapshot, snapshotIndexId); @@ -448,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 valid name + // Make sure that the index we are about to create has a validate name boolean isHidden = IndexMetadata.INDEX_HIDDEN_SETTING.get(snapshotIndexMetadata.getSettings()); createIndexService.validateIndexName(renamedIndexName, currentState); createIndexService.validateDotIndex(renamedIndexName, isHidden); @@ -653,54 +640,6 @@ private void checkAliasNameConflicts(Map renamedIndices, Set ignoreShards) { for (SnapshotShardFailure failure : snapshotInfo.shardFailures()) { if (index.equals(failure.index())) { @@ -774,9 +713,7 @@ private void validateExistingIndex( private IndexMetadata updateIndexSettings( IndexMetadata indexMetadata, Settings changeSettings, - String[] ignoreSettings, - Settings changeSettingsInternal, - String[] ignoreSettingsInternal + String[] ignoreSettings ) { Settings normalizedChangeSettings = Settings.builder() .put(changeSettings) @@ -796,7 +733,7 @@ private IndexMetadata updateIndexSettings( List simpleMatchPatterns = new ArrayList<>(); for (String ignoredSetting : ignoreSettings) { if (!Regex.isSimpleMatchPattern(ignoredSetting)) { - if (USER_UNREMOVABLE_SETTINGS.contains(ignoredSetting)) { + if (UNREMOVABLE_SETTINGS.contains(ignoredSetting)) { throw new SnapshotRestoreException( snapshot, "cannot remove setting [" + ignoredSetting + "] on restore" @@ -808,18 +745,8 @@ private IndexMetadata updateIndexSettings( simpleMatchPatterns.add(ignoredSetting); } } - - // add internal settings to ignore settings list - for (String ignoredSetting : ignoreSettingsInternal) { - if (!Regex.isSimpleMatchPattern(ignoredSetting)) { - keyFilters.add(ignoredSetting); - } else { - simpleMatchPatterns.add(ignoredSetting); - } - } - Predicate settingsFilter = k -> { - if (USER_UNREMOVABLE_SETTINGS.contains(k) == false) { + if (UNREMOVABLE_SETTINGS.contains(k) == false) { for (String filterKey : keyFilters) { if (k.equals(filterKey)) { return false; @@ -836,17 +763,12 @@ private IndexMetadata updateIndexSettings( Settings.Builder settingsBuilder = Settings.builder() .put(settings.filter(settingsFilter)) .put(normalizedChangeSettings.filter(k -> { - if (USER_UNMODIFIABLE_SETTINGS.contains(k)) { + if (UNMODIFIABLE_SETTINGS.contains(k)) { throw new SnapshotRestoreException(snapshot, "cannot modify setting [" + k + "] on restore"); } else { return true; } })); - - // override internal settings - if (changeSettingsInternal != null) { - settingsBuilder.put(changeSettingsInternal).normalizePrefix(IndexMetadata.INDEX_SETTING_PREFIX); - } settingsBuilder.remove(MetadataIndexStateService.VERIFIED_BEFORE_CLOSE_SETTING.getKey()); return builder.settings(settingsBuilder).build(); }