Handle case where index level override is removed and index should pick up cluster level default

Author: linuxpi

server/src/internalClusterTest/java/org/opensearch/index/ClusterMaxMergesAtOnceIT.java

@@ -34,6 +34,7 @@
 
 import org.opensearch.action.admin.indices.get.GetIndexRequest;
 import org.opensearch.action.admin.indices.get.GetIndexResponse;
+import org.opensearch.action.admin.indices.settings.put.UpdateSettingsRequestBuilder;
 import org.opensearch.cluster.metadata.IndexMetadata;
 import org.opensearch.common.settings.Settings;
 import org.opensearch.core.index.Index;
@@ -90,8 +91,44 @@ public void testClusterLevelDefaultUpdatesMergePolicy() throws ExecutionExceptio
         getIndexResponse = client(clusterManagerName).admin().indices().getIndex(new GetIndexRequest()).get();
         indicesService = internalCluster().getInstance(IndicesService.class, randomFrom(dataNodes));
         uuid = getIndexResponse.getSettings().get(indexName).get(IndexMetadata.SETTING_INDEX_UUID);
-        IndexService newIndexService = indicesService.indexService(new Index(indexName, uuid));
+        IndexService secondIndexService = indicesService.indexService(new Index(indexName, uuid));
         assertEquals(20, ((OpenSearchTieredMergePolicy) indexService.getIndexSettings().getMergePolicy(true)).getMaxMergeAtOnce());
-        assertEquals(20, ((OpenSearchTieredMergePolicy) newIndexService.getIndexSettings().getMergePolicy(true)).getMaxMergeAtOnce());
+        assertEquals(20, ((OpenSearchTieredMergePolicy) secondIndexService.getIndexSettings().getMergePolicy(true)).getMaxMergeAtOnce());
+
+        // Create index with index level override in settings
+        indexName = "log-myindex-3";
+        createIndex(
+            indexName,
+            Settings.builder().put(TieredMergePolicyProvider.INDEX_MERGE_POLICY_MAX_MERGE_AT_ONCE_SETTING.getKey(), 15).build()
+        );
+        ensureYellowAndNoInitializingShards(indexName);
+        ensureGreen(indexName);
+        getIndexResponse = client(clusterManagerName).admin().indices().getIndex(new GetIndexRequest()).get();
+        indicesService = internalCluster().getInstance(IndicesService.class, randomFrom(dataNodes));
+        uuid = getIndexResponse.getSettings().get(indexName).get(IndexMetadata.SETTING_INDEX_UUID);
+        IndexService thirdIndexService = indicesService.indexService(new Index(indexName, uuid));
+        assertEquals(15, ((OpenSearchTieredMergePolicy) thirdIndexService.getIndexSettings().getMergePolicy(true)).getMaxMergeAtOnce());
+
+        // changing cluster level default should only affect indices without index level override
+        client(clusterManagerName).admin()
+            .cluster()
+            .prepareUpdateSettings()
+            .setTransientSettings(Settings.builder().put(CLUSTER_DEFAULT_INDEX_MAX_MERGE_AT_ONCE_SETTING.getKey(), 35))
+            .get();
+        assertEquals(35, ((OpenSearchTieredMergePolicy) indexService.getIndexSettings().getMergePolicy(true)).getMaxMergeAtOnce());
+        assertEquals(35, ((OpenSearchTieredMergePolicy) secondIndexService.getIndexSettings().getMergePolicy(true)).getMaxMergeAtOnce());
+        assertEquals(15, ((OpenSearchTieredMergePolicy) thirdIndexService.getIndexSettings().getMergePolicy(true)).getMaxMergeAtOnce());
+
+        // removing index level override should pick up the cluster level default
+        final UpdateSettingsRequestBuilder builder = client().admin().indices().prepareUpdateSettings(indexName);
+        builder.setSettings(
+            Settings.builder().putNull(TieredMergePolicyProvider.INDEX_MERGE_POLICY_MAX_MERGE_AT_ONCE_SETTING.getKey()).build()
+        );
+        builder.execute().actionGet();
+
+        assertEquals(35, ((OpenSearchTieredMergePolicy) indexService.getIndexSettings().getMergePolicy(true)).getMaxMergeAtOnce());
+        assertEquals(35, ((OpenSearchTieredMergePolicy) secondIndexService.getIndexSettings().getMergePolicy(true)).getMaxMergeAtOnce());
+        assertEquals(35, ((OpenSearchTieredMergePolicy) thirdIndexService.getIndexSettings().getMergePolicy(true)).getMaxMergeAtOnce());
+
     }
 }

server/src/main/java/org/opensearch/index/IndexService.java

@@ -198,7 +198,6 @@ public class IndexService extends AbstractIndexComponent implements IndicesClust
     private final CompositeIndexSettings compositeIndexSettings;
     private final Consumer<IndexShard> replicator;
     private final Function<ShardId, ReplicationStats> segmentReplicationStatsProvider;
