Fix template setting override for replication type

Signed-off-by: Kunal Kotwani <kkotwani@amazon.com>
kotwanikunal · Dec 1, 2023 · 5569cd8 · 5569cd8
1 parent edf7861
commit 5569cd8
Show file tree

Hide file tree

Showing 3 changed files with 53 additions and 9 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -172,6 +172,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 - Fix for stuck update action in a bulk with `retry_on_conflict` property ([#11152](https://github.com/opensearch-project/OpenSearch/issues/11152))
 - Remove shadowJar from `lang-painless` module publication ([#11369](https://github.com/opensearch-project/OpenSearch/issues/11369))
 - Fix remote shards balancer and remove unused variables ([#11167](https://github.com/opensearch-project/OpenSearch/pull/11167))
+- Fix template setting override for replication type ([#11417](https://github.com/opensearch-project/OpenSearch/pull/11417))
 
 ### Security
 

diff --git a/server/src/main/java/org/opensearch/cluster/metadata/MetadataCreateIndexService.java b/server/src/main/java/org/opensearch/cluster/metadata/MetadataCreateIndexService.java
@@ -891,7 +891,7 @@ static Settings aggregateIndexSettings(
         indexSettingsBuilder.put(IndexMetadata.SETTING_INDEX_PROVIDED_NAME, request.getProvidedName());
         indexSettingsBuilder.put(SETTING_INDEX_UUID, UUIDs.randomBase64UUID());
 
-        updateReplicationStrategy(indexSettingsBuilder, request.settings(), settings);
+        updateReplicationStrategy(indexSettingsBuilder, request.settings(), settings, combinedTemplateSettings, sourceMetadata);
         updateRemoteStoreSettings(indexSettingsBuilder, settings);
 
         if (sourceMetadata != null) {
@@ -934,17 +934,38 @@ static Settings aggregateIndexSettings(
      * @param settingsBuilder index settings builder to be updated with relevant settings
      * @param requestSettings settings passed in during index create request
      * @param clusterSettings cluster level settings
+     * @param combinedTemplateSettings combined template settings which satisfy the index
+     *
      */
-    private static void updateReplicationStrategy(Settings.Builder settingsBuilder, Settings requestSettings, Settings clusterSettings) {
-        if (INDEX_REPLICATION_TYPE_SETTING.exists(requestSettings)) {
-            settingsBuilder.put(SETTING_REPLICATION_TYPE, INDEX_REPLICATION_TYPE_SETTING.get(requestSettings));
-        } else if (CLUSTER_REPLICATION_TYPE_SETTING.exists(clusterSettings)) {
-            settingsBuilder.put(SETTING_REPLICATION_TYPE, CLUSTER_REPLICATION_TYPE_SETTING.get(clusterSettings));
-        } else if (isRemoteStoreAttributePresent(clusterSettings)) {
+    private static void updateReplicationStrategy(
+        Settings.Builder settingsBuilder,
+        Settings requestSettings,
+        Settings clusterSettings,
+        Settings combinedTemplateSettings,
+        IndexMetadata sourceMetadata
+    ) {
+        // The replication setting is applied in the following order:
+        // 1. For remote store clusters, SegRep is the only replication type supported
+        // 2. For non-remote store clusters, the order is as follows -
+        // i. Explicit index creation request parameter
+        // ii. Source index metadata for replication type property
+        // iii. Template property for replication type
+        // iv. Default cluster level setting
+
+        if (isRemoteStoreAttributePresent(clusterSettings)) {
             settingsBuilder.put(SETTING_REPLICATION_TYPE, ReplicationType.SEGMENT);
-        } else {
-            settingsBuilder.put(SETTING_REPLICATION_TYPE, CLUSTER_REPLICATION_TYPE_SETTING.getDefault(clusterSettings));
+            return;
+        }
+
+        ReplicationType indexReplicationType = CLUSTER_REPLICATION_TYPE_SETTING.get(clusterSettings);
+        if (INDEX_REPLICATION_TYPE_SETTING.exists(requestSettings)) {
+            indexReplicationType = INDEX_REPLICATION_TYPE_SETTING.get(requestSettings);
+        } else if (sourceMetadata != null && INDEX_REPLICATION_TYPE_SETTING.exists(sourceMetadata.getSettings())) {
+            indexReplicationType = INDEX_REPLICATION_TYPE_SETTING.get(sourceMetadata.getSettings());
+        } else if (INDEX_REPLICATION_TYPE_SETTING.exists(combinedTemplateSettings)) {
+            indexReplicationType = INDEX_REPLICATION_TYPE_SETTING.get(combinedTemplateSettings);
         }
+        settingsBuilder.put(SETTING_REPLICATION_TYPE, indexReplicationType);
     }
 
     /**

diff --git a/server/src/test/java/org/opensearch/cluster/metadata/MetadataCreateIndexServiceTests.java b/server/src/test/java/org/opensearch/cluster/metadata/MetadataCreateIndexServiceTests.java
@@ -117,6 +117,7 @@
 import static org.opensearch.cluster.metadata.IndexMetadata.INDEX_NUMBER_OF_ROUTING_SHARDS_SETTING;
 import static org.opensearch.cluster.metadata.IndexMetadata.INDEX_NUMBER_OF_SHARDS_SETTING;
 import static org.opensearch.cluster.metadata.IndexMetadata.INDEX_READ_ONLY_BLOCK;
+import static org.opensearch.cluster.metadata.IndexMetadata.INDEX_REPLICATION_TYPE_SETTING;
 import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_NUMBER_OF_REPLICAS;
 import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_NUMBER_OF_SHARDS;
 import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_READ_ONLY;
@@ -1217,6 +1218,27 @@ public void testvalidateIndexSettings() {
         threadPool.shutdown();
     }
 
+    public void testIndexTemplateReplicationType() {
+        Settings templateSettings = Settings.builder().put(INDEX_REPLICATION_TYPE_SETTING.getKey(), ReplicationType.SEGMENT).build();
+
+        request = new CreateIndexClusterStateUpdateRequest("create index", "test", "test");
+        final Settings.Builder requestSettings = Settings.builder();
+        request.settings(requestSettings.build());
+        Settings indexSettings = aggregateIndexSettings(
+            ClusterState.EMPTY_STATE,
+            request,
+            templateSettings,
+            null,
+            Settings.EMPTY,
+            IndexScopedSettings.DEFAULT_SCOPED_SETTINGS,
+            randomShardLimitService(),
+            Collections.emptySet(),
+            clusterSettings
+        );
+        assertNotEquals(ReplicationType.SEGMENT, clusterSettings.get(CLUSTER_REPLICATION_TYPE_SETTING));
+        assertEquals(ReplicationType.SEGMENT.toString(), indexSettings.get(INDEX_REPLICATION_TYPE_SETTING.getKey()));
+    }
+
     public void testRemoteStoreNoUserOverrideExceptReplicationTypeSegmentIndexSettings() {
         Settings settings = Settings.builder()
             .put(CLUSTER_REPLICATION_TYPE_SETTING.getKey(), ReplicationType.DOCUMENT)