From 9ef15a30822bcc3f06c9e46a0c2e11ac733be6d6 Mon Sep 17 00:00:00 2001 From: Suraj Singh Date: Wed, 13 Dec 2023 16:01:50 -0800 Subject: [PATCH 1/6] Add new cluster level setting that prevents index level replication type setting overrides Signed-off-by: Suraj Singh --- CHANGELOG.md | 1 + .../SegmentReplicationClusterSettingIT.java | 284 +++++++++++++++++- .../SegmentReplicationSnapshotIT.java | 68 +++++ .../metadata/MetadataCreateIndexService.java | 45 ++- .../common/settings/ClusterSettings.java | 3 +- .../opensearch/indices/IndicesService.java | 12 + .../opensearch/snapshots/RestoreService.java | 7 +- 7 files changed, 406 insertions(+), 14 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 52b89e57c0959..50be6ee2744ef 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -122,6 +122,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - [BWC and API enforcement] Introduce checks for enforcing the API restrictions ([#11175](https://github.com/opensearch-project/OpenSearch/pull/11175)) - Create separate transport action for render search template action ([#11170](https://github.com/opensearch-project/OpenSearch/pull/11170)) - Add additional handling in SearchTemplateRequest when simulate is set to true ([#11591](https://github.com/opensearch-project/OpenSearch/pull/11591)) +- Introduce cluster level setting `cluster.force.index.replication_type` that prevents new create index requests when replication type mis-matches with cluster level replication `cluster.indices.replication.strategy`([#11583](https://github.com/opensearch-project/OpenSearch/pull/11583)) ### Dependencies - Bump Lucene from 9.7.0 to 9.8.0 ([10276](https://github.com/opensearch-project/OpenSearch/pull/10276)) diff --git a/server/src/internalClusterTest/java/org/opensearch/indices/replication/SegmentReplicationClusterSettingIT.java b/server/src/internalClusterTest/java/org/opensearch/indices/replication/SegmentReplicationClusterSettingIT.java index a82fd8d845709..91a5d42ae07a8 100644 --- a/server/src/internalClusterTest/java/org/opensearch/indices/replication/SegmentReplicationClusterSettingIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/indices/replication/SegmentReplicationClusterSettingIT.java @@ -10,16 +10,31 @@ import org.opensearch.action.admin.indices.settings.get.GetSettingsRequest; import org.opensearch.action.admin.indices.settings.get.GetSettingsResponse; +import org.opensearch.action.admin.indices.shrink.ResizeType; +import org.opensearch.action.admin.indices.template.get.GetIndexTemplatesResponse; import org.opensearch.cluster.metadata.IndexMetadata; +import org.opensearch.common.collect.Tuple; import org.opensearch.common.settings.Settings; import org.opensearch.core.index.Index; import org.opensearch.index.IndexModule; +import org.opensearch.index.query.TermsQueryBuilder; import org.opensearch.indices.IndicesService; import org.opensearch.indices.replication.common.ReplicationType; import org.opensearch.test.OpenSearchIntegTestCase; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collections; +import java.util.List; +import java.util.Locale; +import java.util.concurrent.TimeUnit; +import java.util.function.BiConsumer; + import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_REPLICATION_TYPE; +import static org.opensearch.indices.IndicesService.CLUSTER_RESTRICT_INDEX_REPLICATION_TYPE_SETTING; import static org.opensearch.indices.IndicesService.CLUSTER_SETTING_REPLICATION_TYPE; +import static org.opensearch.test.hamcrest.OpenSearchAssertions.assertHitCount; +import static org.hamcrest.Matchers.hasSize; @OpenSearchIntegTestCase.ClusterScope(scope = OpenSearchIntegTestCase.Scope.TEST, numDataNodes = 0) public class SegmentReplicationClusterSettingIT extends OpenSearchIntegTestCase { @@ -29,6 +44,9 @@ public class SegmentReplicationClusterSettingIT extends OpenSearchIntegTestCase protected static final int SHARD_COUNT = 1; protected static final int REPLICA_COUNT = 1; + protected static final String REPLICATION_MISMATCH_VALIDATION_ERROR = + "Validation Failed: 1: index setting [index.replication.type] is not allowed to be set as [cluster.force.index.replication_type=true];"; + @Override public Settings indexSettings() { return Settings.builder() @@ -44,14 +62,6 @@ protected boolean addMockInternalEngine() { return false; } - @Override - protected Settings nodeSettings(int nodeOrdinal) { - return Settings.builder() - .put(super.nodeSettings(nodeOrdinal)) - .put(CLUSTER_SETTING_REPLICATION_TYPE, ReplicationType.SEGMENT) - .build(); - } - public void testIndexReplicationSettingOverridesSegRepClusterSetting() throws Exception { Settings settings = Settings.builder().put(CLUSTER_SETTING_REPLICATION_TYPE, ReplicationType.SEGMENT).build(); final String ANOTHER_INDEX = "test-index"; @@ -123,4 +133,262 @@ public void testIndexReplicationSettingOverridesDocRepClusterSetting() throws Ex assertEquals(indicesService.indexService(anotherIndex).getIndexSettings().isSegRepEnabled(), false); } + public void testDifferentReplicationTypes_CreateIndex_StrictMode() throws Throwable { + final int documentCount = scaledRandomIntBetween(1, 10); + BiConsumer, List> consumer = (replicationList, dataNodesList) -> { + Settings indexSettings = Settings.builder().put(indexSettings()).put(SETTING_REPLICATION_TYPE, replicationList.get(1)).build(); + createIndex(INDEX_NAME, indexSettings); + }; + executeTest(true, consumer, INDEX_NAME, documentCount); + } + + public void testDifferentReplicationTypes_CreateIndex_NonStrictMode() throws Throwable { + final int documentCount = scaledRandomIntBetween(1, 10); + BiConsumer, List> consumer = (replicationList, dataNodesList) -> { + Settings indexSettings = Settings.builder().put(indexSettings()).put(SETTING_REPLICATION_TYPE, replicationList.get(1)).build(); + createIndex(INDEX_NAME, indexSettings); + for (int i = 0; i < documentCount; i++) { + client().prepareIndex(INDEX_NAME).setId(String.valueOf(i)).setSource("foo", "bar").get(); + } + }; + executeTest(false, consumer, INDEX_NAME, documentCount); + } + + public void testDifferentReplicationTypes_IndexTemplates_StrictMode() throws Throwable { + final int documentCount = scaledRandomIntBetween(1, 10); + + BiConsumer, List> consumer = (replicationList, dataNodesList) -> { + client().admin() + .indices() + .preparePutTemplate("template_1") + .setPatterns(Collections.singletonList("test-idx*")) + .setSettings(Settings.builder().put(indexSettings()).put(SETTING_REPLICATION_TYPE, ReplicationType.DOCUMENT).build()) + .setOrder(0) + .get(); + + GetIndexTemplatesResponse response = client().admin().indices().prepareGetTemplates().get(); + assertThat(response.getIndexTemplates(), hasSize(1)); + + createIndex(INDEX_NAME); + }; + executeTest(true, consumer, INDEX_NAME, documentCount); + } + + public void testDifferentReplicationTypes_IndexTemplates_NonStrictMode() throws Throwable { + final int documentCount = scaledRandomIntBetween(1, 10); + + // Create an index using current cluster level settings + BiConsumer, List> consumer = (replicationList, dataNodesList) -> { + client().admin() + .indices() + .preparePutTemplate("template_1") + .setPatterns(Collections.singletonList("test-idx*")) + .setSettings(Settings.builder().put(indexSettings()).put(SETTING_REPLICATION_TYPE, replicationList.get(1)).build()) + .setOrder(0) + .get(); + + GetIndexTemplatesResponse response = client().admin().indices().prepareGetTemplates().get(); + assertThat(response.getIndexTemplates(), hasSize(1)); + + createIndex(INDEX_NAME); + for (int i = 0; i < documentCount; i++) { + client().prepareIndex(INDEX_NAME).setId(String.valueOf(i)).setSource("foo", "bar").get(); + } + }; + executeTest(false, consumer, INDEX_NAME, documentCount); + } + + public void testMismatchingReplicationType_ResizeAction_StrictMode() throws Throwable { + // Define resize action and target shard count. + List> resizeActionsList = new ArrayList<>(); + final int initialShardCount = 2; + resizeActionsList.add(new Tuple<>(ResizeType.SPLIT, 2 * initialShardCount)); + resizeActionsList.add(new Tuple<>(ResizeType.SHRINK, SHARD_COUNT)); + resizeActionsList.add(new Tuple<>(ResizeType.CLONE, initialShardCount)); + + Tuple resizeActionTuple = resizeActionsList.get(random().nextInt(resizeActionsList.size())); + final String targetIndexName = resizeActionTuple.v1().name().toLowerCase(Locale.ROOT) + "-target"; + final int documentCount = scaledRandomIntBetween(1, 10); + + logger.info("--> Performing resize action {} with shard count {}", resizeActionTuple.v1(), resizeActionTuple.v2()); + + // Create an index using current cluster level settings + BiConsumer, List> consumer = (replicationList, dataNodesList) -> { + Settings indexSettings = Settings.builder() + .put(indexSettings()) + .put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, initialShardCount) + .put(SETTING_REPLICATION_TYPE, replicationList.get(0)) + .build(); + createIndex(INDEX_NAME, indexSettings); + + for (int i = 0; i < documentCount; i++) { + client().prepareIndex(INDEX_NAME).setId(String.valueOf(i)).setSource("foo", "bar").get(); + } + + // Block writes + client().admin() + .indices() + .prepareUpdateSettings(INDEX_NAME) + .setSettings(Settings.builder().put("index.blocks.write", true)) + .get(); + ensureGreen(); + + try { + for (String node : dataNodesList) { + assertBusy(() -> { + assertHitCount( + client(node).prepareSearch(INDEX_NAME).setSize(100).setQuery(new TermsQueryBuilder("foo", "bar")).get(), + documentCount + ); + }, 30, TimeUnit.SECONDS); + } + } catch (Exception e) { + throw new RuntimeException(e); + } + + // Test create index fails + client().admin() + .indices() + .prepareResizeIndex(INDEX_NAME, targetIndexName) + .setResizeType(resizeActionTuple.v1()) + .setSettings( + Settings.builder() + .put("index.number_of_replicas", 0) + .put("index.number_of_shards", resizeActionTuple.v2()) + .putNull("index.blocks.write") + .put(SETTING_REPLICATION_TYPE, replicationList.get(1)) + .build() + ) + .get(); + }; + executeTest(true, consumer, targetIndexName, documentCount); + } + + public void testMismatchingReplicationType_ResizeAction_NonStrictMode() throws Throwable { + final int initialShardCount = 2; + // Define resize action and target shard count. + List> resizeActionsList = new ArrayList<>(); + resizeActionsList.add(new Tuple<>(ResizeType.SPLIT, 2 * initialShardCount)); + resizeActionsList.add(new Tuple<>(ResizeType.SHRINK, SHARD_COUNT)); + resizeActionsList.add(new Tuple<>(ResizeType.CLONE, initialShardCount)); + + Tuple resizeActionTuple = resizeActionsList.get(random().nextInt(resizeActionsList.size())); + final String targetIndexName = resizeActionTuple.v1().name().toLowerCase(Locale.ROOT) + "-target"; + final int documentCount = scaledRandomIntBetween(1, 10); + + logger.info("--> Performing resize action {} with shard count {}", resizeActionTuple.v1(), resizeActionTuple.v2()); + + // Create an index using current cluster level settings + BiConsumer, List> consumer = (replicationList, dataNodesList) -> { + Settings indexSettings = Settings.builder() + .put(indexSettings()) + .put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, initialShardCount) + .put(SETTING_REPLICATION_TYPE, replicationList.get(0)) + .build(); + + createIndex(INDEX_NAME, indexSettings); + + for (int i = 0; i < documentCount; i++) { + client().prepareIndex(INDEX_NAME).setId(String.valueOf(i)).setSource("foo", "bar").get(); + } + + // Block writes + client().admin() + .indices() + .prepareUpdateSettings(INDEX_NAME) + .setSettings(Settings.builder().put("index.blocks.write", true)) + .get(); + ensureGreen(); + + try { + for (String node : dataNodesList) { + assertBusy(() -> { + assertHitCount( + client(node).prepareSearch(INDEX_NAME).setSize(100).setQuery(new TermsQueryBuilder("foo", "bar")).get(), + documentCount + ); + }, 30, TimeUnit.SECONDS); + } + } catch (Exception e) { + throw new RuntimeException(e); + } + + client().admin() + .indices() + .prepareResizeIndex(INDEX_NAME, targetIndexName) + .setResizeType(resizeActionTuple.v1()) + .setSettings( + Settings.builder() + .put("index.number_of_replicas", 0) + .put("index.number_of_shards", resizeActionTuple.v2()) + .putNull("index.blocks.write") + .put(SETTING_REPLICATION_TYPE, replicationList.get(1)) + .build() + ) + .get(); + }; + executeTest(false, consumer, targetIndexName, documentCount); + } + + // Creates a cluster with mis-matching cluster level and index level replication strategies and validates that index + // creation fails when cluster level setting `cluster.force.index.replication_type` is set to true and creation goes + // through when it is false. + private void executeTest( + boolean restrictIndexLevelReplicationTypeSetting, + BiConsumer, List> createIndexRunnable, + String targetIndexName, + int documentCount + ) throws Throwable { + // Generate mutually exclusive replication strategies at cluster and index level + List replicationStrategies = getRandomReplicationTypesAsList(); + ReplicationType clusterLevelReplication = replicationStrategies.get(0); + ReplicationType indexLevelReplication = replicationStrategies.get(1); + + Settings settings = Settings.builder() + .put(CLUSTER_SETTING_REPLICATION_TYPE, clusterLevelReplication) + .put(CLUSTER_RESTRICT_INDEX_REPLICATION_TYPE_SETTING.getKey(), restrictIndexLevelReplicationTypeSetting) + .build(); + internalCluster().startClusterManagerOnlyNode(settings); + final String dataNodeOne = internalCluster().startDataOnlyNode(settings); + final String dataNodeTwo = internalCluster().startDataOnlyNode(settings); + List dataNodes = Arrays.asList(dataNodeOne, dataNodeTwo); + + logger.info( + "--> Create index with cluster level setting {} and index level replication setting {}", + clusterLevelReplication, + indexLevelReplication + ); + + if (restrictIndexLevelReplicationTypeSetting) { + try { + createIndexRunnable.accept(replicationStrategies, dataNodes); + } catch (IllegalArgumentException exception) { + assertEquals(REPLICATION_MISMATCH_VALIDATION_ERROR, exception.getMessage()); + } + } else { + createIndexRunnable.accept(replicationStrategies, dataNodes); + ensureGreen(targetIndexName); + for (String node : dataNodes) { + assertBusy(() -> { + assertHitCount( + client(node).prepareSearch(targetIndexName).setSize(100).setQuery(new TermsQueryBuilder("foo", "bar")).get(), + documentCount + ); + }); + } + } + } + + /** + * Generate a list of ReplicationType with random ordering + * + * @return List of ReplicationType values + */ + private List getRandomReplicationTypesAsList() { + List replicationStrategies = List.of(ReplicationType.SEGMENT, ReplicationType.DOCUMENT); + int randomReplicationIndex = random().nextInt(replicationStrategies.size()); + ReplicationType clusterLevelReplication = replicationStrategies.get(randomReplicationIndex); + ReplicationType indexLevelReplication = replicationStrategies.get(1 - randomReplicationIndex); + return List.of(clusterLevelReplication, indexLevelReplication); + } } diff --git a/server/src/internalClusterTest/java/org/opensearch/snapshots/SegmentReplicationSnapshotIT.java b/server/src/internalClusterTest/java/org/opensearch/snapshots/SegmentReplicationSnapshotIT.java index c2ce7e48f92d2..5b26f9ef7ba0e 100644 --- a/server/src/internalClusterTest/java/org/opensearch/snapshots/SegmentReplicationSnapshotIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/snapshots/SegmentReplicationSnapshotIT.java @@ -31,6 +31,7 @@ import java.util.concurrent.TimeUnit; import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_REPLICATION_TYPE; +import static org.opensearch.indices.IndicesService.CLUSTER_RESTRICT_INDEX_REPLICATION_TYPE_SETTING; import static org.opensearch.indices.IndicesService.CLUSTER_SETTING_REPLICATION_TYPE; import static org.opensearch.indices.replication.SegmentReplicationBaseIT.waitForSearchableDocs; import static org.opensearch.test.hamcrest.OpenSearchAssertions.assertAcked; @@ -306,4 +307,71 @@ public void testSnapshotRestoreOnIndexWithSegRepClusterSetting() throws Exceptio IndicesService indicesService = internalCluster().getInstance(IndicesService.class); assertEquals(indicesService.indexService(index).getIndexSettings().isSegRepEnabled(), false); } + + /** + * 1. Create index in DOCUMENT replication type + * 2. Snapshot index + * 3. Add new set of nodes with `cluster.indices.replication.strategy` set to SEGMENT and `cluster.force.index.replication_type` + * set to true. + * 4. Perform restore on new set of nodes to validate restored index has `DOCUMENT` replication. + */ + public void testSnapshotRestoreOnRestrictReplicationSetting() throws Exception { + final int documentCount = scaledRandomIntBetween(1, 10); + String originalClusterManagerNode = internalCluster().startClusterManagerOnlyNode(); + + // Starting two nodes with primary and replica shards respectively. + final String primaryNode = internalCluster().startDataOnlyNode(); + prepareCreate( + INDEX_NAME, + Settings.builder() + // we want to override cluster replication setting by passing a index replication setting + .put(IndexMetadata.SETTING_REPLICATION_TYPE, ReplicationType.DOCUMENT) + .put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, REPLICA_COUNT) + ).get(); + ensureYellowAndNoInitializingShards(INDEX_NAME); + final String replicaNode = internalCluster().startDataOnlyNode(); + ensureGreen(INDEX_NAME); + + for (int i = 0; i < documentCount; i++) { + client().prepareIndex(INDEX_NAME).setId(String.valueOf(i)).setSource("foo", "bar").get(); + } + + createSnapshot(); + + // Delete index + assertAcked(client().admin().indices().delete(new DeleteIndexRequest(INDEX_NAME)).get()); + assertFalse("index [" + INDEX_NAME + "] should have been deleted", indexExists(INDEX_NAME)); + + // Start new set of nodes with cluster level replication type setting and restrict replication type setting. + Settings settings = Settings.builder() + .put(CLUSTER_SETTING_REPLICATION_TYPE, ReplicationType.SEGMENT) + .put(CLUSTER_RESTRICT_INDEX_REPLICATION_TYPE_SETTING.getKey(), true) + .build(); + String newClusterManagerNode = internalCluster().startClusterManagerOnlyNode(settings); + + // Remove older nodes + internalCluster().stopRandomNode(InternalTestCluster.nameFilter(originalClusterManagerNode)); + internalCluster().stopRandomNode(InternalTestCluster.nameFilter(primaryNode)); + internalCluster().stopRandomNode(InternalTestCluster.nameFilter(replicaNode)); + + String newPrimaryNode = internalCluster().startDataOnlyNode(settings); + String newReplicaNode = internalCluster().startDataOnlyNode(settings); + + RestoreSnapshotResponse restoreSnapshotResponse = restoreSnapshotWithSettings(restoreIndexSegRepSettings()); + + // Assertions + assertEquals(restoreSnapshotResponse.status(), RestStatus.ACCEPTED); + ensureGreen(RESTORED_INDEX_NAME); + logger.info("--> ClusterState {}", client().admin().cluster().prepareState().execute().actionGet().getState()); + 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.DOCUMENT.toString()); + + // Verify index setting isSegRepEnabled. + Index index = resolveIndex(RESTORED_INDEX_NAME); + IndicesService indicesService = internalCluster().getInstance(IndicesService.class); + assertEquals(indicesService.indexService(index).getIndexSettings().isSegRepEnabled(), false); + } } 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 bb1bf94f5e984..49ab6df1bea4a 100644 --- a/server/src/main/java/org/opensearch/cluster/metadata/MetadataCreateIndexService.java +++ b/server/src/main/java/org/opensearch/cluster/metadata/MetadataCreateIndexService.java @@ -907,6 +907,12 @@ static Settings aggregateIndexSettings( ); } + // Validate index settings here to validate replication type where replication type is updated via templates or + // resize index requests + List validationErrors = new ArrayList<>(); + validateIndexReplicationTypeSettings(indexSettingsBuilder.build(), clusterSettings).ifPresent(validationErrors::add); + validateErrors(request.index(), validationErrors); + Settings indexSettings = indexSettingsBuilder.build(); /* * We can not validate settings until we have applied templates, otherwise we do not know the actual settings @@ -1240,13 +1246,23 @@ private static void validateActiveShardCount(ActiveShardCount waitForActiveShard private void validate(CreateIndexClusterStateUpdateRequest request, ClusterState state) { validateIndexName(request.index(), state); - validateIndexSettings(request.index(), request.settings(), forbidPrivateIndexSettings); + validateIndexSettings(request.index(), request.settings(), forbidPrivateIndexSettings, false); } - public void validateIndexSettings(String indexName, final Settings settings, final boolean forbidPrivateIndexSettings) - throws IndexCreationException { - List validationErrors = getIndexSettingsValidationErrors(settings, forbidPrivateIndexSettings, indexName); + public void validateIndexSettings( + String indexName, + final Settings settings, + final boolean forbidPrivateIndexSettings, + final boolean ignoreRestrictReplicationTypeSetting + ) throws IndexCreationException { + List validationErrors = getIndexSettingsValidationErrors(settings, forbidPrivateIndexSettings, Optional.of(indexName)); + if (ignoreRestrictReplicationTypeSetting == false) { + validateIndexReplicationTypeSettings(settings, clusterService.getClusterSettings()).ifPresent(validationErrors::add); + } + validateErrors(indexName, validationErrors); + } + private static void validateErrors(String indexName, List validationErrors) { if (validationErrors.isEmpty() == false) { ValidationException validationException = new ValidationException(); validationException.addValidationErrors(validationErrors); @@ -1322,6 +1338,27 @@ private static List validateIndexCustomPath(Settings settings, @Nullable return validationErrors; } + /** + * Validates {@code index.replication.type} is matches with cluster level setting {@code cluster.indices.replication.strategy} + * when {@code cluster.force.index.replication_type} is set to true. + * + * @param requestSettings settings passed in during index create request + * @param clusterSettings cluster setting + */ + private static Optional validateIndexReplicationTypeSettings(Settings requestSettings, ClusterSettings clusterSettings) { + if (clusterSettings.get(IndicesService.CLUSTER_RESTRICT_INDEX_REPLICATION_TYPE_SETTING) + && requestSettings.hasValue(SETTING_REPLICATION_TYPE) + && requestSettings.get(INDEX_REPLICATION_TYPE_SETTING.getKey()) + .equals(clusterSettings.get(CLUSTER_REPLICATION_TYPE_SETTING).name()) == false) { + return Optional.of( + "index setting [index.replication.type] is not allowed to be set as [" + + IndicesService.CLUSTER_RESTRICT_INDEX_REPLICATION_TYPE_SETTING.getKey() + + "=true]" + ); + } + return Optional.empty(); + } + /** * Validates the settings and mappings for shrinking an index. * diff --git a/server/src/main/java/org/opensearch/common/settings/ClusterSettings.java b/server/src/main/java/org/opensearch/common/settings/ClusterSettings.java index ab0ea89f4734d..9179b8e2c6da7 100644 --- a/server/src/main/java/org/opensearch/common/settings/ClusterSettings.java +++ b/server/src/main/java/org/opensearch/common/settings/ClusterSettings.java @@ -701,7 +701,8 @@ public void apply(Settings value, Settings current, Settings previous) { AdmissionControlSettings.ADMISSION_CONTROL_TRANSPORT_LAYER_MODE, CpuBasedAdmissionControllerSettings.CPU_BASED_ADMISSION_CONTROLLER_TRANSPORT_LAYER_MODE, CpuBasedAdmissionControllerSettings.INDEXING_CPU_USAGE_LIMIT, - CpuBasedAdmissionControllerSettings.SEARCH_CPU_USAGE_LIMIT + CpuBasedAdmissionControllerSettings.SEARCH_CPU_USAGE_LIMIT, + IndicesService.CLUSTER_RESTRICT_INDEX_REPLICATION_TYPE_SETTING ) ) ); diff --git a/server/src/main/java/org/opensearch/indices/IndicesService.java b/server/src/main/java/org/opensearch/indices/IndicesService.java index 3d1794f8d3197..f8d237e003b1a 100644 --- a/server/src/main/java/org/opensearch/indices/IndicesService.java +++ b/server/src/main/java/org/opensearch/indices/IndicesService.java @@ -301,6 +301,18 @@ public class IndicesService extends AbstractLifecycleComponent Property.Final ); + /** + * This setting is used to prevents creation of indices where the 'index.replication.type' setting does not match + * with the replication type index setting, set at cluster level i.e. 'cluster.indices.replication.strategy'. + * If disabled, the replication type can be specified. + */ + public static final Setting CLUSTER_RESTRICT_INDEX_REPLICATION_TYPE_SETTING = Setting.boolSetting( + "cluster.force.index.replication_type", + false, + Property.NodeScope, + Property.Final + ); + /** * The node's settings. */ diff --git a/server/src/main/java/org/opensearch/snapshots/RestoreService.java b/server/src/main/java/org/opensearch/snapshots/RestoreService.java index 9d2c7eb882fa1..e6cd8fac65fd7 100644 --- a/server/src/main/java/org/opensearch/snapshots/RestoreService.java +++ b/server/src/main/java/org/opensearch/snapshots/RestoreService.java @@ -428,7 +428,12 @@ public ClusterState execute(ClusterState currentState) { boolean isHidden = IndexMetadata.INDEX_HIDDEN_SETTING.get(snapshotIndexMetadata.getSettings()); createIndexService.validateIndexName(renamedIndexName, currentState); createIndexService.validateDotIndex(renamedIndexName, isHidden); - createIndexService.validateIndexSettings(renamedIndexName, snapshotIndexMetadata.getSettings(), false); + createIndexService.validateIndexSettings( + renamedIndexName, + snapshotIndexMetadata.getSettings(), + false, + true + ); MetadataCreateIndexService.validateRefreshIntervalSettings( snapshotIndexMetadata.getSettings(), clusterSettings From 20cf1299a4cf0227d47389a179c45ca79d3fe071 Mon Sep 17 00:00:00 2001 From: Suraj Singh Date: Fri, 15 Dec 2023 13:56:58 -0800 Subject: [PATCH 2/6] Block restore on mis-matching replication type setting Signed-off-by: Suraj Singh --- .../SegmentReplicationClusterSettingIT.java | 4 +-- .../SegmentReplicationSnapshotIT.java | 29 ++++++++----------- .../metadata/MetadataCreateIndexService.java | 22 +++++--------- .../common/settings/ClusterSettings.java | 2 +- .../opensearch/indices/IndicesService.java | 2 +- .../opensearch/snapshots/RestoreService.java | 7 +---- 6 files changed, 24 insertions(+), 42 deletions(-) diff --git a/server/src/internalClusterTest/java/org/opensearch/indices/replication/SegmentReplicationClusterSettingIT.java b/server/src/internalClusterTest/java/org/opensearch/indices/replication/SegmentReplicationClusterSettingIT.java index 91a5d42ae07a8..76b61a37891f8 100644 --- a/server/src/internalClusterTest/java/org/opensearch/indices/replication/SegmentReplicationClusterSettingIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/indices/replication/SegmentReplicationClusterSettingIT.java @@ -31,7 +31,7 @@ import java.util.function.BiConsumer; import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_REPLICATION_TYPE; -import static org.opensearch.indices.IndicesService.CLUSTER_RESTRICT_INDEX_REPLICATION_TYPE_SETTING; +import static org.opensearch.indices.IndicesService.CLUSTER_FORCE_INDEX_REPLICATION_TYPE_SETTING; import static org.opensearch.indices.IndicesService.CLUSTER_SETTING_REPLICATION_TYPE; import static org.opensearch.test.hamcrest.OpenSearchAssertions.assertHitCount; import static org.hamcrest.Matchers.hasSize; @@ -346,7 +346,7 @@ private void executeTest( Settings settings = Settings.builder() .put(CLUSTER_SETTING_REPLICATION_TYPE, clusterLevelReplication) - .put(CLUSTER_RESTRICT_INDEX_REPLICATION_TYPE_SETTING.getKey(), restrictIndexLevelReplicationTypeSetting) + .put(CLUSTER_FORCE_INDEX_REPLICATION_TYPE_SETTING.getKey(), restrictIndexLevelReplicationTypeSetting) .build(); internalCluster().startClusterManagerOnlyNode(settings); final String dataNodeOne = internalCluster().startDataOnlyNode(settings); diff --git a/server/src/internalClusterTest/java/org/opensearch/snapshots/SegmentReplicationSnapshotIT.java b/server/src/internalClusterTest/java/org/opensearch/snapshots/SegmentReplicationSnapshotIT.java index 5b26f9ef7ba0e..6d06bb5d866d3 100644 --- a/server/src/internalClusterTest/java/org/opensearch/snapshots/SegmentReplicationSnapshotIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/snapshots/SegmentReplicationSnapshotIT.java @@ -31,7 +31,7 @@ import java.util.concurrent.TimeUnit; import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_REPLICATION_TYPE; -import static org.opensearch.indices.IndicesService.CLUSTER_RESTRICT_INDEX_REPLICATION_TYPE_SETTING; +import static org.opensearch.indices.IndicesService.CLUSTER_FORCE_INDEX_REPLICATION_TYPE_SETTING; import static org.opensearch.indices.IndicesService.CLUSTER_SETTING_REPLICATION_TYPE; import static org.opensearch.indices.replication.SegmentReplicationBaseIT.waitForSearchableDocs; import static org.opensearch.test.hamcrest.OpenSearchAssertions.assertAcked; @@ -48,6 +48,9 @@ public class SegmentReplicationSnapshotIT extends AbstractSnapshotIntegTestCase private static final String REPOSITORY_NAME = "test-segrep-repo"; private static final String SNAPSHOT_NAME = "test-segrep-snapshot"; + protected static final String REPLICATION_MISMATCH_VALIDATION_ERROR = + "Validation Failed: 1: index setting [index.replication.type] is not allowed to be set as [cluster.force.index.replication_type=true];"; + public Settings segRepEnableIndexSettings() { return getShardSettings().put(IndexMetadata.SETTING_REPLICATION_TYPE, ReplicationType.SEGMENT).build(); } @@ -327,6 +330,7 @@ public void testSnapshotRestoreOnRestrictReplicationSetting() throws Exception { // we want to override cluster replication setting by passing a index replication setting .put(IndexMetadata.SETTING_REPLICATION_TYPE, ReplicationType.DOCUMENT) .put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, REPLICA_COUNT) + .put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, SHARD_COUNT) ).get(); ensureYellowAndNoInitializingShards(INDEX_NAME); final String replicaNode = internalCluster().startDataOnlyNode(); @@ -345,8 +349,10 @@ public void testSnapshotRestoreOnRestrictReplicationSetting() throws Exception { // Start new set of nodes with cluster level replication type setting and restrict replication type setting. Settings settings = Settings.builder() .put(CLUSTER_SETTING_REPLICATION_TYPE, ReplicationType.SEGMENT) - .put(CLUSTER_RESTRICT_INDEX_REPLICATION_TYPE_SETTING.getKey(), true) + .put(CLUSTER_FORCE_INDEX_REPLICATION_TYPE_SETTING.getKey(), true) .build(); + + // Start new cluster manager node String newClusterManagerNode = internalCluster().startClusterManagerOnlyNode(settings); // Remove older nodes @@ -357,21 +363,10 @@ public void testSnapshotRestoreOnRestrictReplicationSetting() throws Exception { String newPrimaryNode = internalCluster().startDataOnlyNode(settings); String newReplicaNode = internalCluster().startDataOnlyNode(settings); - RestoreSnapshotResponse restoreSnapshotResponse = restoreSnapshotWithSettings(restoreIndexSegRepSettings()); - - // Assertions - assertEquals(restoreSnapshotResponse.status(), RestStatus.ACCEPTED); - ensureGreen(RESTORED_INDEX_NAME); - logger.info("--> ClusterState {}", client().admin().cluster().prepareState().execute().actionGet().getState()); - 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.DOCUMENT.toString()); + // Perform snapshot restore + logger.info("--> Performing snapshot restore to target index"); - // Verify index setting isSegRepEnabled. - Index index = resolveIndex(RESTORED_INDEX_NAME); - IndicesService indicesService = internalCluster().getInstance(IndicesService.class); - assertEquals(indicesService.indexService(index).getIndexSettings().isSegRepEnabled(), false); + IllegalArgumentException exception = expectThrows(IllegalArgumentException.class, () -> restoreSnapshotWithSettings(null)); + 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 index 49ab6df1bea4a..73dbc6b3fe48a 100644 --- a/server/src/main/java/org/opensearch/cluster/metadata/MetadataCreateIndexService.java +++ b/server/src/main/java/org/opensearch/cluster/metadata/MetadataCreateIndexService.java @@ -907,8 +907,6 @@ static Settings aggregateIndexSettings( ); } - // Validate index settings here to validate replication type where replication type is updated via templates or - // resize index requests List validationErrors = new ArrayList<>(); validateIndexReplicationTypeSettings(indexSettingsBuilder.build(), clusterSettings).ifPresent(validationErrors::add); validateErrors(request.index(), validationErrors); @@ -1246,19 +1244,13 @@ private static void validateActiveShardCount(ActiveShardCount waitForActiveShard private void validate(CreateIndexClusterStateUpdateRequest request, ClusterState state) { validateIndexName(request.index(), state); - validateIndexSettings(request.index(), request.settings(), forbidPrivateIndexSettings, false); + validateIndexSettings(request.index(), request.settings(), forbidPrivateIndexSettings); } - public void validateIndexSettings( - String indexName, - final Settings settings, - final boolean forbidPrivateIndexSettings, - final boolean ignoreRestrictReplicationTypeSetting - ) throws IndexCreationException { - List validationErrors = getIndexSettingsValidationErrors(settings, forbidPrivateIndexSettings, Optional.of(indexName)); - if (ignoreRestrictReplicationTypeSetting == false) { - validateIndexReplicationTypeSettings(settings, clusterService.getClusterSettings()).ifPresent(validationErrors::add); - } + public void validateIndexSettings(String indexName, final Settings settings, final boolean forbidPrivateIndexSettings) + throws IndexCreationException { + List validationErrors = getIndexSettingsValidationErrors(settings, forbidPrivateIndexSettings, indexName); + validateIndexReplicationTypeSettings(settings, clusterService.getClusterSettings()).ifPresent(validationErrors::add); validateErrors(indexName, validationErrors); } @@ -1346,13 +1338,13 @@ private static List validateIndexCustomPath(Settings settings, @Nullable * @param clusterSettings cluster setting */ private static Optional validateIndexReplicationTypeSettings(Settings requestSettings, ClusterSettings clusterSettings) { - if (clusterSettings.get(IndicesService.CLUSTER_RESTRICT_INDEX_REPLICATION_TYPE_SETTING) + if (clusterSettings.get(IndicesService.CLUSTER_FORCE_INDEX_REPLICATION_TYPE_SETTING) && requestSettings.hasValue(SETTING_REPLICATION_TYPE) && requestSettings.get(INDEX_REPLICATION_TYPE_SETTING.getKey()) .equals(clusterSettings.get(CLUSTER_REPLICATION_TYPE_SETTING).name()) == false) { return Optional.of( "index setting [index.replication.type] is not allowed to be set as [" - + IndicesService.CLUSTER_RESTRICT_INDEX_REPLICATION_TYPE_SETTING.getKey() + + IndicesService.CLUSTER_FORCE_INDEX_REPLICATION_TYPE_SETTING.getKey() + "=true]" ); } diff --git a/server/src/main/java/org/opensearch/common/settings/ClusterSettings.java b/server/src/main/java/org/opensearch/common/settings/ClusterSettings.java index 9179b8e2c6da7..1870397f44d59 100644 --- a/server/src/main/java/org/opensearch/common/settings/ClusterSettings.java +++ b/server/src/main/java/org/opensearch/common/settings/ClusterSettings.java @@ -702,7 +702,7 @@ public void apply(Settings value, Settings current, Settings previous) { CpuBasedAdmissionControllerSettings.CPU_BASED_ADMISSION_CONTROLLER_TRANSPORT_LAYER_MODE, CpuBasedAdmissionControllerSettings.INDEXING_CPU_USAGE_LIMIT, CpuBasedAdmissionControllerSettings.SEARCH_CPU_USAGE_LIMIT, - IndicesService.CLUSTER_RESTRICT_INDEX_REPLICATION_TYPE_SETTING + IndicesService.CLUSTER_FORCE_INDEX_REPLICATION_TYPE_SETTING ) ) ); diff --git a/server/src/main/java/org/opensearch/indices/IndicesService.java b/server/src/main/java/org/opensearch/indices/IndicesService.java index f8d237e003b1a..e09423a250c89 100644 --- a/server/src/main/java/org/opensearch/indices/IndicesService.java +++ b/server/src/main/java/org/opensearch/indices/IndicesService.java @@ -306,7 +306,7 @@ public class IndicesService extends AbstractLifecycleComponent * with the replication type index setting, set at cluster level i.e. 'cluster.indices.replication.strategy'. * If disabled, the replication type can be specified. */ - public static final Setting CLUSTER_RESTRICT_INDEX_REPLICATION_TYPE_SETTING = Setting.boolSetting( + public static final Setting CLUSTER_FORCE_INDEX_REPLICATION_TYPE_SETTING = Setting.boolSetting( "cluster.force.index.replication_type", false, Property.NodeScope, diff --git a/server/src/main/java/org/opensearch/snapshots/RestoreService.java b/server/src/main/java/org/opensearch/snapshots/RestoreService.java index e6cd8fac65fd7..9d2c7eb882fa1 100644 --- a/server/src/main/java/org/opensearch/snapshots/RestoreService.java +++ b/server/src/main/java/org/opensearch/snapshots/RestoreService.java @@ -428,12 +428,7 @@ public ClusterState execute(ClusterState currentState) { boolean isHidden = IndexMetadata.INDEX_HIDDEN_SETTING.get(snapshotIndexMetadata.getSettings()); createIndexService.validateIndexName(renamedIndexName, currentState); createIndexService.validateDotIndex(renamedIndexName, isHidden); - createIndexService.validateIndexSettings( - renamedIndexName, - snapshotIndexMetadata.getSettings(), - false, - true - ); + createIndexService.validateIndexSettings(renamedIndexName, snapshotIndexMetadata.getSettings(), false); MetadataCreateIndexService.validateRefreshIntervalSettings( snapshotIndexMetadata.getSettings(), clusterSettings From 39d47bec31b8a9f60bca02fc710d1c6fbb9aa84e Mon Sep 17 00:00:00 2001 From: Suraj Singh Date: Tue, 19 Dec 2023 18:36:03 -0800 Subject: [PATCH 3/6] Address review comments and rebase Signed-off-by: Suraj Singh --- CHANGELOG.md | 2 +- .../SegmentReplicationClusterSettingIT.java | 106 +----------------- .../SegmentReplicationSnapshotIT.java | 4 +- .../metadata/MetadataCreateIndexService.java | 2 +- .../opensearch/indices/IndicesService.java | 9 +- .../MetadataCreateIndexServiceTests.java | 104 +++++++++++++++++ 6 files changed, 115 insertions(+), 112 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 50be6ee2744ef..d1b8aeaed29c2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -122,7 +122,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - [BWC and API enforcement] Introduce checks for enforcing the API restrictions ([#11175](https://github.com/opensearch-project/OpenSearch/pull/11175)) - Create separate transport action for render search template action ([#11170](https://github.com/opensearch-project/OpenSearch/pull/11170)) - Add additional handling in SearchTemplateRequest when simulate is set to true ([#11591](https://github.com/opensearch-project/OpenSearch/pull/11591)) -- Introduce cluster level setting `cluster.force.index.replication_type` that prevents new create index requests when replication type mis-matches with cluster level replication `cluster.indices.replication.strategy`([#11583](https://github.com/opensearch-project/OpenSearch/pull/11583)) +- Introduce cluster level setting `cluster.force.index.replication.type` to prevent replication type setting override during index creations([#11583](https://github.com/opensearch-project/OpenSearch/pull/11583)) ### Dependencies - Bump Lucene from 9.7.0 to 9.8.0 ([10276](https://github.com/opensearch-project/OpenSearch/pull/10276)) diff --git a/server/src/internalClusterTest/java/org/opensearch/indices/replication/SegmentReplicationClusterSettingIT.java b/server/src/internalClusterTest/java/org/opensearch/indices/replication/SegmentReplicationClusterSettingIT.java index 76b61a37891f8..bbdfd9cd0ff9c 100644 --- a/server/src/internalClusterTest/java/org/opensearch/indices/replication/SegmentReplicationClusterSettingIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/indices/replication/SegmentReplicationClusterSettingIT.java @@ -45,7 +45,7 @@ public class SegmentReplicationClusterSettingIT extends OpenSearchIntegTestCase protected static final int REPLICA_COUNT = 1; protected static final String REPLICATION_MISMATCH_VALIDATION_ERROR = - "Validation Failed: 1: index setting [index.replication.type] is not allowed to be set as [cluster.force.index.replication_type=true];"; + "Validation Failed: 1: index setting [index.replication.type] is not allowed to be set as [cluster.force.index.replication.type=true];"; @Override public Settings indexSettings() { @@ -142,18 +142,6 @@ public void testDifferentReplicationTypes_CreateIndex_StrictMode() throws Throwa executeTest(true, consumer, INDEX_NAME, documentCount); } - public void testDifferentReplicationTypes_CreateIndex_NonStrictMode() throws Throwable { - final int documentCount = scaledRandomIntBetween(1, 10); - BiConsumer, List> consumer = (replicationList, dataNodesList) -> { - Settings indexSettings = Settings.builder().put(indexSettings()).put(SETTING_REPLICATION_TYPE, replicationList.get(1)).build(); - createIndex(INDEX_NAME, indexSettings); - for (int i = 0; i < documentCount; i++) { - client().prepareIndex(INDEX_NAME).setId(String.valueOf(i)).setSource("foo", "bar").get(); - } - }; - executeTest(false, consumer, INDEX_NAME, documentCount); - } - public void testDifferentReplicationTypes_IndexTemplates_StrictMode() throws Throwable { final int documentCount = scaledRandomIntBetween(1, 10); @@ -174,30 +162,6 @@ public void testDifferentReplicationTypes_IndexTemplates_StrictMode() throws Thr executeTest(true, consumer, INDEX_NAME, documentCount); } - public void testDifferentReplicationTypes_IndexTemplates_NonStrictMode() throws Throwable { - final int documentCount = scaledRandomIntBetween(1, 10); - - // Create an index using current cluster level settings - BiConsumer, List> consumer = (replicationList, dataNodesList) -> { - client().admin() - .indices() - .preparePutTemplate("template_1") - .setPatterns(Collections.singletonList("test-idx*")) - .setSettings(Settings.builder().put(indexSettings()).put(SETTING_REPLICATION_TYPE, replicationList.get(1)).build()) - .setOrder(0) - .get(); - - GetIndexTemplatesResponse response = client().admin().indices().prepareGetTemplates().get(); - assertThat(response.getIndexTemplates(), hasSize(1)); - - createIndex(INDEX_NAME); - for (int i = 0; i < documentCount; i++) { - client().prepareIndex(INDEX_NAME).setId(String.valueOf(i)).setSource("foo", "bar").get(); - } - }; - executeTest(false, consumer, INDEX_NAME, documentCount); - } - public void testMismatchingReplicationType_ResizeAction_StrictMode() throws Throwable { // Define resize action and target shard count. List> resizeActionsList = new ArrayList<>(); @@ -264,74 +228,8 @@ public void testMismatchingReplicationType_ResizeAction_StrictMode() throws Thro executeTest(true, consumer, targetIndexName, documentCount); } - public void testMismatchingReplicationType_ResizeAction_NonStrictMode() throws Throwable { - final int initialShardCount = 2; - // Define resize action and target shard count. - List> resizeActionsList = new ArrayList<>(); - resizeActionsList.add(new Tuple<>(ResizeType.SPLIT, 2 * initialShardCount)); - resizeActionsList.add(new Tuple<>(ResizeType.SHRINK, SHARD_COUNT)); - resizeActionsList.add(new Tuple<>(ResizeType.CLONE, initialShardCount)); - - Tuple resizeActionTuple = resizeActionsList.get(random().nextInt(resizeActionsList.size())); - final String targetIndexName = resizeActionTuple.v1().name().toLowerCase(Locale.ROOT) + "-target"; - final int documentCount = scaledRandomIntBetween(1, 10); - - logger.info("--> Performing resize action {} with shard count {}", resizeActionTuple.v1(), resizeActionTuple.v2()); - - // Create an index using current cluster level settings - BiConsumer, List> consumer = (replicationList, dataNodesList) -> { - Settings indexSettings = Settings.builder() - .put(indexSettings()) - .put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, initialShardCount) - .put(SETTING_REPLICATION_TYPE, replicationList.get(0)) - .build(); - - createIndex(INDEX_NAME, indexSettings); - - for (int i = 0; i < documentCount; i++) { - client().prepareIndex(INDEX_NAME).setId(String.valueOf(i)).setSource("foo", "bar").get(); - } - - // Block writes - client().admin() - .indices() - .prepareUpdateSettings(INDEX_NAME) - .setSettings(Settings.builder().put("index.blocks.write", true)) - .get(); - ensureGreen(); - - try { - for (String node : dataNodesList) { - assertBusy(() -> { - assertHitCount( - client(node).prepareSearch(INDEX_NAME).setSize(100).setQuery(new TermsQueryBuilder("foo", "bar")).get(), - documentCount - ); - }, 30, TimeUnit.SECONDS); - } - } catch (Exception e) { - throw new RuntimeException(e); - } - - client().admin() - .indices() - .prepareResizeIndex(INDEX_NAME, targetIndexName) - .setResizeType(resizeActionTuple.v1()) - .setSettings( - Settings.builder() - .put("index.number_of_replicas", 0) - .put("index.number_of_shards", resizeActionTuple.v2()) - .putNull("index.blocks.write") - .put(SETTING_REPLICATION_TYPE, replicationList.get(1)) - .build() - ) - .get(); - }; - executeTest(false, consumer, targetIndexName, documentCount); - } - // Creates a cluster with mis-matching cluster level and index level replication strategies and validates that index - // creation fails when cluster level setting `cluster.force.index.replication_type` is set to true and creation goes + // creation fails when cluster level setting `cluster.force.index.replication.type` is set to true and creation goes // through when it is false. private void executeTest( boolean restrictIndexLevelReplicationTypeSetting, diff --git a/server/src/internalClusterTest/java/org/opensearch/snapshots/SegmentReplicationSnapshotIT.java b/server/src/internalClusterTest/java/org/opensearch/snapshots/SegmentReplicationSnapshotIT.java index 6d06bb5d866d3..7013d0dbc5003 100644 --- a/server/src/internalClusterTest/java/org/opensearch/snapshots/SegmentReplicationSnapshotIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/snapshots/SegmentReplicationSnapshotIT.java @@ -49,7 +49,7 @@ public class SegmentReplicationSnapshotIT extends AbstractSnapshotIntegTestCase private static final String SNAPSHOT_NAME = "test-segrep-snapshot"; protected static final String REPLICATION_MISMATCH_VALIDATION_ERROR = - "Validation Failed: 1: index setting [index.replication.type] is not allowed to be set as [cluster.force.index.replication_type=true];"; + "Validation Failed: 1: index setting [index.replication.type] is not allowed to be set as [cluster.force.index.replication.type=true];"; public Settings segRepEnableIndexSettings() { return getShardSettings().put(IndexMetadata.SETTING_REPLICATION_TYPE, ReplicationType.SEGMENT).build(); @@ -314,7 +314,7 @@ public void testSnapshotRestoreOnIndexWithSegRepClusterSetting() throws Exceptio /** * 1. Create index in DOCUMENT replication type * 2. Snapshot index - * 3. Add new set of nodes with `cluster.indices.replication.strategy` set to SEGMENT and `cluster.force.index.replication_type` + * 3. Add new set of nodes with `cluster.indices.replication.strategy` set to SEGMENT and `cluster.force.index.replication.type` * set to true. * 4. Perform restore on new set of nodes to validate restored index has `DOCUMENT` replication. */ 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 73dbc6b3fe48a..bb25193dd858e 100644 --- a/server/src/main/java/org/opensearch/cluster/metadata/MetadataCreateIndexService.java +++ b/server/src/main/java/org/opensearch/cluster/metadata/MetadataCreateIndexService.java @@ -1332,7 +1332,7 @@ private static List validateIndexCustomPath(Settings settings, @Nullable /** * Validates {@code index.replication.type} is matches with cluster level setting {@code cluster.indices.replication.strategy} - * when {@code cluster.force.index.replication_type} is set to true. + * when {@code cluster.force.index.replication.type} is set to true. * * @param requestSettings settings passed in during index create request * @param clusterSettings cluster setting diff --git a/server/src/main/java/org/opensearch/indices/IndicesService.java b/server/src/main/java/org/opensearch/indices/IndicesService.java index e09423a250c89..c729c52cc7675 100644 --- a/server/src/main/java/org/opensearch/indices/IndicesService.java +++ b/server/src/main/java/org/opensearch/indices/IndicesService.java @@ -302,12 +302,13 @@ public class IndicesService extends AbstractLifecycleComponent ); /** - * This setting is used to prevents creation of indices where the 'index.replication.type' setting does not match - * with the replication type index setting, set at cluster level i.e. 'cluster.indices.replication.strategy'. - * If disabled, the replication type can be specified. + * If enabled, this setting enforces that indexes will be created with a replication type matching the cluster setting + * defined in cluster.indices.replication.strategy by rejecting any request that specifies a replication type that + * does not match the cluster setting. If disabled, a user can choose a replication type on a per-index basis using + * the index.replication.type setting. */ public static final Setting CLUSTER_FORCE_INDEX_REPLICATION_TYPE_SETTING = Setting.boolSetting( - "cluster.force.index.replication_type", + "cluster.force.index.replication.type", false, Property.NodeScope, Property.Final diff --git a/server/src/test/java/org/opensearch/cluster/metadata/MetadataCreateIndexServiceTests.java b/server/src/test/java/org/opensearch/cluster/metadata/MetadataCreateIndexServiceTests.java index c4a782209421b..83bf233d4940e 100644 --- a/server/src/test/java/org/opensearch/cluster/metadata/MetadataCreateIndexServiceTests.java +++ b/server/src/test/java/org/opensearch/cluster/metadata/MetadataCreateIndexServiceTests.java @@ -71,6 +71,7 @@ import org.opensearch.index.mapper.MapperService; import org.opensearch.index.query.QueryShardContext; import org.opensearch.index.translog.Translog; +import org.opensearch.indices.IndexCreationException; import org.opensearch.indices.IndicesService; import org.opensearch.indices.InvalidAliasNameException; import org.opensearch.indices.InvalidIndexNameException; @@ -137,6 +138,7 @@ import static org.opensearch.index.IndexSettings.INDEX_SOFT_DELETES_SETTING; import static org.opensearch.index.IndexSettings.INDEX_TRANSLOG_DURABILITY_SETTING; import static org.opensearch.indices.IndicesService.CLUSTER_DEFAULT_INDEX_REFRESH_INTERVAL_SETTING; +import static org.opensearch.indices.IndicesService.CLUSTER_FORCE_INDEX_REPLICATION_TYPE_SETTING; import static org.opensearch.indices.IndicesService.CLUSTER_MINIMUM_INDEX_REFRESH_INTERVAL_SETTING; import static org.opensearch.indices.IndicesService.CLUSTER_REMOTE_INDEX_RESTRICT_ASYNC_DURABILITY_SETTING; import static org.opensearch.indices.IndicesService.CLUSTER_REPLICATION_TYPE_SETTING; @@ -166,6 +168,9 @@ public class MetadataCreateIndexServiceTests extends OpenSearchTestCase { private static final String translogRepositoryNameAttributeKey = NODE_ATTRIBUTES.getKey() + REMOTE_STORE_TRANSLOG_REPOSITORY_NAME_ATTRIBUTE_KEY; + final String REPLICATION_MISMATCH_VALIDATION_ERROR = + "Validation Failed: 1: index setting [index.replication.type] is not allowed to be set as [cluster.force.index.replication.type=true];"; + @Before public void setup() throws Exception { super.setUp(); @@ -1239,6 +1244,105 @@ public void testIndexTemplateReplicationType() { assertEquals(ReplicationType.SEGMENT.toString(), indexSettings.get(INDEX_REPLICATION_TYPE_SETTING.getKey())); } + public void testClusterForceReplicationTypeInAggregateSettings() { + Settings settings = Settings.builder() + .put(CLUSTER_REPLICATION_TYPE_SETTING.getKey(), ReplicationType.SEGMENT) + .put(CLUSTER_FORCE_INDEX_REPLICATION_TYPE_SETTING.getKey(), true) + .build(); + ClusterSettings clusterSettings = new ClusterSettings(settings, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS); + Settings matchingReplicationIndexSettings = Settings.builder() + .put(INDEX_REPLICATION_TYPE_SETTING.getKey(), ReplicationType.DOCUMENT) + .build(); + request = new CreateIndexClusterStateUpdateRequest("create index", "test", "test"); + request.settings(matchingReplicationIndexSettings); + IndexCreationException exception = expectThrows( + IndexCreationException.class, + () -> aggregateIndexSettings( + ClusterState.EMPTY_STATE, + request, + Settings.EMPTY, + null, + Settings.EMPTY, + IndexScopedSettings.DEFAULT_SCOPED_SETTINGS, + randomShardLimitService(), + Collections.emptySet(), + clusterSettings + ) + ); + assertEquals(REPLICATION_MISMATCH_VALIDATION_ERROR, exception.getCause().getMessage()); + + Settings nonMatchingReplicationIndexSettings = Settings.builder() + .put(INDEX_REPLICATION_TYPE_SETTING.getKey(), ReplicationType.SEGMENT) + .build(); + request.settings(nonMatchingReplicationIndexSettings); + Settings aggregateIndexSettings = aggregateIndexSettings( + ClusterState.EMPTY_STATE, + request, + Settings.EMPTY, + null, + Settings.EMPTY, + IndexScopedSettings.DEFAULT_SCOPED_SETTINGS, + randomShardLimitService(), + Collections.emptySet(), + clusterSettings + ); + assertEquals(ReplicationType.SEGMENT.toString(), aggregateIndexSettings.get(INDEX_REPLICATION_TYPE_SETTING.getKey())); + } + + public void testClusterForceReplicationTypeInValidateIndexSettings() { + ClusterService clusterService = mock(ClusterService.class); + Metadata metadata = Metadata.builder() + .transientSettings(Settings.builder().put(Metadata.DEFAULT_REPLICA_COUNT_SETTING.getKey(), 1).build()) + .build(); + ClusterState clusterState = ClusterState.builder(org.opensearch.cluster.ClusterName.CLUSTER_NAME_SETTING.getDefault(Settings.EMPTY)) + .metadata(metadata) + .build(); + ThreadPool threadPool = new TestThreadPool(getTestName()); + // Enforce cluster level replication type setting + final Settings forceClusterSettingEnabled = Settings.builder() + .put(CLUSTER_REPLICATION_TYPE_SETTING.getKey(), ReplicationType.SEGMENT) + .put(CLUSTER_FORCE_INDEX_REPLICATION_TYPE_SETTING.getKey(), true) + .build(); + ClusterSettings clusterSettings = new ClusterSettings(forceClusterSettingEnabled, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS); + when(clusterService.getSettings()).thenReturn(forceClusterSettingEnabled); + when(clusterService.getClusterSettings()).thenReturn(clusterSettings); + when(clusterService.state()).thenReturn(clusterState); + + final MetadataCreateIndexService checkerService = new MetadataCreateIndexService( + forceClusterSettingEnabled, + clusterService, + null, + null, + null, + createTestShardLimitService(randomIntBetween(1, 1000), false, clusterService), + new Environment(Settings.builder().put("path.home", "dummy").build(), null), + IndexScopedSettings.DEFAULT_SCOPED_SETTINGS, + threadPool, + null, + new SystemIndices(Collections.emptyMap()), + true, + new AwarenessReplicaBalance(forceClusterSettingEnabled, clusterService.getClusterSettings()) + ); + // Use DOCUMENT replication type setting for index creation + final Settings indexSettings = Settings.builder().put(INDEX_REPLICATION_TYPE_SETTING.getKey(), ReplicationType.DOCUMENT).build(); + + IndexCreationException exception = expectThrows( + IndexCreationException.class, + () -> checkerService.validateIndexSettings("test", indexSettings, false) + ); + assertEquals(REPLICATION_MISMATCH_VALIDATION_ERROR, exception.getCause().getMessage()); + + // Cluster level replication type setting not enforced + final Settings forceClusterSettingDisabled = Settings.builder() + .put(CLUSTER_REPLICATION_TYPE_SETTING.getKey(), ReplicationType.SEGMENT) + .put(CLUSTER_FORCE_INDEX_REPLICATION_TYPE_SETTING.getKey(), false) + .build(); + clusterSettings = new ClusterSettings(forceClusterSettingDisabled, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS); + when(clusterService.getClusterSettings()).thenReturn(clusterSettings); + checkerService.validateIndexSettings("test", indexSettings, false); + threadPool.shutdown(); + } + public void testRemoteStoreNoUserOverrideExceptReplicationTypeSegmentIndexSettings() { Settings settings = Settings.builder() .put(CLUSTER_REPLICATION_TYPE_SETTING.getKey(), ReplicationType.DOCUMENT) From 81d63bacbc1e82b35e8e703bd9da8e2e64665b0e Mon Sep 17 00:00:00 2001 From: Suraj Singh Date: Wed, 20 Dec 2023 11:21:08 -0800 Subject: [PATCH 4/6] Address review comments Signed-off-by: Suraj Singh --- CHANGELOG.md | 2 +- .../SegmentReplicationClusterSettingIT.java | 200 +++++++----------- .../SegmentReplicationSnapshotIT.java | 8 +- .../metadata/MetadataCreateIndexService.java | 6 +- .../common/settings/ClusterSettings.java | 2 +- .../opensearch/indices/IndicesService.java | 4 +- .../MetadataCreateIndexServiceTests.java | 10 +- 7 files changed, 96 insertions(+), 136 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d1b8aeaed29c2..ca10ecf57e2b6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -122,7 +122,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - [BWC and API enforcement] Introduce checks for enforcing the API restrictions ([#11175](https://github.com/opensearch-project/OpenSearch/pull/11175)) - Create separate transport action for render search template action ([#11170](https://github.com/opensearch-project/OpenSearch/pull/11170)) - Add additional handling in SearchTemplateRequest when simulate is set to true ([#11591](https://github.com/opensearch-project/OpenSearch/pull/11591)) -- Introduce cluster level setting `cluster.force.index.replication.type` to prevent replication type setting override during index creations([#11583](https://github.com/opensearch-project/OpenSearch/pull/11583)) +- Introduce cluster level setting `cluster.index.restrict.replication.type` to prevent replication type setting override during index creations([#11583](https://github.com/opensearch-project/OpenSearch/pull/11583)) ### Dependencies - Bump Lucene from 9.7.0 to 9.8.0 ([10276](https://github.com/opensearch-project/OpenSearch/pull/10276)) diff --git a/server/src/internalClusterTest/java/org/opensearch/indices/replication/SegmentReplicationClusterSettingIT.java b/server/src/internalClusterTest/java/org/opensearch/indices/replication/SegmentReplicationClusterSettingIT.java index bbdfd9cd0ff9c..0f542a70ea92e 100644 --- a/server/src/internalClusterTest/java/org/opensearch/indices/replication/SegmentReplicationClusterSettingIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/indices/replication/SegmentReplicationClusterSettingIT.java @@ -17,23 +17,18 @@ import org.opensearch.common.settings.Settings; import org.opensearch.core.index.Index; import org.opensearch.index.IndexModule; -import org.opensearch.index.query.TermsQueryBuilder; import org.opensearch.indices.IndicesService; import org.opensearch.indices.replication.common.ReplicationType; import org.opensearch.test.OpenSearchIntegTestCase; import java.util.ArrayList; -import java.util.Arrays; import java.util.Collections; import java.util.List; import java.util.Locale; -import java.util.concurrent.TimeUnit; -import java.util.function.BiConsumer; import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_REPLICATION_TYPE; -import static org.opensearch.indices.IndicesService.CLUSTER_FORCE_INDEX_REPLICATION_TYPE_SETTING; +import static org.opensearch.indices.IndicesService.CLUSTER_INDEX_RESTRICT_REPLICATION_TYPE_SETTING; import static org.opensearch.indices.IndicesService.CLUSTER_SETTING_REPLICATION_TYPE; -import static org.opensearch.test.hamcrest.OpenSearchAssertions.assertHitCount; import static org.hamcrest.Matchers.hasSize; @OpenSearchIntegTestCase.ClusterScope(scope = OpenSearchIntegTestCase.Scope.TEST, numDataNodes = 0) @@ -45,7 +40,7 @@ public class SegmentReplicationClusterSettingIT extends OpenSearchIntegTestCase protected static final int REPLICA_COUNT = 1; protected static final String REPLICATION_MISMATCH_VALIDATION_ERROR = - "Validation Failed: 1: index setting [index.replication.type] is not allowed to be set as [cluster.force.index.replication.type=true];"; + "Validation Failed: 1: index setting [index.replication.type] is not allowed to be set as [cluster.index.restrict.replication.type=true];"; @Override public Settings indexSettings() { @@ -133,36 +128,72 @@ public void testIndexReplicationSettingOverridesDocRepClusterSetting() throws Ex assertEquals(indicesService.indexService(anotherIndex).getIndexSettings().isSegRepEnabled(), false); } - public void testDifferentReplicationTypes_CreateIndex_StrictMode() throws Throwable { - final int documentCount = scaledRandomIntBetween(1, 10); - BiConsumer, List> consumer = (replicationList, dataNodesList) -> { - Settings indexSettings = Settings.builder().put(indexSettings()).put(SETTING_REPLICATION_TYPE, replicationList.get(1)).build(); - createIndex(INDEX_NAME, indexSettings); - }; - executeTest(true, consumer, INDEX_NAME, documentCount); + public void testReplicationTypesOverrideNotAllowed_IndexAPI() { + // Generate mutually exclusive replication strategies at cluster and index level + List replicationStrategies = getRandomReplicationTypesAsList(); + ReplicationType clusterLevelReplication = replicationStrategies.get(0); + ReplicationType indexLevelReplication = replicationStrategies.get(1); + Settings nodeSettings = Settings.builder() + .put(CLUSTER_SETTING_REPLICATION_TYPE, clusterLevelReplication) + .put(CLUSTER_INDEX_RESTRICT_REPLICATION_TYPE_SETTING.getKey(), true) + .build(); + internalCluster().startClusterManagerOnlyNode(nodeSettings); + internalCluster().startDataOnlyNode(nodeSettings); + Settings indexSettings = Settings.builder().put(indexSettings()).put(SETTING_REPLICATION_TYPE, indexLevelReplication).build(); + IllegalArgumentException exception = expectThrows(IllegalArgumentException.class, () -> createIndex(INDEX_NAME, indexSettings)); + assertEquals(REPLICATION_MISMATCH_VALIDATION_ERROR, exception.getMessage()); } - public void testDifferentReplicationTypes_IndexTemplates_StrictMode() throws Throwable { - final int documentCount = scaledRandomIntBetween(1, 10); - - BiConsumer, List> consumer = (replicationList, dataNodesList) -> { - client().admin() - .indices() - .preparePutTemplate("template_1") - .setPatterns(Collections.singletonList("test-idx*")) - .setSettings(Settings.builder().put(indexSettings()).put(SETTING_REPLICATION_TYPE, ReplicationType.DOCUMENT).build()) - .setOrder(0) - .get(); - - GetIndexTemplatesResponse response = client().admin().indices().prepareGetTemplates().get(); - assertThat(response.getIndexTemplates(), hasSize(1)); - - createIndex(INDEX_NAME); - }; - executeTest(true, consumer, INDEX_NAME, documentCount); + public void testReplicationTypesOverrideNotAllowed_WithTemplates() { + // Generate mutually exclusive replication strategies at cluster and index level + List replicationStrategies = getRandomReplicationTypesAsList(); + ReplicationType clusterLevelReplication = replicationStrategies.get(0); + ReplicationType indexLevelReplication = replicationStrategies.get(1); + Settings nodeSettings = Settings.builder() + .put(CLUSTER_SETTING_REPLICATION_TYPE, clusterLevelReplication) + .put(CLUSTER_INDEX_RESTRICT_REPLICATION_TYPE_SETTING.getKey(), true) + .build(); + internalCluster().startClusterManagerOnlyNode(nodeSettings); + internalCluster().startDataOnlyNode(nodeSettings); + internalCluster().startDataOnlyNode(nodeSettings); + logger.info( + "--> Create index with index level replication {} and cluster level replication {}", + indexLevelReplication, + clusterLevelReplication + ); + // Create index template + client().admin() + .indices() + .preparePutTemplate("template_1") + .setPatterns(Collections.singletonList("test-idx*")) + .setSettings(Settings.builder().put(indexSettings()).put(SETTING_REPLICATION_TYPE, ReplicationType.DOCUMENT).build()) + .setOrder(0) + .get(); + + GetIndexTemplatesResponse response = client().admin().indices().prepareGetTemplates().get(); + assertThat(response.getIndexTemplates(), hasSize(1)); + IllegalArgumentException exception = expectThrows(IllegalArgumentException.class, () -> createIndex(INDEX_NAME)); + assertEquals(REPLICATION_MISMATCH_VALIDATION_ERROR, exception.getMessage()); } - public void testMismatchingReplicationType_ResizeAction_StrictMode() throws Throwable { + public void testReplicationTypesOverrideNotAllowed_WithResizeAction() { + // Generate mutually exclusive replication strategies at cluster and index level + List replicationStrategies = getRandomReplicationTypesAsList(); + ReplicationType clusterLevelReplication = replicationStrategies.get(0); + ReplicationType indexLevelReplication = replicationStrategies.get(1); + Settings nodeSettings = Settings.builder() + .put(CLUSTER_SETTING_REPLICATION_TYPE, clusterLevelReplication) + .put(CLUSTER_INDEX_RESTRICT_REPLICATION_TYPE_SETTING.getKey(), true) + .build(); + internalCluster().startClusterManagerOnlyNode(nodeSettings); + internalCluster().startDataOnlyNode(nodeSettings); + internalCluster().startDataOnlyNode(nodeSettings); + logger.info( + "--> Create index with index level replication {} and cluster level replication {}", + indexLevelReplication, + clusterLevelReplication + ); + // Define resize action and target shard count. List> resizeActionsList = new ArrayList<>(); final int initialShardCount = 2; @@ -172,46 +203,24 @@ public void testMismatchingReplicationType_ResizeAction_StrictMode() throws Thro Tuple resizeActionTuple = resizeActionsList.get(random().nextInt(resizeActionsList.size())); final String targetIndexName = resizeActionTuple.v1().name().toLowerCase(Locale.ROOT) + "-target"; - final int documentCount = scaledRandomIntBetween(1, 10); logger.info("--> Performing resize action {} with shard count {}", resizeActionTuple.v1(), resizeActionTuple.v2()); - // Create an index using current cluster level settings - BiConsumer, List> consumer = (replicationList, dataNodesList) -> { - Settings indexSettings = Settings.builder() - .put(indexSettings()) - .put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, initialShardCount) - .put(SETTING_REPLICATION_TYPE, replicationList.get(0)) - .build(); - createIndex(INDEX_NAME, indexSettings); - - for (int i = 0; i < documentCount; i++) { - client().prepareIndex(INDEX_NAME).setId(String.valueOf(i)).setSource("foo", "bar").get(); - } - - // Block writes - client().admin() - .indices() - .prepareUpdateSettings(INDEX_NAME) - .setSettings(Settings.builder().put("index.blocks.write", true)) - .get(); - ensureGreen(); + Settings indexSettings = Settings.builder() + .put(indexSettings()) + .put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, initialShardCount) + .put(SETTING_REPLICATION_TYPE, clusterLevelReplication) + .build(); + createIndex(INDEX_NAME, indexSettings); - try { - for (String node : dataNodesList) { - assertBusy(() -> { - assertHitCount( - client(node).prepareSearch(INDEX_NAME).setSize(100).setQuery(new TermsQueryBuilder("foo", "bar")).get(), - documentCount - ); - }, 30, TimeUnit.SECONDS); - } - } catch (Exception e) { - throw new RuntimeException(e); - } + // Block writes + client().admin().indices().prepareUpdateSettings(INDEX_NAME).setSettings(Settings.builder().put("index.blocks.write", true)).get(); + ensureGreen(); - // Test create index fails - client().admin() + // Validate resize action fails + IllegalArgumentException exception = expectThrows( + IllegalArgumentException.class, + () -> client().admin() .indices() .prepareResizeIndex(INDEX_NAME, targetIndexName) .setResizeType(resizeActionTuple.v1()) @@ -220,61 +229,12 @@ public void testMismatchingReplicationType_ResizeAction_StrictMode() throws Thro .put("index.number_of_replicas", 0) .put("index.number_of_shards", resizeActionTuple.v2()) .putNull("index.blocks.write") - .put(SETTING_REPLICATION_TYPE, replicationList.get(1)) + .put(SETTING_REPLICATION_TYPE, indexLevelReplication) .build() ) - .get(); - }; - executeTest(true, consumer, targetIndexName, documentCount); - } - - // Creates a cluster with mis-matching cluster level and index level replication strategies and validates that index - // creation fails when cluster level setting `cluster.force.index.replication.type` is set to true and creation goes - // through when it is false. - private void executeTest( - boolean restrictIndexLevelReplicationTypeSetting, - BiConsumer, List> createIndexRunnable, - String targetIndexName, - int documentCount - ) throws Throwable { - // Generate mutually exclusive replication strategies at cluster and index level - List replicationStrategies = getRandomReplicationTypesAsList(); - ReplicationType clusterLevelReplication = replicationStrategies.get(0); - ReplicationType indexLevelReplication = replicationStrategies.get(1); - - Settings settings = Settings.builder() - .put(CLUSTER_SETTING_REPLICATION_TYPE, clusterLevelReplication) - .put(CLUSTER_FORCE_INDEX_REPLICATION_TYPE_SETTING.getKey(), restrictIndexLevelReplicationTypeSetting) - .build(); - internalCluster().startClusterManagerOnlyNode(settings); - final String dataNodeOne = internalCluster().startDataOnlyNode(settings); - final String dataNodeTwo = internalCluster().startDataOnlyNode(settings); - List dataNodes = Arrays.asList(dataNodeOne, dataNodeTwo); - - logger.info( - "--> Create index with cluster level setting {} and index level replication setting {}", - clusterLevelReplication, - indexLevelReplication + .get() ); - - if (restrictIndexLevelReplicationTypeSetting) { - try { - createIndexRunnable.accept(replicationStrategies, dataNodes); - } catch (IllegalArgumentException exception) { - assertEquals(REPLICATION_MISMATCH_VALIDATION_ERROR, exception.getMessage()); - } - } else { - createIndexRunnable.accept(replicationStrategies, dataNodes); - ensureGreen(targetIndexName); - for (String node : dataNodes) { - assertBusy(() -> { - assertHitCount( - client(node).prepareSearch(targetIndexName).setSize(100).setQuery(new TermsQueryBuilder("foo", "bar")).get(), - documentCount - ); - }); - } - } + assertEquals(REPLICATION_MISMATCH_VALIDATION_ERROR, exception.getMessage()); } /** diff --git a/server/src/internalClusterTest/java/org/opensearch/snapshots/SegmentReplicationSnapshotIT.java b/server/src/internalClusterTest/java/org/opensearch/snapshots/SegmentReplicationSnapshotIT.java index 7013d0dbc5003..2c12c0abb202b 100644 --- a/server/src/internalClusterTest/java/org/opensearch/snapshots/SegmentReplicationSnapshotIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/snapshots/SegmentReplicationSnapshotIT.java @@ -31,7 +31,7 @@ import java.util.concurrent.TimeUnit; import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_REPLICATION_TYPE; -import static org.opensearch.indices.IndicesService.CLUSTER_FORCE_INDEX_REPLICATION_TYPE_SETTING; +import static org.opensearch.indices.IndicesService.CLUSTER_INDEX_RESTRICT_REPLICATION_TYPE_SETTING; import static org.opensearch.indices.IndicesService.CLUSTER_SETTING_REPLICATION_TYPE; import static org.opensearch.indices.replication.SegmentReplicationBaseIT.waitForSearchableDocs; import static org.opensearch.test.hamcrest.OpenSearchAssertions.assertAcked; @@ -49,7 +49,7 @@ public class SegmentReplicationSnapshotIT extends AbstractSnapshotIntegTestCase private static final String SNAPSHOT_NAME = "test-segrep-snapshot"; protected static final String REPLICATION_MISMATCH_VALIDATION_ERROR = - "Validation Failed: 1: index setting [index.replication.type] is not allowed to be set as [cluster.force.index.replication.type=true];"; + "Validation Failed: 1: index setting [index.replication.type] is not allowed to be set as [cluster.index.restrict.replication.type=true];"; public Settings segRepEnableIndexSettings() { return getShardSettings().put(IndexMetadata.SETTING_REPLICATION_TYPE, ReplicationType.SEGMENT).build(); @@ -314,7 +314,7 @@ public void testSnapshotRestoreOnIndexWithSegRepClusterSetting() throws Exceptio /** * 1. Create index in DOCUMENT replication type * 2. Snapshot index - * 3. Add new set of nodes with `cluster.indices.replication.strategy` set to SEGMENT and `cluster.force.index.replication.type` + * 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. */ @@ -349,7 +349,7 @@ public void testSnapshotRestoreOnRestrictReplicationSetting() throws Exception { // Start new set of nodes with cluster level replication type setting and restrict replication type setting. Settings settings = Settings.builder() .put(CLUSTER_SETTING_REPLICATION_TYPE, ReplicationType.SEGMENT) - .put(CLUSTER_FORCE_INDEX_REPLICATION_TYPE_SETTING.getKey(), true) + .put(CLUSTER_INDEX_RESTRICT_REPLICATION_TYPE_SETTING.getKey(), true) .build(); // Start new cluster manager node 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 bb25193dd858e..3384393d8feaf 100644 --- a/server/src/main/java/org/opensearch/cluster/metadata/MetadataCreateIndexService.java +++ b/server/src/main/java/org/opensearch/cluster/metadata/MetadataCreateIndexService.java @@ -1332,19 +1332,19 @@ private static List validateIndexCustomPath(Settings settings, @Nullable /** * Validates {@code index.replication.type} is matches with cluster level setting {@code cluster.indices.replication.strategy} - * when {@code cluster.force.index.replication.type} is set to true. + * when {@code cluster.index.restrict.replication.type} is set to true. * * @param requestSettings settings passed in during index create request * @param clusterSettings cluster setting */ private static Optional validateIndexReplicationTypeSettings(Settings requestSettings, ClusterSettings clusterSettings) { - if (clusterSettings.get(IndicesService.CLUSTER_FORCE_INDEX_REPLICATION_TYPE_SETTING) + if (clusterSettings.get(IndicesService.CLUSTER_INDEX_RESTRICT_REPLICATION_TYPE_SETTING) && requestSettings.hasValue(SETTING_REPLICATION_TYPE) && requestSettings.get(INDEX_REPLICATION_TYPE_SETTING.getKey()) .equals(clusterSettings.get(CLUSTER_REPLICATION_TYPE_SETTING).name()) == false) { return Optional.of( "index setting [index.replication.type] is not allowed to be set as [" - + IndicesService.CLUSTER_FORCE_INDEX_REPLICATION_TYPE_SETTING.getKey() + + IndicesService.CLUSTER_INDEX_RESTRICT_REPLICATION_TYPE_SETTING.getKey() + "=true]" ); } diff --git a/server/src/main/java/org/opensearch/common/settings/ClusterSettings.java b/server/src/main/java/org/opensearch/common/settings/ClusterSettings.java index 1870397f44d59..fa4b0f475edc5 100644 --- a/server/src/main/java/org/opensearch/common/settings/ClusterSettings.java +++ b/server/src/main/java/org/opensearch/common/settings/ClusterSettings.java @@ -702,7 +702,7 @@ public void apply(Settings value, Settings current, Settings previous) { CpuBasedAdmissionControllerSettings.CPU_BASED_ADMISSION_CONTROLLER_TRANSPORT_LAYER_MODE, CpuBasedAdmissionControllerSettings.INDEXING_CPU_USAGE_LIMIT, CpuBasedAdmissionControllerSettings.SEARCH_CPU_USAGE_LIMIT, - IndicesService.CLUSTER_FORCE_INDEX_REPLICATION_TYPE_SETTING + IndicesService.CLUSTER_INDEX_RESTRICT_REPLICATION_TYPE_SETTING ) ) ); diff --git a/server/src/main/java/org/opensearch/indices/IndicesService.java b/server/src/main/java/org/opensearch/indices/IndicesService.java index c729c52cc7675..5c3beaf8509bd 100644 --- a/server/src/main/java/org/opensearch/indices/IndicesService.java +++ b/server/src/main/java/org/opensearch/indices/IndicesService.java @@ -307,8 +307,8 @@ public class IndicesService extends AbstractLifecycleComponent * does not match the cluster setting. If disabled, a user can choose a replication type on a per-index basis using * the index.replication.type setting. */ - public static final Setting CLUSTER_FORCE_INDEX_REPLICATION_TYPE_SETTING = Setting.boolSetting( - "cluster.force.index.replication.type", + public static final Setting CLUSTER_INDEX_RESTRICT_REPLICATION_TYPE_SETTING = Setting.boolSetting( + "cluster.index.restrict.replication.type", false, Property.NodeScope, Property.Final diff --git a/server/src/test/java/org/opensearch/cluster/metadata/MetadataCreateIndexServiceTests.java b/server/src/test/java/org/opensearch/cluster/metadata/MetadataCreateIndexServiceTests.java index 83bf233d4940e..d48eb62f0697b 100644 --- a/server/src/test/java/org/opensearch/cluster/metadata/MetadataCreateIndexServiceTests.java +++ b/server/src/test/java/org/opensearch/cluster/metadata/MetadataCreateIndexServiceTests.java @@ -138,7 +138,7 @@ import static org.opensearch.index.IndexSettings.INDEX_SOFT_DELETES_SETTING; import static org.opensearch.index.IndexSettings.INDEX_TRANSLOG_DURABILITY_SETTING; import static org.opensearch.indices.IndicesService.CLUSTER_DEFAULT_INDEX_REFRESH_INTERVAL_SETTING; -import static org.opensearch.indices.IndicesService.CLUSTER_FORCE_INDEX_REPLICATION_TYPE_SETTING; +import static org.opensearch.indices.IndicesService.CLUSTER_INDEX_RESTRICT_REPLICATION_TYPE_SETTING; import static org.opensearch.indices.IndicesService.CLUSTER_MINIMUM_INDEX_REFRESH_INTERVAL_SETTING; import static org.opensearch.indices.IndicesService.CLUSTER_REMOTE_INDEX_RESTRICT_ASYNC_DURABILITY_SETTING; import static org.opensearch.indices.IndicesService.CLUSTER_REPLICATION_TYPE_SETTING; @@ -169,7 +169,7 @@ public class MetadataCreateIndexServiceTests extends OpenSearchTestCase { + REMOTE_STORE_TRANSLOG_REPOSITORY_NAME_ATTRIBUTE_KEY; final String REPLICATION_MISMATCH_VALIDATION_ERROR = - "Validation Failed: 1: index setting [index.replication.type] is not allowed to be set as [cluster.force.index.replication.type=true];"; + "Validation Failed: 1: index setting [index.replication.type] is not allowed to be set as [cluster.index.restrict.replication.type=true];"; @Before public void setup() throws Exception { @@ -1247,7 +1247,7 @@ public void testIndexTemplateReplicationType() { public void testClusterForceReplicationTypeInAggregateSettings() { Settings settings = Settings.builder() .put(CLUSTER_REPLICATION_TYPE_SETTING.getKey(), ReplicationType.SEGMENT) - .put(CLUSTER_FORCE_INDEX_REPLICATION_TYPE_SETTING.getKey(), true) + .put(CLUSTER_INDEX_RESTRICT_REPLICATION_TYPE_SETTING.getKey(), true) .build(); ClusterSettings clusterSettings = new ClusterSettings(settings, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS); Settings matchingReplicationIndexSettings = Settings.builder() @@ -1301,7 +1301,7 @@ public void testClusterForceReplicationTypeInValidateIndexSettings() { // Enforce cluster level replication type setting final Settings forceClusterSettingEnabled = Settings.builder() .put(CLUSTER_REPLICATION_TYPE_SETTING.getKey(), ReplicationType.SEGMENT) - .put(CLUSTER_FORCE_INDEX_REPLICATION_TYPE_SETTING.getKey(), true) + .put(CLUSTER_INDEX_RESTRICT_REPLICATION_TYPE_SETTING.getKey(), true) .build(); ClusterSettings clusterSettings = new ClusterSettings(forceClusterSettingEnabled, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS); when(clusterService.getSettings()).thenReturn(forceClusterSettingEnabled); @@ -1335,7 +1335,7 @@ public void testClusterForceReplicationTypeInValidateIndexSettings() { // Cluster level replication type setting not enforced final Settings forceClusterSettingDisabled = Settings.builder() .put(CLUSTER_REPLICATION_TYPE_SETTING.getKey(), ReplicationType.SEGMENT) - .put(CLUSTER_FORCE_INDEX_REPLICATION_TYPE_SETTING.getKey(), false) + .put(CLUSTER_INDEX_RESTRICT_REPLICATION_TYPE_SETTING.getKey(), false) .build(); clusterSettings = new ClusterSettings(forceClusterSettingDisabled, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS); when(clusterService.getClusterSettings()).thenReturn(clusterSettings); From 46c530d673bff3470d6c137275604d4f86b78f0f Mon Sep 17 00:00:00 2001 From: Suraj Singh Date: Wed, 20 Dec 2023 11:26:08 -0800 Subject: [PATCH 5/6] Use appropriate variable names Signed-off-by: Suraj Singh --- .../cluster/metadata/MetadataCreateIndexServiceTests.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/server/src/test/java/org/opensearch/cluster/metadata/MetadataCreateIndexServiceTests.java b/server/src/test/java/org/opensearch/cluster/metadata/MetadataCreateIndexServiceTests.java index d48eb62f0697b..cea151748bfb6 100644 --- a/server/src/test/java/org/opensearch/cluster/metadata/MetadataCreateIndexServiceTests.java +++ b/server/src/test/java/org/opensearch/cluster/metadata/MetadataCreateIndexServiceTests.java @@ -1250,11 +1250,11 @@ public void testClusterForceReplicationTypeInAggregateSettings() { .put(CLUSTER_INDEX_RESTRICT_REPLICATION_TYPE_SETTING.getKey(), true) .build(); ClusterSettings clusterSettings = new ClusterSettings(settings, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS); - Settings matchingReplicationIndexSettings = Settings.builder() + Settings nonMatchingReplicationIndexSettings = Settings.builder() .put(INDEX_REPLICATION_TYPE_SETTING.getKey(), ReplicationType.DOCUMENT) .build(); request = new CreateIndexClusterStateUpdateRequest("create index", "test", "test"); - request.settings(matchingReplicationIndexSettings); + request.settings(nonMatchingReplicationIndexSettings); IndexCreationException exception = expectThrows( IndexCreationException.class, () -> aggregateIndexSettings( @@ -1271,10 +1271,10 @@ public void testClusterForceReplicationTypeInAggregateSettings() { ); assertEquals(REPLICATION_MISMATCH_VALIDATION_ERROR, exception.getCause().getMessage()); - Settings nonMatchingReplicationIndexSettings = Settings.builder() + Settings matchingReplicationIndexSettings = Settings.builder() .put(INDEX_REPLICATION_TYPE_SETTING.getKey(), ReplicationType.SEGMENT) .build(); - request.settings(nonMatchingReplicationIndexSettings); + request.settings(matchingReplicationIndexSettings); Settings aggregateIndexSettings = aggregateIndexSettings( ClusterState.EMPTY_STATE, request, From 948ce3a3a40f120db2fa6034564fa1c1f46d1f8a Mon Sep 17 00:00:00 2001 From: Suraj Singh Date: Wed, 20 Dec 2023 14:21:06 -0800 Subject: [PATCH 6/6] Fix failing integ test Signed-off-by: Suraj Singh --- .../replication/SegmentReplicationClusterSettingIT.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/server/src/internalClusterTest/java/org/opensearch/indices/replication/SegmentReplicationClusterSettingIT.java b/server/src/internalClusterTest/java/org/opensearch/indices/replication/SegmentReplicationClusterSettingIT.java index 0f542a70ea92e..c4e8ccfc0ecec 100644 --- a/server/src/internalClusterTest/java/org/opensearch/indices/replication/SegmentReplicationClusterSettingIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/indices/replication/SegmentReplicationClusterSettingIT.java @@ -148,7 +148,7 @@ public void testReplicationTypesOverrideNotAllowed_WithTemplates() { // Generate mutually exclusive replication strategies at cluster and index level List replicationStrategies = getRandomReplicationTypesAsList(); ReplicationType clusterLevelReplication = replicationStrategies.get(0); - ReplicationType indexLevelReplication = replicationStrategies.get(1); + ReplicationType templateReplicationType = replicationStrategies.get(1); Settings nodeSettings = Settings.builder() .put(CLUSTER_SETTING_REPLICATION_TYPE, clusterLevelReplication) .put(CLUSTER_INDEX_RESTRICT_REPLICATION_TYPE_SETTING.getKey(), true) @@ -157,8 +157,8 @@ public void testReplicationTypesOverrideNotAllowed_WithTemplates() { internalCluster().startDataOnlyNode(nodeSettings); internalCluster().startDataOnlyNode(nodeSettings); logger.info( - "--> Create index with index level replication {} and cluster level replication {}", - indexLevelReplication, + "--> Create index with template replication {} and cluster level replication {}", + templateReplicationType, clusterLevelReplication ); // Create index template @@ -166,7 +166,7 @@ public void testReplicationTypesOverrideNotAllowed_WithTemplates() { .indices() .preparePutTemplate("template_1") .setPatterns(Collections.singletonList("test-idx*")) - .setSettings(Settings.builder().put(indexSettings()).put(SETTING_REPLICATION_TYPE, ReplicationType.DOCUMENT).build()) + .setSettings(Settings.builder().put(indexSettings()).put(SETTING_REPLICATION_TYPE, templateReplicationType).build()) .setOrder(0) .get();