Disabled auto_expand_replicas setting when search replica is enabled

CHANGELOG.md

@@ -27,6 +27,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 - Introduce a setting to disable download of full cluster state from remote on term mismatch([#16798](https://github.com/opensearch-project/OpenSearch/pull/16798/))
 - Added ability to retrieve value from DocValues in a flat_object filed([#16802](https://github.com/opensearch-project/OpenSearch/pull/16802))
 - Introduce framework for auxiliary transports and an experimental gRPC transport plugin ([#16534](https://github.com/opensearch-project/OpenSearch/pull/16534))
+- Disabled auto_expand_replicas setting when search replica is enabled ([#16916](https://github.com/opensearch-project/OpenSearch/pull/16916))
 
 ### Dependencies
 - Bump `com.google.cloud:google-cloud-core-http` from 2.23.0 to 2.47.0 ([#16504](https://github.com/opensearch-project/OpenSearch/pull/16504))

server/src/internalClusterTest/java/org/opensearch/cluster/metadata/AutoExpandWithSearchReplicaIT.java

@@ -0,0 +1,113 @@
+/*
+ * SPDX-License-Identifier: Apache-2.0
+ *
+ * The OpenSearch Contributors require contributions made to
+ * this file be licensed under the Apache-2.0 license or a
+ * compatible open source license.
+ */
+
+package org.opensearch.cluster.metadata;
+
+import org.opensearch.action.admin.indices.settings.get.GetSettingsResponse;
+import org.opensearch.common.settings.Settings;
+import org.opensearch.common.util.FeatureFlags;
+import org.opensearch.indices.replication.common.ReplicationType;
+import org.opensearch.test.OpenSearchIntegTestCase;
+
+public class AutoExpandWithSearchReplicaIT extends OpenSearchIntegTestCase {
+
+    private static final String INDEX_NAME = "test-idx-1";
+
+    @Override
+    protected Settings featureFlagSettings() {
+        return Settings.builder().put(super.featureFlagSettings()).put(FeatureFlags.READER_WRITER_SPLIT_EXPERIMENTAL, true).build();
+    }
+
+    public void testEnableAutoExpandWhenSearchReplicaActive() {
+        internalCluster().startDataOnlyNodes(3);
+        createIndex(
+            INDEX_NAME,
+            Settings.builder()
+                .put("number_of_shards", 1)
+                .put("number_of_replicas", 1)
+                .put("number_of_search_only_replicas", 1)
+                .put(IndexMetadata.SETTING_REPLICATION_TYPE, ReplicationType.SEGMENT)
+                .build()
+        );
+        ensureGreen(INDEX_NAME);
+
+        expectThrows(IllegalArgumentException.class, () -> {
+            client().admin()
+                .indices()
+                .prepareUpdateSettings(INDEX_NAME)
+                .setSettings(Settings.builder().put("index.auto_expand_replicas", "0-1"))
+                .execute()
+                .actionGet();
+        });
+
+        client().admin()
+            .indices()
+            .prepareUpdateSettings(INDEX_NAME)
+            .setSettings(Settings.builder().put("index.number_of_search_only_replicas", "0"))
+            .execute()
+            .actionGet();
+
+        client().admin()
+            .indices()
+            .prepareUpdateSettings(INDEX_NAME)
+            .setSettings(Settings.builder().put("index.auto_expand_replicas", "0-1"))
+            .execute()
+            .actionGet();
+
+        GetSettingsResponse response = client().admin().indices().prepareGetSettings(INDEX_NAME).execute().actionGet();
+
+        assertEquals("0-1", response.getSetting(INDEX_NAME, "index.auto_expand_replicas"));
+    }
+
+    public void testEnableSearchReplicaWithAutoExpandActive() {
+        internalCluster().startDataOnlyNodes(3);
+        createIndex(
+            INDEX_NAME,
+            Settings.builder()
+                .put("number_of_shards", 1)
+                .put("number_of_replicas", 1)
+                .put(IndexMetadata.SETTING_REPLICATION_TYPE, ReplicationType.SEGMENT)
+                .build()
+        );
+        ensureGreen(INDEX_NAME);
+
+        client().admin()
+            .indices()
+            .prepareUpdateSettings(INDEX_NAME)
+            .setSettings(Settings.builder().put("index.auto_expand_replicas", "0-1"))
+            .execute()
+            .actionGet();
+
+        expectThrows(IllegalArgumentException.class, () -> {
+            client().admin()
+                .indices()
+                .prepareUpdateSettings(INDEX_NAME)
+                .setSettings(Settings.builder().put("index.number_of_search_only_replicas", "1"))
+                .execute()
+                .actionGet();
+        });
+
+        client().admin()
+            .indices()
+            .prepareUpdateSettings(INDEX_NAME)
+            .setSettings(Settings.builder().put("index.auto_expand_replicas", false))
+            .execute()
+            .actionGet();
+
+        client().admin()
+            .indices()
+            .prepareUpdateSettings(INDEX_NAME)
+            .setSettings(Settings.builder().put("index.number_of_search_only_replicas", "1"))
+            .execute()
+            .actionGet();
+
+        GetSettingsResponse response = client().admin().indices().prepareGetSettings(INDEX_NAME).execute().actionGet();
+
+        assertEquals("1", response.getSetting(INDEX_NAME, "index.number_of_search_only_replicas"));
+    }
+}

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

@@ -1469,6 +1469,9 @@ public void validateIndexSettings(String indexName, final Settings settings, fin
         throws IndexCreationException {
         List<String> validationErrors = getIndexSettingsValidationErrors(settings, forbidPrivateIndexSettings, indexName);
         validateIndexReplicationTypeSettings(settings, clusterService.getClusterSettings()).ifPresent(validationErrors::add);
+        if (FeatureFlags.isEnabled(FeatureFlags.READER_WRITER_SPLIT_EXPERIMENTAL_SETTING)) {
+            validateAutoExpandReplicaConflictInRequest(settings).ifPresent(validationErrors::add);
+        }
         validateErrors(indexName, validationErrors);
     }
 
@@ -1738,6 +1741,63 @@ public static int calculateNumRoutingShards(int numShards, Version indexVersionC
         return numShards * 1 << numSplits;
     }
 
+    /**
+     * Validates that the settings do not conflict with each other.
+     *
+     * @param requestSettings settings passed in during index create request
+     * @return the validation error if there is one, otherwise {@code Optional.empty()}
+     */
+    public static Optional<String> validateAutoExpandReplicaConflictInRequest(Settings requestSettings) {
+        boolean hasRequestAutoExpand = AutoExpandReplicas.SETTING.get(requestSettings).isEnabled();
+        boolean hasRequestSearchReplica = INDEX_NUMBER_OF_SEARCH_REPLICAS_SETTING.get(requestSettings) > 0;
+        if (hasRequestAutoExpand && hasRequestSearchReplica) {
+            return Optional.of(
+                "Cannot set both ["
+                    + SETTING_AUTO_EXPAND_REPLICAS
+                    + "] and ["
+                    + SETTING_NUMBER_OF_SEARCH_REPLICAS
+                    + "]. These settings are mutually exclusive."
+            );
+        }
+        return Optional.empty();
+    }
+
+    /**
+     * Validates that the settings do not conflict with each other.
+     * Check conflicts between request and existing settings
+     * @param requestSettings
+     * @param indexSettings
+     * @return
+     */
+    public static Optional<String> validateAutoExpandReplicaConflictWithIndex(Settings requestSettings, Settings indexSettings) {
+        boolean hasRequestAutoExpand = AutoExpandReplicas.SETTING.get(requestSettings).isEnabled();
+        boolean hasRequestSearchReplica = INDEX_NUMBER_OF_SEARCH_REPLICAS_SETTING.get(requestSettings) > 0;
+        boolean hasIndexAutoExpand = AutoExpandReplicas.SETTING.get(indexSettings).isEnabled();
+        boolean hasIndexSearchReplica = INDEX_NUMBER_OF_SEARCH_REPLICAS_SETTING.get(indexSettings) > 0;
+
+        if (hasRequestAutoExpand && hasIndexSearchReplica) {
+            return Optional.of(
+                "Cannot set ["
+                    + AutoExpandReplicas.SETTING.getKey()
+                    + "] because ["
+                    + INDEX_NUMBER_OF_SEARCH_REPLICAS_SETTING.getKey()
+                    + "] is set"
+                    + ". These settings are mutually exclusive."
+            );
+        }
+
+        if (hasRequestSearchReplica && hasIndexAutoExpand) {
+            return Optional.of(
+                "Cannot set ["
+                    + INDEX_NUMBER_OF_SEARCH_REPLICAS_SETTING.getKey()
+                    + "] because ["
+                    + AutoExpandReplicas.SETTING.getKey()
+                    + "] is configured. These settings are mutually exclusive."
+            );
+        }
+        return Optional.empty();
+    }
+
     public static void validateTranslogRetentionSettings(Settings indexSettings) {
         if (IndexSettings.INDEX_SOFT_DELETES_SETTING.get(indexSettings)) {
             if (IndexSettings.INDEX_TRANSLOG_RETENTION_AGE_SETTING.exists(indexSettings)

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

@@ -79,6 +79,8 @@
 import static org.opensearch.action.support.ContextPreservingActionListener.wrapPreservingContext;
 import static org.opensearch.cluster.metadata.IndexMetadata.INDEX_REPLICATION_TYPE_SETTING;
 import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_NUMBER_OF_SEARCH_REPLICAS;
+import static org.opensearch.cluster.metadata.MetadataCreateIndexService.validateAutoExpandReplicaConflictInRequest;
+import static org.opensearch.cluster.metadata.MetadataCreateIndexService.validateAutoExpandReplicaConflictWithIndex;
 import static org.opensearch.cluster.metadata.MetadataCreateIndexService.validateOverlap;
 import static org.opensearch.cluster.metadata.MetadataCreateIndexService.validateRefreshIntervalSettings;
 import static org.opensearch.cluster.metadata.MetadataCreateIndexService.validateTranslogDurabilitySettings;
@@ -229,6 +231,12 @@ public ClusterState execute(ClusterState currentState) {
                             metadata.getSettings()
                         ).ifPresent(validationErrors::add);
 
+                        if (FeatureFlags.isEnabled(FeatureFlags.READER_WRITER_SPLIT_EXPERIMENTAL_SETTING)) {
+                            validateAutoExpandReplicaConflictInRequest(normalizedSettings).ifPresent(validationErrors::add);
+                            validateAutoExpandReplicaConflictWithIndex(normalizedSettings, metadata.getSettings()).ifPresent(
+                                validationErrors::add
+                            );
+                        }
                     }
 
                     if (validationErrors.size() > 0) {

server/src/test/java/org/opensearch/cluster/metadata/DisableAutoExpandReplicaForSearchReplicaTests.java

@@ -0,0 +1,84 @@
+/*
+ * SPDX-License-Identifier: Apache-2.0
+ *
+ * The OpenSearch Contributors require contributions made to
+ * this file be licensed under the Apache-2.0 license or a
+ * compatible open source license.
+ */
+
+package org.opensearch.cluster.metadata;
+
+import org.opensearch.common.settings.Settings;
+import org.opensearch.test.OpenSearchTestCase;
+
+/**
+ * AutoExpandReplica Disabled for Search Replica
+ */
+public class DisableAutoExpandReplicaForSearchReplicaTests extends OpenSearchTestCase {
+
+    public void testWhenAutoExpandReplicaAndSearchReplicaIsSetInTheRequest() {
+        Settings conflictRequestSettings = Settings.builder()
+            .put("index.number_of_search_only_replicas", 1)
+            .put("index.auto_expand_replicas", "0-all")
+            .build();
+
+        assertEquals(
+            "Cannot set both [index.auto_expand_replicas] and [index.number_of_search_only_replicas]. These settings are mutually exclusive.",
+            MetadataCreateIndexService.validateAutoExpandReplicaConflictInRequest(conflictRequestSettings).get()
+        );
+    }
+
+    public void testWhenOnlyAutoExpandReplicaIsSetInTheRequest() {
+        Settings reqestSettings = Settings.builder()
+            .put("index.number_of_search_only_replicas", 0)
+            .put("index.auto_expand_replicas", "0-all")
+            .build();
+
+        assertTrue(MetadataCreateIndexService.validateAutoExpandReplicaConflictInRequest(reqestSettings).isEmpty());
+    }
+
+    public void testWhenAutoExpandReplicaEnabledForTheIndex() {
+        Settings reqestSettings = Settings.builder().put("index.number_of_search_only_replicas", 1).build();
+
+        Settings indexSettings = Settings.builder().put("index.auto_expand_replicas", "0-all").build();
+
+        assertEquals(
+            "Cannot set [index.number_of_search_only_replicas] because [index.auto_expand_replicas] is configured. These settings are mutually exclusive.",
+            MetadataCreateIndexService.validateAutoExpandReplicaConflictWithIndex(reqestSettings, indexSettings).get()
+        );
+    }
+
+    public void testWhenSearchReplicaEnabledForTheIndex() {
+        Settings reqestSettings = Settings.builder().put("index.auto_expand_replicas", "0-all").build();
+
+        Settings indexSettings = Settings.builder().put("index.number_of_search_only_replicas", 1).build();
+
+        assertEquals(
+            "Cannot set [index.auto_expand_replicas] because [index.number_of_search_only_replicas] is set. These settings are mutually exclusive.",
+            MetadataCreateIndexService.validateAutoExpandReplicaConflictWithIndex(reqestSettings, indexSettings).get()
+        );
+    }
+
+    public void testWhenSearchReplicaSetZeroForTheIndex() {
+        Settings reqestSettings = Settings.builder().put("index.auto_expand_replicas", "0-all").build();
+
+        Settings indexSettings = Settings.builder().put("index.number_of_search_only_replicas", 0).build();
+
+        assertTrue(MetadataCreateIndexService.validateAutoExpandReplicaConflictWithIndex(reqestSettings, indexSettings).isEmpty());
+    }
+
+    public void testIfSearchReplicaEnabledEnablingAutoExpandReplicasAndDisablingSearchReplicasNotPossibleInSingleRequest() {
+        Settings reqestSettings = Settings.builder()
+            .put("index.auto_expand_replicas", "0-all")
+            .put("index.number_of_search_only_replicas", 0)
+            .build();
+
+        Settings indexSettings = Settings.builder().put("index.number_of_search_only_replicas", 1).build();
+
+        assertEquals(
+            "Cannot set [index.auto_expand_replicas] because [index.number_of_search_only_replicas] "
+                + "is set. These settings are mutually exclusive.",
+            MetadataCreateIndexService.validateAutoExpandReplicaConflictWithIndex(reqestSettings, indexSettings).get()
+        );
+    }
+}