diff --git a/server/src/internalClusterTest/java/org/opensearch/remotestore/CreateRemoteIndexIT.java b/server/src/internalClusterTest/java/org/opensearch/remotestore/CreateRemoteIndexIT.java index cf53fa7fac2cd..46966e289e75e 100644 --- a/server/src/internalClusterTest/java/org/opensearch/remotestore/CreateRemoteIndexIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/remotestore/CreateRemoteIndexIT.java @@ -12,6 +12,8 @@ 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.unit.TimeValue; @@ -131,21 +133,30 @@ public void testDefaultRemoteStoreNoUserOverride() throws Exception { private static final String SYSTEM_INDEX_NAME = ".test-system-index"; public void testSystemIndexWithRemoteStoreClusterSetting() throws Exception { - IllegalArgumentException illegalArgumentException = expectThrows( - IllegalArgumentException.class, - () -> createIndex(SYSTEM_INDEX_NAME) - ); - assertThat( - illegalArgumentException.getMessage(), - containsString( - "Cannot enable [" - + SETTING_REMOTE_STORE_ENABLED - + "] when [" - + SETTING_REPLICATION_TYPE - + "] is " - + ReplicationType.DOCUMENT - ) - ); + 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 { 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 a2a11b6bc9271..0a49feec61621 100644 --- a/server/src/main/java/org/opensearch/cluster/metadata/MetadataCreateIndexService.java +++ b/server/src/main/java/org/opensearch/cluster/metadata/MetadataCreateIndexService.java @@ -72,6 +72,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.common.xcontent.XContentHelper; import org.opensearch.core.common.Strings; import org.opensearch.core.xcontent.NamedXContentRegistry; @@ -136,6 +137,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; @@ -898,8 +900,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; @@ -939,16 +951,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)); return; 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 206c8914a61ee..94a1ca9992cb6 100644 --- a/server/src/test/java/org/opensearch/cluster/metadata/MetadataCreateIndexServiceTests.java +++ b/server/src/test/java/org/opensearch/cluster/metadata/MetadataCreateIndexServiceTests.java @@ -1715,6 +1715,36 @@ 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);