Removing throwing of exception when remote store is used with system …

…indices. Signed-off-by: Rishikesh1159 <rishireddy1159@gmail.com>
opensearch-project · Jun 23, 2023 · 285630d · 285630d
1 parent 84dffac
commit 285630d
Show file tree

Hide file tree

Showing 2 changed files with 34 additions and 27 deletions.
diff --git 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
@@ -898,8 +898,13 @@ 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())) {
+            indexSettingsBuilder.put(SETTING_REPLICATION_TYPE, ReplicationType.DOCUMENT);
+            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 +944,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;