From 50b03b316d42d842068e10d3ab8413019eb9569a Mon Sep 17 00:00:00 2001
From: Marc Handalian <handalm@amazon.com>
Date: Mon, 19 Jun 2023 09:56:36 -0700
Subject: [PATCH] Add validation to index.search.idle.after when segment
 replication is enabled.

This change updates search idle setting to not be configurable on segrep indices with replicas.

Signed-off-by: Marc Handalian <handalm@amazon.com>
---
 .../action/search/TransportSearchIT.java      | 29 ++++++++++++++++
 .../org/opensearch/index/IndexSettings.java   | 33 +++++++++++++++++++
 2 files changed, 62 insertions(+)

diff --git a/server/src/internalClusterTest/java/org/opensearch/action/search/TransportSearchIT.java b/server/src/internalClusterTest/java/org/opensearch/action/search/TransportSearchIT.java
index 895d7ebea88b6..afc0fca8e41da 100644
--- a/server/src/internalClusterTest/java/org/opensearch/action/search/TransportSearchIT.java
+++ b/server/src/internalClusterTest/java/org/opensearch/action/search/TransportSearchIT.java
@@ -61,6 +61,7 @@
 import org.opensearch.index.query.RangeQueryBuilder;
 import org.opensearch.index.shard.IndexShard;
 import org.opensearch.indices.IndicesService;
+import org.opensearch.indices.replication.common.ReplicationType;
 import org.opensearch.plugins.Plugin;
 import org.opensearch.plugins.SearchPlugin;
 import org.opensearch.rest.RestStatus;
@@ -370,6 +371,34 @@ public void testSearchIdle() throws Exception {
         });
     }
 
+    public void testSearchIdle_SegmentReplication() {
+        // fail with replica count > 0
+        int numOfReplicas = 1;
+        internalCluster().ensureAtLeastNumDataNodes(numOfReplicas + 1);
+        final Settings.Builder settings = Settings.builder()
+            .put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, randomIntBetween(1, 5))
+            .put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, numOfReplicas)
+            .put(IndexMetadata.SETTING_REPLICATION_TYPE, ReplicationType.SEGMENT)
+            .put(IndexSettings.INDEX_SEARCH_IDLE_AFTER.getKey(), TimeValue.timeValueMillis(randomIntBetween(50, 500)));
+
+        expectThrows(IllegalArgumentException.class, () -> {
+            assertAcked(prepareCreate("test").setSettings(settings).setMapping("created_date", "type=date,format=yyyy-MM-dd"));
+        });
+
+        // setting allowed with 0 replicas.
+        settings.put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, 0);
+        assertAcked(prepareCreate("test").setSettings(settings).setMapping("created_date", "type=date,format=yyyy-MM-dd"));
+
+        // add a replica
+        assertAcked(
+            client().admin()
+                .indices()
+                .prepareUpdateSettings("test")
+                .setSettings(Settings.builder().put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, 1))
+        );
+
+    }
+
     public void testCircuitBreakerReduceFail() throws Exception {
         int numShards = randomIntBetween(1, 10);
         indexSomeDocs("test", numShards, numShards * 3);
diff --git a/server/src/main/java/org/opensearch/index/IndexSettings.java b/server/src/main/java/org/opensearch/index/IndexSettings.java
index 9c6613495ba80..b186f56ba8577 100644
--- a/server/src/main/java/org/opensearch/index/IndexSettings.java
+++ b/server/src/main/java/org/opensearch/index/IndexSettings.java
@@ -54,8 +54,10 @@
 import org.opensearch.search.pipeline.SearchPipelineService;
 
 import java.util.Collections;
+import java.util.Iterator;
 import java.util.List;
 import java.util.Locale;
+import java.util.Map;
 import java.util.Optional;
 import java.util.concurrent.TimeUnit;
 import java.util.function.Consumer;
@@ -70,6 +72,7 @@
 import static org.opensearch.index.mapper.MapperService.INDEX_MAPPING_NESTED_FIELDS_LIMIT_SETTING;
 import static org.opensearch.index.mapper.MapperService.INDEX_MAPPING_TOTAL_FIELDS_LIMIT_SETTING;
 import static org.opensearch.index.store.remote.directory.RemoteSnapshotDirectory.SEARCHABLE_SNAPSHOT_EXTENDED_COMPATIBILITY_MINIMUM_VERSION;
+import static org.opensearch.indices.IndicesService.CLUSTER_REPLICATION_TYPE_SETTING;
 
 /**
  * This class encapsulates all index level settings and handles settings updates.
@@ -122,6 +125,36 @@ public final class IndexSettings {
         "index.search.idle.after",
         TimeValue.timeValueSeconds(30),
         TimeValue.timeValueMinutes(0),
+        new Setting.Validator<>() {
+
+            @Override
+            public void validate(TimeValue value) {
+
+            }
+
+            @Override
+            public void validate(TimeValue value, Map<Setting<?>, Object> settings, boolean isPresent) {
+                if (isPresent) {
+                    final Object clusterSettingReplicationType = settings.get(CLUSTER_REPLICATION_TYPE_SETTING);
+                    final Object replicationType = settings.get(IndexMetadata.INDEX_REPLICATION_TYPE_SETTING);
+                    int numReplicas = (int) settings.get(IndexMetadata.INDEX_NUMBER_OF_REPLICAS_SETTING);
+                    if ((ReplicationType.SEGMENT.equals(clusterSettingReplicationType) || ReplicationType.SEGMENT.equals(replicationType))
+                        && numReplicas > 0) {
+                        throw new IllegalArgumentException("Shard idle is disabled for indices with replicas using Segment Replication");
+                    }
+                }
+            }
+
+            @Override
+            public Iterator<Setting<?>> settings() {
+                final List<Setting<?>> settings = List.of(
+                    CLUSTER_REPLICATION_TYPE_SETTING,
+                    IndexMetadata.INDEX_REPLICATION_TYPE_SETTING,
+                    IndexMetadata.INDEX_NUMBER_OF_REPLICAS_SETTING
+                );
+                return settings.iterator();
+            }
+        },
         Property.IndexScope,
         Property.Dynamic
     );