-    private final Supplier<Integer> clusterDefaultMaxMergeAtOnce;
 
     public IndexService(
         IndexSettings indexSettings,
@@ -327,9 +326,7 @@ public IndexService(
         this.fileCache = fileCache;
         this.replicator = replicator;
         this.segmentReplicationStatsProvider = segmentReplicationStatsProvider;
-        this.clusterDefaultMaxMergeAtOnce = clusterDefaultMaxMergeAtOnceSupplier;
-
-        indexSettings.setMaxMergesAtOnce(this.clusterDefaultMaxMergeAtOnce.get());
+        indexSettings.setDefaultMaxMergesAtOnce(clusterDefaultMaxMergeAtOnceSupplier.get());
         updateFsyncTaskIfNecessary();
     }
 
@@ -1131,8 +1128,8 @@ private void updateReplicationTask() {
     /**
      * Called whenever the cluster level {@code cluster.default.index.max_merge_at_once} changes.
      */
-    public void onDefaultMaxMergeAtOnceChanged(int defaultMaxMergeAtOnce) {
-        indexSettings.setMaxMergesAtOnce(defaultMaxMergeAtOnce);
+    public void onDefaultMaxMergeAtOnceChanged(int newDefaultMaxMergeAtOnce) {
+        indexSettings.setDefaultMaxMergesAtOnce(newDefaultMaxMergeAtOnce);
     }
 
     /**

server/src/main/java/org/opensearch/index/IndexSettings.java

@@ -1096,7 +1096,7 @@ public IndexSettings(final IndexMetadata indexMetadata, final Settings nodeSetti
         );
         scopedSettings.addSettingsUpdateConsumer(
             TieredMergePolicyProvider.INDEX_MERGE_POLICY_MAX_MERGE_AT_ONCE_SETTING,
-            tieredMergePolicyProvider::setMaxMergesAtOnce
+            this::updateMaxMergesAtOnce
         );
         scopedSettings.addSettingsUpdateConsumer(
             TieredMergePolicyProvider.INDEX_MERGE_POLICY_MAX_MERGED_SEGMENT_SETTING,
@@ -1251,11 +1251,26 @@ private void setRefreshInterval(TimeValue timeValue) {
     }
 
     /**
-    * Update the maxMergesAtOnce if no index level override exists in index level settings
-    */
-    void setMaxMergesAtOnce(int newMaxMergesAtOnce) {
+     * Update the default maxMergesAtOnce
+     * 1. sets the new default in {@code TieredMergePolicyProvide}
+     * 2. sets the maxMergesAtOnce on the actual TieredMergePolicy used by the engine if no index level override exists
+     */
+    void setDefaultMaxMergesAtOnce(int newDefaultMaxMergesAtOnce) {
+        tieredMergePolicyProvider.setDefaultMaxMergesAtOnce(newDefaultMaxMergesAtOnce);
+        if (TieredMergePolicyProvider.INDEX_MERGE_POLICY_MAX_MERGE_AT_ONCE_SETTING.exists(getSettings()) == false) {
+            tieredMergePolicyProvider.setMaxMergesAtOnceToDefault();
+        }
+    }
+
+    /**
+     * Updates the maxMergesAtOnce for actual TieredMergePolicy used by the engine.
+     * Sets it to default maxMergesAtOnce if index level settings is being removed
+     */
+    void updateMaxMergesAtOnce(int newMaxMergesAtOnce) {
         if (TieredMergePolicyProvider.INDEX_MERGE_POLICY_MAX_MERGE_AT_ONCE_SETTING.exists(getSettings()) == false) {
             tieredMergePolicyProvider.setMaxMergesAtOnce(newMaxMergesAtOnce);
+        } else {
+            tieredMergePolicyProvider.setMaxMergesAtOnceToDefault();
         }
     }
 

server/src/main/java/org/opensearch/index/TieredMergePolicyProvider.java

@@ -134,6 +134,7 @@ public final class TieredMergePolicyProvider implements MergePolicyProvider {
 
     private final Logger logger;
     private final boolean mergesEnabled;
+    private int defaultMaxMergeAtOnce = 30;
 
     public static final double DEFAULT_EXPUNGE_DELETES_ALLOWED = 10d;
 
@@ -249,6 +250,21 @@ void setMaxMergesAtOnce(Integer maxMergeAtOnce) {
         tieredMergePolicy.setMaxMergeAtOnce(maxMergeAtOnce);
     }
 
+    /**
+     * Update the value for maxMergesAtOnce in TieredMergePolicy used by engine to default value.
+     * This would happen if index level override is being removed and we need to fallback to cluster level default
+     */
+    void setMaxMergesAtOnceToDefault() {
+        tieredMergePolicy.setMaxMergeAtOnce(defaultMaxMergeAtOnce);
+    }
+
+    /**
+     * Update the default value for maxMergesAtOnce. It is used when index level override is not present
+     */
+    void setDefaultMaxMergesAtOnce(Integer defaultMaxMergesAtOnce) {
+        this.defaultMaxMergeAtOnce = defaultMaxMergesAtOnce;
+    }
+
     void setFloorSegmentSetting(ByteSizeValue floorSegementSetting) {
         tieredMergePolicy.setFloorSegmentMB(floorSegementSetting.getMbFrac());
     }