From d8a054acfdf85204620d84144f4a951dc8850844 Mon Sep 17 00:00:00 2001 From: Rishikesh1159 Date: Fri, 23 Jun 2023 22:41:49 +0000 Subject: [PATCH] Backport PR's #8186 and #8235 Signed-off-by: Rishikesh1159 --- ...CreateRemoteIndexClusterDefaultDocRep.java | 2 +- .../remotestore/CreateRemoteIndexIT.java | 52 +++++++++++++++++++ .../cluster/metadata/IndexMetadata.java | 8 +-- .../metadata/MetadataCreateIndexService.java | 42 +++++++++------ .../MetadataCreateIndexServiceTests.java | 31 ++++++++++- 5 files changed, 110 insertions(+), 25 deletions(-) diff --git a/server/src/internalClusterTest/java/org/opensearch/remotestore/CreateRemoteIndexClusterDefaultDocRep.java b/server/src/internalClusterTest/java/org/opensearch/remotestore/CreateRemoteIndexClusterDefaultDocRep.java index a4f6aceedc586..fdb6819beab97 100644 --- a/server/src/internalClusterTest/java/org/opensearch/remotestore/CreateRemoteIndexClusterDefaultDocRep.java +++ b/server/src/internalClusterTest/java/org/opensearch/remotestore/CreateRemoteIndexClusterDefaultDocRep.java @@ -61,7 +61,7 @@ public void testDefaultRemoteStoreNoUserOverride() throws Exception { ); assertThat( exc.getMessage(), - containsString("Cannot enable [index.remote_store.enabled] when [cluster.indices.replication.strategy] is DOCUMENT") + containsString("Cannot enable [index.remote_store.enabled] when [index.replication.type] is DOCUMENT") ); } diff --git a/server/src/internalClusterTest/java/org/opensearch/remotestore/CreateRemoteIndexIT.java b/server/src/internalClusterTest/java/org/opensearch/remotestore/CreateRemoteIndexIT.java index a3235fd2a91d4..71e70f8d30e9a 100644 --- a/server/src/internalClusterTest/java/org/opensearch/remotestore/CreateRemoteIndexIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/remotestore/CreateRemoteIndexIT.java @@ -12,13 +12,22 @@ import org.junit.Before; import org.opensearch.action.admin.indices.get.GetIndexRequest; import org.opensearch.action.admin.indices.get.GetIndexResponse; +import org.opensearch.action.admin.indices.settings.get.GetSettingsRequest; +import org.opensearch.action.admin.indices.settings.get.GetSettingsResponse; import org.opensearch.cluster.metadata.IndexMetadata; import org.opensearch.common.settings.Settings; import org.opensearch.common.util.FeatureFlags; +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.FeatureFlagSetter; import org.opensearch.test.OpenSearchIntegTestCase; +import java.util.Arrays; +import java.util.Collection; +import java.util.Collections; + import static org.hamcrest.Matchers.containsString; import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_REMOTE_STORE_ENABLED; import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_REMOTE_STORE_REPOSITORY; @@ -57,6 +66,20 @@ protected Settings nodeSettings(int nodeOriginal) { return builder.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(CreateRemoteIndexIT.TestPlugin.class); + } + @Override protected Settings featureFlagSettings() { return Settings.builder().put(super.featureFlagSettings()).put(FeatureFlags.REMOTE_STORE, "true").build(); @@ -105,6 +128,35 @@ public void testDefaultRemoteStoreNoUserOverride() throws Exception { ); } + private static final String SYSTEM_INDEX_NAME = ".test-system-index"; + + public void testSystemIndexWithRemoteStoreClusterSetting() throws Exception { + createIndex(SYSTEM_INDEX_NAME); + ensureGreen(SYSTEM_INDEX_NAME); + final GetSettingsResponse response = client().admin() + .indices() + .getSettings(new GetSettingsRequest().indices(SYSTEM_INDEX_NAME).includeDefaults(true)) + .actionGet(); + // Verify that Document replication strategy is used + assertEquals(response.getSetting(SYSTEM_INDEX_NAME, SETTING_REPLICATION_TYPE), ReplicationType.DOCUMENT.toString()); + assertEquals(response.getSetting(SYSTEM_INDEX_NAME, SETTING_REMOTE_STORE_ENABLED), "false"); + } + + public void testSystemIndexWithRemoteStoreIndexSettings() throws Exception { + prepareCreate( + SYSTEM_INDEX_NAME, + Settings.builder().put(SETTING_REPLICATION_TYPE, ReplicationType.SEGMENT).put(SETTING_REMOTE_STORE_ENABLED, true) + ).get(); + ensureGreen(SYSTEM_INDEX_NAME); + final GetSettingsResponse response = client().admin() + .indices() + .getSettings(new GetSettingsRequest().indices(SYSTEM_INDEX_NAME).includeDefaults(true)) + .actionGet(); + // Verify that Document replication strategy is used + assertEquals(response.getSetting(SYSTEM_INDEX_NAME, SETTING_REPLICATION_TYPE), ReplicationType.DOCUMENT.toString()); + assertEquals(response.getSetting(SYSTEM_INDEX_NAME, SETTING_REMOTE_STORE_ENABLED), "false"); + } + public void testRemoteStoreDisabledByUser() throws Exception { Settings settings = Settings.builder() .put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, 1) diff --git a/server/src/main/java/org/opensearch/cluster/metadata/IndexMetadata.java b/server/src/main/java/org/opensearch/cluster/metadata/IndexMetadata.java index 8884613b4c598..728c583960e46 100644 --- a/server/src/main/java/org/opensearch/cluster/metadata/IndexMetadata.java +++ b/server/src/main/java/org/opensearch/cluster/metadata/IndexMetadata.java @@ -96,7 +96,6 @@ import static org.opensearch.cluster.node.DiscoveryNodeFilters.OpType.OR; import static org.opensearch.common.settings.Settings.readSettingsFromStream; import static org.opensearch.common.settings.Settings.writeSettingsToStream; -import static org.opensearch.indices.IndicesService.CLUSTER_REPLICATION_TYPE_SETTING; /** * Index metadata information @@ -318,11 +317,8 @@ public void validate(final Boolean value) {} @Override public void validate(final Boolean value, final Map, Object> settings) { - final Object clusterSettingReplicationType = settings.get(CLUSTER_REPLICATION_TYPE_SETTING); final Object replicationType = settings.get(INDEX_REPLICATION_TYPE_SETTING); - if ((replicationType).equals(ReplicationType.SEGMENT) == false - && (clusterSettingReplicationType).equals(ReplicationType.SEGMENT) == false - && value == true) { + if (ReplicationType.SEGMENT.equals(replicationType) == false && value) { throw new IllegalArgumentException( "To enable " + INDEX_REMOTE_STORE_ENABLED_SETTING.getKey() @@ -336,7 +332,7 @@ public void validate(final Boolean value, final Map, Object> settings @Override public Iterator> settings() { - final List> settings = List.of(INDEX_REPLICATION_TYPE_SETTING, CLUSTER_REPLICATION_TYPE_SETTING); + final List> settings = List.of(INDEX_REPLICATION_TYPE_SETTING); return settings.iterator(); } }, 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 6083e79dc9e20..99df67699e4d3 100644 --- a/server/src/main/java/org/opensearch/cluster/metadata/MetadataCreateIndexService.java +++ b/server/src/main/java/org/opensearch/cluster/metadata/MetadataCreateIndexService.java @@ -74,6 +74,7 @@ import org.opensearch.common.settings.IndexScopedSettings; import org.opensearch.common.settings.Setting; import org.opensearch.common.settings.Settings; +import org.opensearch.common.util.FeatureFlags; import org.opensearch.core.xcontent.NamedXContentRegistry; import org.opensearch.common.xcontent.XContentHelper; import org.opensearch.env.Environment; @@ -137,6 +138,7 @@ import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_REMOTE_TRANSLOG_STORE_REPOSITORY; import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_REPLICATION_TYPE; import static org.opensearch.cluster.metadata.Metadata.DEFAULT_REPLICA_COUNT_SETTING; +import static org.opensearch.common.util.FeatureFlags.REMOTE_STORE; import static org.opensearch.indices.IndicesService.CLUSTER_REMOTE_STORE_REPOSITORY_SETTING; import static org.opensearch.indices.IndicesService.CLUSTER_REMOTE_TRANSLOG_REPOSITORY_SETTING; import static org.opensearch.indices.IndicesService.CLUSTER_REMOTE_STORE_ENABLED_SETTING; @@ -902,8 +904,18 @@ 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); - updateRemoteStoreSettings(indexSettingsBuilder, request.settings(), settings); + if (isSystemIndex || IndexMetadata.INDEX_HIDDEN_SETTING.get(request.settings())) { + logger.warn( + "Setting replication.type: DOCUMENT will be used for Index until Segment Replication supports System and Hidden indices" + ); + indexSettingsBuilder.put(SETTING_REPLICATION_TYPE, ReplicationType.DOCUMENT); + if (FeatureFlags.isEnabled(REMOTE_STORE)) { + indexSettingsBuilder.put(SETTING_REMOTE_STORE_ENABLED, false); + } + } else { + updateReplicationStrategy(indexSettingsBuilder, request.settings(), settings); + updateRemoteStoreSettings(indexSettingsBuilder, request.settings(), settings); + } if (sourceMetadata != null) { assert request.resizeType() != null; @@ -943,19 +955,16 @@ 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)); + return; + } + if (INDEX_REPLICATION_TYPE_SETTING.exists(requestSettings) == true) { + settingsBuilder.put(SETTING_REPLICATION_TYPE, INDEX_REPLICATION_TYPE_SETTING.get(requestSettings)); + return; } + settingsBuilder.put(SETTING_REPLICATION_TYPE, CLUSTER_REPLICATION_TYPE_SETTING.getDefault(clusterSettings)); } /** @@ -971,20 +980,19 @@ private static void updateRemoteStoreSettings(Settings.Builder settingsBuilder, return; } - // Verify REPLICATION_TYPE cluster level setting is not conflicting with Remote Store - if (INDEX_REPLICATION_TYPE_SETTING.exists(requestSettings) == false - && CLUSTER_REPLICATION_TYPE_SETTING.get(clusterSettings).equals(ReplicationType.DOCUMENT)) { + // Verify index has replication type as SEGMENT + if (ReplicationType.DOCUMENT.equals(ReplicationType.parseString(settingsBuilder.get(SETTING_REPLICATION_TYPE)))) { throw new IllegalArgumentException( "Cannot enable [" + SETTING_REMOTE_STORE_ENABLED + "] when [" - + CLUSTER_REPLICATION_TYPE_SETTING.getKey() + + SETTING_REPLICATION_TYPE + "] is " + ReplicationType.DOCUMENT ); } - settingsBuilder.put(SETTING_REPLICATION_TYPE, ReplicationType.SEGMENT).put(SETTING_REMOTE_STORE_ENABLED, true); + settingsBuilder.put(SETTING_REMOTE_STORE_ENABLED, true); String remoteStoreRepo; if (Objects.equals(requestSettings.get(INDEX_REMOTE_STORE_ENABLED_SETTING.getKey()), "true")) { 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 21fab839fe5c6..067eed7b6b068 100644 --- a/server/src/test/java/org/opensearch/cluster/metadata/MetadataCreateIndexServiceTests.java +++ b/server/src/test/java/org/opensearch/cluster/metadata/MetadataCreateIndexServiceTests.java @@ -1237,7 +1237,7 @@ public void testRemoteStoreNoUserOverrideConflictingReplicationTypeIndexSettings ); assertThat( exc.getMessage(), - containsString("Cannot enable [index.remote_store.enabled] when [cluster.indices.replication.strategy] is DOCUMENT") + containsString("Cannot enable [index.remote_store.enabled] when [index.replication.type] is DOCUMENT") ); } @@ -1702,6 +1702,35 @@ public void testSystemIndexUsesDocumentReplication() { assertEquals(ReplicationType.DOCUMENT.toString(), indexSettings.get(SETTING_REPLICATION_TYPE)); } + public void testRemoteStoreDisabledForSystemIndices() { + Settings settings = Settings.builder() + .put(CLUSTER_REPLICATION_TYPE_SETTING.getKey(), ReplicationType.SEGMENT) + .put(CLUSTER_REMOTE_STORE_ENABLED_SETTING.getKey(), true) + .put(CLUSTER_REMOTE_STORE_REPOSITORY_SETTING.getKey(), "my-segment-repo-1") + .put(CLUSTER_REMOTE_TRANSLOG_STORE_ENABLED_SETTING.getKey(), true) + .put(CLUSTER_REMOTE_TRANSLOG_REPOSITORY_SETTING.getKey(), "my-translog-repo-1") + .build(); + FeatureFlagSetter.set(FeatureFlags.REMOTE_STORE); + 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 that remote store is disabled. + assertEquals(indexSettings.get(SETTING_REMOTE_STORE_ENABLED), "false"); + assertEquals(ReplicationType.DOCUMENT.toString(), indexSettings.get(SETTING_REPLICATION_TYPE)); + } + private IndexTemplateMetadata addMatchingTemplate(Consumer configurator) { IndexTemplateMetadata.Builder builder = templateMetadataBuilder("template1", "te*"); configurator.accept(builder);