From aa9b732cf5e9ddd98196a61da31b70b5c1cb8d17 Mon Sep 17 00:00:00 2001 From: Rishikesh1159 Date: Wed, 21 Jun 2023 20:16:26 +0000 Subject: [PATCH] remove logic of strictly using only docrep for system indices and hidden indices. Signed-off-by: Rishikesh1159 --- .../SegmentReplicationClusterSettingIT.java | 66 ------------ .../metadata/MetadataCreateIndexService.java | 25 ++--- .../MetadataCreateIndexServiceTests.java | 100 ++++-------------- 3 files changed, 25 insertions(+), 166 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 57578cdbfa8e8..c1326c1b50c9e 100644 --- a/server/src/internalClusterTest/java/org/opensearch/indices/replication/SegmentReplicationClusterSettingIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/indices/replication/SegmentReplicationClusterSettingIT.java @@ -15,16 +15,8 @@ import org.opensearch.index.Index; import org.opensearch.index.IndexModule; import org.opensearch.indices.IndicesService; -import org.opensearch.indices.SystemIndexDescriptor; import org.opensearch.indices.replication.common.ReplicationType; -import org.opensearch.plugins.Plugin; -import org.opensearch.plugins.SystemIndexPlugin; import org.opensearch.test.OpenSearchIntegTestCase; -import org.opensearch.test.transport.MockTransportService; - -import java.util.Collection; -import java.util.Collections; -import java.util.Arrays; import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_REPLICATION_TYPE; import static org.opensearch.indices.IndicesService.CLUSTER_SETTING_REPLICATION_TYPE; @@ -60,40 +52,6 @@ protected Settings nodeSettings(int nodeOrdinal) { .build(); } - public static class TestPlugin extends Plugin implements SystemIndexPlugin { - @Override - public Collection getSystemIndexDescriptors(Settings settings) { - return Collections.singletonList( - new SystemIndexDescriptor(SYSTEM_INDEX_NAME, "System index for [" + getTestClass().getName() + ']') - ); - } - } - - @Override - protected Collection> nodePlugins() { - return Arrays.asList(SegmentReplicationClusterSettingIT.TestPlugin.class, MockTransportService.TestPlugin.class); - } - - public void testSystemIndexWithSegmentReplicationClusterSetting() throws Exception { - - // Starting two nodes with primary and replica shards respectively. - final String primaryNode = internalCluster().startNode(); - createIndex(SYSTEM_INDEX_NAME); - ensureYellowAndNoInitializingShards(SYSTEM_INDEX_NAME); - final String replicaNode = internalCluster().startNode(); - ensureGreen(SYSTEM_INDEX_NAME); - final GetSettingsResponse response = client().admin() - .indices() - .getSettings(new GetSettingsRequest().indices(SYSTEM_INDEX_NAME).includeDefaults(true)) - .actionGet(); - assertEquals(response.getSetting(SYSTEM_INDEX_NAME, SETTING_REPLICATION_TYPE), ReplicationType.DOCUMENT.toString()); - - // Verify index setting isSegRepEnabled is false. - Index index = resolveIndex(SYSTEM_INDEX_NAME); - IndicesService indicesService = internalCluster().getInstance(IndicesService.class, primaryNode); - assertEquals(indicesService.indexService(index).getIndexSettings().isSegRepEnabled(), false); - } - public void testIndexReplicationSettingOverridesSegRepClusterSetting() throws Exception { Settings settings = Settings.builder().put(CLUSTER_SETTING_REPLICATION_TYPE, ReplicationType.SEGMENT).build(); final String ANOTHER_INDEX = "test-index"; @@ -165,28 +123,4 @@ public void testIndexReplicationSettingOverridesDocRepClusterSetting() throws Ex assertEquals(indicesService.indexService(anotherIndex).getIndexSettings().isSegRepEnabled(), false); } - public void testHiddenIndicesWithReplicationStrategyClusterSetting() throws Exception { - final String primaryNode = internalCluster().startNode(); - final String replicaNode = internalCluster().startNode(); - prepareCreate( - INDEX_NAME, - Settings.builder() - // we want to set index as hidden - .put("index.hidden", true) - ).get(); - ensureGreen(INDEX_NAME); - - // Verify that document replication strategy is used for hidden indices. - final GetSettingsResponse response = client().admin() - .indices() - .getSettings(new GetSettingsRequest().indices(INDEX_NAME).includeDefaults(true)) - .actionGet(); - assertEquals(response.getSetting(INDEX_NAME, SETTING_REPLICATION_TYPE), ReplicationType.DOCUMENT.toString()); - - // Verify index setting isSegRepEnabled. - Index index = resolveIndex(INDEX_NAME); - IndicesService indicesService = internalCluster().getInstance(IndicesService.class, primaryNode); - 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 6e22de7c72d07..0b13b5273ba34 100644 --- a/server/src/main/java/org/opensearch/cluster/metadata/MetadataCreateIndexService.java +++ b/server/src/main/java/org/opensearch/cluster/metadata/MetadataCreateIndexService.java @@ -583,8 +583,7 @@ private ClusterState applyCreateIndexRequestWithV1Templates( settings, indexScopedSettings, shardLimitValidator, - indexSettingProviders, - systemIndices.validateSystemIndex(request.index()) + indexSettingProviders ); int routingNumShards = getIndexNumberOfRoutingShards(aggregatedIndexSettings, null); IndexMetadata tmpImd = buildAndValidateTemporaryIndexMetadata(currentState, aggregatedIndexSettings, request, routingNumShards); @@ -648,8 +647,7 @@ private ClusterState applyCreateIndexRequestWithV2Template( settings, indexScopedSettings, shardLimitValidator, - indexSettingProviders, - systemIndices.validateSystemIndex(request.index()) + indexSettingProviders ); int routingNumShards = getIndexNumberOfRoutingShards(aggregatedIndexSettings, null); IndexMetadata tmpImd = buildAndValidateTemporaryIndexMetadata(currentState, aggregatedIndexSettings, request, routingNumShards); @@ -729,8 +727,7 @@ private ClusterState applyCreateIndexRequestWithExistingMetadata( settings, indexScopedSettings, shardLimitValidator, - indexSettingProviders, - sourceMetadata.isSystem() + indexSettingProviders ); final int routingNumShards = getIndexNumberOfRoutingShards(aggregatedIndexSettings, sourceMetadata); IndexMetadata tmpImd = buildAndValidateTemporaryIndexMetadata(currentState, aggregatedIndexSettings, request, routingNumShards); @@ -813,8 +810,7 @@ static Settings aggregateIndexSettings( Settings settings, IndexScopedSettings indexScopedSettings, ShardLimitValidator shardLimitValidator, - Set indexSettingProviders, - boolean isSystemIndex + Set indexSettingProviders ) { // Create builders for the template and request settings. We transform these into builders // because we may want settings to be "removed" from these prior to being set on the new @@ -898,7 +894,7 @@ static Settings aggregateIndexSettings( indexSettingsBuilder.put(IndexMetadata.SETTING_INDEX_PROVIDED_NAME, request.getProvidedName()); indexSettingsBuilder.put(SETTING_INDEX_UUID, UUIDs.randomBase64UUID()); - updateReplicationStrategy(indexSettingsBuilder, request.settings(), settings, isSystemIndex); + updateReplicationStrategy(indexSettingsBuilder, request.settings(), settings); updateRemoteStoreSettings(indexSettingsBuilder, request.settings(), settings); if (sourceMetadata != null) { @@ -939,16 +935,7 @@ static Settings aggregateIndexSettings( * @param requestSettings settings passed in during index create request * @param clusterSettings cluster level settings */ - private static void updateReplicationStrategy( - Settings.Builder settingsBuilder, - Settings requestSettings, - Settings clusterSettings, - boolean isSystemIndex - ) { - if (isSystemIndex || IndexMetadata.INDEX_HIDDEN_SETTING.get(requestSettings)) { - settingsBuilder.put(SETTING_REPLICATION_TYPE, ReplicationType.DOCUMENT); - return; - } + private static void updateReplicationStrategy(Settings.Builder settingsBuilder, Settings requestSettings, Settings clusterSettings) { if (CLUSTER_REPLICATION_TYPE_SETTING.exists(clusterSettings) && INDEX_REPLICATION_TYPE_SETTING.exists(requestSettings) == false) { settingsBuilder.put(SETTING_REPLICATION_TYPE, CLUSTER_REPLICATION_TYPE_SETTING.get(clusterSettings)); } 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 2abb1e99facef..fdef3cb5498b4 100644 --- a/server/src/test/java/org/opensearch/cluster/metadata/MetadataCreateIndexServiceTests.java +++ b/server/src/test/java/org/opensearch/cluster/metadata/MetadataCreateIndexServiceTests.java @@ -817,8 +817,7 @@ public void testAggregateSettingsAppliesSettingsFromTemplatesAndRequest() { Settings.EMPTY, IndexScopedSettings.DEFAULT_SCOPED_SETTINGS, randomShardLimitService(), - Collections.emptySet(), - false + Collections.emptySet() ); assertThat(aggregatedIndexSettings.get("template_setting"), equalTo("value1")); @@ -880,8 +879,7 @@ public void testRequestDataHavePriorityOverTemplateData() throws Exception { Settings.EMPTY, IndexScopedSettings.DEFAULT_SCOPED_SETTINGS, randomShardLimitService(), - Collections.emptySet(), - false + Collections.emptySet() ); assertThat(resolvedAliases.get(0).getSearchRouting(), equalTo("fromRequest")); @@ -903,8 +901,7 @@ public void testDefaultSettings() { Settings.EMPTY, IndexScopedSettings.DEFAULT_SCOPED_SETTINGS, randomShardLimitService(), - Collections.emptySet(), - false + Collections.emptySet() ); assertThat(aggregatedIndexSettings.get(SETTING_NUMBER_OF_SHARDS), equalTo("1")); @@ -919,8 +916,7 @@ public void testSettingsFromClusterState() { Settings.builder().put(SETTING_NUMBER_OF_SHARDS, 15).build(), IndexScopedSettings.DEFAULT_SCOPED_SETTINGS, randomShardLimitService(), - Collections.emptySet(), - false + Collections.emptySet() ); assertThat(aggregatedIndexSettings.get(SETTING_NUMBER_OF_SHARDS), equalTo("15")); @@ -957,8 +953,7 @@ public void testTemplateOrder() throws Exception { Settings.EMPTY, IndexScopedSettings.DEFAULT_SCOPED_SETTINGS, randomShardLimitService(), - Collections.emptySet(), - false + Collections.emptySet() ); List resolvedAliases = resolveAndValidateAliases( request.index(), @@ -997,8 +992,7 @@ public void testAggregateIndexSettingsIgnoresTemplatesOnCreateFromSourceIndex() Settings.EMPTY, IndexScopedSettings.DEFAULT_SCOPED_SETTINGS, randomShardLimitService(), - Collections.emptySet(), - false + Collections.emptySet() ); assertThat(aggregatedIndexSettings.get("templateSetting"), is(nullValue())); @@ -1220,8 +1214,7 @@ public void testRemoteStoreNoUserOverrideConflictingReplicationTypeIndexSettings settings, IndexScopedSettings.DEFAULT_SCOPED_SETTINGS, randomShardLimitService(), - Collections.emptySet(), - false + Collections.emptySet() ) ); assertThat( @@ -1252,8 +1245,7 @@ public void testRemoteStoreNoUserOverrideExceptReplicationTypeSegmentIndexSettin settings, IndexScopedSettings.DEFAULT_SCOPED_SETTINGS, randomShardLimitService(), - Collections.emptySet(), - false + Collections.emptySet() ); verifyRemoteStoreIndexSettings( indexSettings, @@ -1285,8 +1277,7 @@ public void testRemoteStoreNoUserOverrideIndexSettings() { settings, IndexScopedSettings.DEFAULT_SCOPED_SETTINGS, randomShardLimitService(), - Collections.emptySet(), - false + Collections.emptySet() ); verifyRemoteStoreIndexSettings( indexSettings, @@ -1320,8 +1311,7 @@ public void testRemoteStoreDisabledByUserIndexSettings() { settings, IndexScopedSettings.DEFAULT_SCOPED_SETTINGS, randomShardLimitService(), - Collections.emptySet(), - false + Collections.emptySet() ); verifyRemoteStoreIndexSettings(indexSettings, "false", null, null, null, ReplicationType.SEGMENT.toString(), null); } @@ -1347,8 +1337,7 @@ public void testRemoteStoreTranslogDisabledByUserIndexSettings() { settings, IndexScopedSettings.DEFAULT_SCOPED_SETTINGS, randomShardLimitService(), - Collections.emptySet(), - false + Collections.emptySet() ); verifyRemoteStoreIndexSettings(indexSettings, "true", "my-segment-repo-1", "false", null, ReplicationType.SEGMENT.toString(), null); } @@ -1377,8 +1366,7 @@ public void testRemoteStoreOverrideSegmentRepoIndexSettings() { settings, IndexScopedSettings.DEFAULT_SCOPED_SETTINGS, randomShardLimitService(), - Collections.emptySet(), - false + Collections.emptySet() ); verifyRemoteStoreIndexSettings( indexSettings, @@ -1412,8 +1400,7 @@ public void testRemoteStoreOverrideTranslogRepoIndexSettings() { settings, IndexScopedSettings.DEFAULT_SCOPED_SETTINGS, randomShardLimitService(), - Collections.emptySet(), - false + Collections.emptySet() ); verifyRemoteStoreIndexSettings( indexSettings, @@ -1447,8 +1434,7 @@ public void testRemoteStoreOverrideReplicationTypeIndexSettings() { settings, IndexScopedSettings.DEFAULT_SCOPED_SETTINGS, randomShardLimitService(), - Collections.emptySet(), - false + Collections.emptySet() ); verifyRemoteStoreIndexSettings(indexSettings, null, null, null, null, ReplicationType.DOCUMENT.toString(), null); } @@ -1522,8 +1508,7 @@ public void testSoftDeletesDisabledIsRejected() { Settings.EMPTY, IndexScopedSettings.DEFAULT_SCOPED_SETTINGS, randomShardLimitService(), - Collections.emptySet(), - false + Collections.emptySet() ); }); assertThat( @@ -1552,8 +1537,7 @@ public void testValidateTranslogRetentionSettings() { Settings.EMPTY, IndexScopedSettings.DEFAULT_SCOPED_SETTINGS, randomShardLimitService(), - Collections.emptySet(), - false + Collections.emptySet() ); assertWarnings( "Translog retention settings [index.translog.retention.age] " @@ -1600,8 +1584,7 @@ public void testDeprecatedSimpleFSStoreSettings() { Settings.EMPTY, IndexScopedSettings.DEFAULT_SCOPED_SETTINGS, randomShardLimitService(), - Collections.emptySet(), - false + Collections.emptySet() ); assertWarnings( "[simplefs] is deprecated and will be removed in 2.0. Use [niofs], which offers equal " @@ -1620,8 +1603,7 @@ public void testClusterReplicationSetting() { settings, IndexScopedSettings.DEFAULT_SCOPED_SETTINGS, randomShardLimitService(), - Collections.emptySet(), - false + Collections.emptySet() ); assertEquals(ReplicationType.SEGMENT.toString(), indexSettings.get(SETTING_REPLICATION_TYPE)); } @@ -1641,56 +1623,12 @@ public void testIndexSettingOverridesClusterReplicationSetting() { settings, IndexScopedSettings.DEFAULT_SCOPED_SETTINGS, randomShardLimitService(), - Collections.emptySet(), - false + Collections.emptySet() ); // Verify if index setting overrides cluster replication setting assertEquals(ReplicationType.DOCUMENT.toString(), indexSettings.get(SETTING_REPLICATION_TYPE)); } - public void testHiddenIndexUsesDocumentReplication() { - Settings settings = Settings.builder().put(CLUSTER_REPLICATION_TYPE_SETTING.getKey(), ReplicationType.SEGMENT).build(); - request = new CreateIndexClusterStateUpdateRequest("create index", "test", "test"); - final Settings.Builder requestSettings = Settings.builder(); - // Set index setting replication type as DOCUMENT - requestSettings.put("index.hidden", true); - request.settings(requestSettings.build()); - Settings indexSettings = aggregateIndexSettings( - ClusterState.EMPTY_STATE, - request, - Settings.EMPTY, - null, - settings, - IndexScopedSettings.DEFAULT_SCOPED_SETTINGS, - randomShardLimitService(), - Collections.emptySet(), - false - ); - // Verify replication type is Document Replication - assertEquals(ReplicationType.DOCUMENT.toString(), indexSettings.get(SETTING_REPLICATION_TYPE)); - } - - public void testSystemIndexUsesDocumentReplication() { - Settings settings = Settings.builder().put(CLUSTER_REPLICATION_TYPE_SETTING.getKey(), ReplicationType.SEGMENT).build(); - request = new CreateIndexClusterStateUpdateRequest("create index", "test", "test"); - final Settings.Builder requestSettings = Settings.builder(); - request.settings(requestSettings.build()); - // set isSystemIndex parameter as true - Settings indexSettings = aggregateIndexSettings( - ClusterState.EMPTY_STATE, - request, - Settings.EMPTY, - null, - settings, - IndexScopedSettings.DEFAULT_SCOPED_SETTINGS, - randomShardLimitService(), - Collections.emptySet(), - true - ); - // Verify replication type is Document Replication - assertEquals(ReplicationType.DOCUMENT.toString(), indexSettings.get(SETTING_REPLICATION_TYPE)); - } - private IndexTemplateMetadata addMatchingTemplate(Consumer configurator) { IndexTemplateMetadata.Builder builder = templateMetadataBuilder("template1", "te*"); configurator.accept(builder);