[Segment Replication] [Revert] Remove Restriction of strictly using only docrep for system indices and hidden indices (#8502)

server/src/internalClusterTest/java/org/opensearch/indices/replication/SegmentReplicationClusterSettingIT.java

@@ -15,8 +15,16 @@
 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;
@@ -52,6 +60,40 @@ protected Settings nodeSettings(int nodeOrdinal) {
             .build();
     }
 
+    public static class TestPlugin extends Plugin implements SystemIndexPlugin {
+        @Override
+        public Collection<SystemIndexDescriptor> getSystemIndexDescriptors(Settings settings) {
+            return Collections.singletonList(
+                new SystemIndexDescriptor(SYSTEM_INDEX_NAME, "System index for [" + getTestClass().getName() + ']')
+            );
+        }
+    }
+
+    @Override
+    protected Collection<Class<? extends Plugin>> 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";
@@ -123,4 +165,28 @@ 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);
+    }
+
 }

server/src/internalClusterTest/java/org/opensearch/remotestore/CreateRemoteIndexIT.java

@@ -12,15 +12,24 @@
 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;
 import org.opensearch.common.util.FeatureFlags;
+import org.opensearch.indices.SystemIndexDescriptor;
 import org.opensearch.index.IndexSettings;
 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;
@@ -59,6 +68,20 @@ protected Settings nodeSettings(int nodeOriginal) {
         return builder.build();
     }
 
+    public static class TestPlugin extends Plugin implements SystemIndexPlugin {
+        @Override
+        public Collection<SystemIndexDescriptor> getSystemIndexDescriptors(Settings settings) {
+            return Collections.singletonList(
+                new SystemIndexDescriptor(SYSTEM_INDEX_NAME, "System index for [" + getTestClass().getName() + ']')
+            );
+        }
+    }
+
+    @Override
+    protected Collection<Class<? extends Plugin>> nodePlugins() {
+        return Arrays.asList(CreateRemoteIndexIT.TestPlugin.class);
+    }
+
     @Override
     protected Settings featureFlagSettings() {
         return Settings.builder().put(super.featureFlagSettings()).put(FeatureFlags.REMOTE_STORE, "true").build();
@@ -107,6 +130,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)

server/src/main/java/org/opensearch/cluster/metadata/MetadataCreateIndexService.java

@@ -73,6 +73,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;
@@ -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;
@@ -584,7 +586,8 @@ private ClusterState applyCreateIndexRequestWithV1Templates(
             settings,
             indexScopedSettings,
             shardLimitValidator,
-            indexSettingProviders
+            indexSettingProviders,
+            systemIndices.validateSystemIndex(request.index())
         );
         int routingNumShards = getIndexNumberOfRoutingShards(aggregatedIndexSettings, null);
         IndexMetadata tmpImd = buildAndValidateTemporaryIndexMetadata(currentState, aggregatedIndexSettings, request, routingNumShards);
@@ -648,7 +651,8 @@ private ClusterState applyCreateIndexRequestWithV2Template(
             settings,
             indexScopedSettings,
             shardLimitValidator,
-            indexSettingProviders
+            indexSettingProviders,
+            systemIndices.validateSystemIndex(request.index())
         );
         int routingNumShards = getIndexNumberOfRoutingShards(aggregatedIndexSettings, null);
         IndexMetadata tmpImd = buildAndValidateTemporaryIndexMetadata(currentState, aggregatedIndexSettings, request, routingNumShards);
@@ -728,7 +732,8 @@ private ClusterState applyCreateIndexRequestWithExistingMetadata(
             settings,
             indexScopedSettings,
             shardLimitValidator,
-            indexSettingProviders
+            indexSettingProviders,
+            sourceMetadata.isSystem()
         );
         final int routingNumShards = getIndexNumberOfRoutingShards(aggregatedIndexSettings, sourceMetadata);
         IndexMetadata tmpImd = buildAndValidateTemporaryIndexMetadata(currentState, aggregatedIndexSettings, request, routingNumShards);
@@ -811,7 +816,8 @@ static Settings aggregateIndexSettings(
         Settings settings,
         IndexScopedSettings indexScopedSettings,
         ShardLimitValidator shardLimitValidator,
-        Set<IndexSettingProvider> indexSettingProviders
+        Set<IndexSettingProvider> indexSettingProviders,
+        boolean isSystemIndex
     ) {
         // 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
@@ -901,8 +907,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);
-        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;