Deprecate indices.merge.scheduler.use_thread_pool setting (#129464)

This deprecates the `indices.merge.scheduler.use_thread_pool` setting
that was introduced in
#120869 because this
setting should not normally be used, unless instructed so by engineering
to get around temporary issues with the new threadpool-based merge
scheduler.

Author: albertzaharovits

docs/changelog/129464.yaml

@@ -0,0 +1,10 @@
+pr: 129464
+summary: Deprecate `indices.merge.scheduler.use_thread_pool` setting
+area: Engine
+type: deprecation
+issues: []
+deprecation:
+  title: Deprecate `indices.merge.scheduler.use_thread_pool` setting
+  area: Ingest
+  details: This deprecates the `indices.merge.scheduler.use_thread_pool` node setting that was introduced in #120869. This setting should not normally be used, unless instructed so by engineering to get around temporary issues with the new threadpool-based merge scheduler.
+  impact: There should be no impact to users since the setting was not released before its deprecation here (and is not documented).

server/src/main/java/org/elasticsearch/index/engine/ElasticsearchConcurrentMergeScheduler.java

@@ -31,7 +31,11 @@
 /**
  * An extension to the {@link ConcurrentMergeScheduler} that provides tracking on merge times, total
  * and current merges.
+ * @deprecated Replaced by {@link org.elasticsearch.index.engine.ThreadPoolMergeScheduler}. This merge scheduler
+ * implementation should only be used to get around unexpected issues with the {@link ThreadPoolMergeScheduler},
+ * which is the default one.
  */
+@Deprecated
 public class ElasticsearchConcurrentMergeScheduler extends ConcurrentMergeScheduler implements ElasticsearchMergeScheduler {
 
     protected final Logger logger;

server/src/main/java/org/elasticsearch/index/engine/ThreadPoolMergeScheduler.java

@@ -43,10 +43,22 @@
 import java.util.concurrent.atomic.AtomicLong;
 
 public class ThreadPoolMergeScheduler extends MergeScheduler implements ElasticsearchMergeScheduler {
+    /**
+     * This setting switches between the original {@link ElasticsearchConcurrentMergeScheduler}
+     * and the new {@link ThreadPoolMergeScheduler} merge scheduler implementations (the latter is switched ON by default).
+     * This setting is purposefully undocumented, because we expect that only the new {@link ThreadPoolMergeScheduler} implementation
+     * (which is enabled by default) be used from now on. Our users should not touch this setting in their deployments,
+     * unless consulting with engineering, because the original implementation should only be used (by setting this to {@code false})
+     * to get around unexpected issues with the new one.
+     * The setting is also <b>deprecated</b> in the hope that any unexpected issues with the new merge scheduler implementation are
+     * promptly resolved, such that, in the near future, there's never a need to switch to the original implementation,
+     * which will then be removed together with this setting.
+     */
     public static final Setting<Boolean> USE_THREAD_POOL_MERGE_SCHEDULER_SETTING = Setting.boolSetting(
         "indices.merge.scheduler.use_thread_pool",
         true,
-        Setting.Property.NodeScope
+        Setting.Property.NodeScope,
+        Setting.Property.Deprecated
     );
     private final ShardId shardId;
     private final MergeSchedulerConfig config;

server/src/test/java/org/elasticsearch/action/support/replication/TransportReplicationAllPermitsAcquisitionTests.java

@@ -391,6 +391,10 @@ void runWithPrimaryShardReference(final TransportReplicationAction.PrimaryShardR
                 }
             }
         }
+        assertWarnings(
+            "[indices.merge.scheduler.use_thread_pool] setting was deprecated in Elasticsearch and will be removed in a future release. "
+                + "See the breaking changes documentation for the next major version."
+        );
     }
 
     private void assertSuccessfulOperation(final TestAction action, final Response response) {

server/src/test/java/org/elasticsearch/index/engine/ThreadPoolMergeExecutorServiceDiskSpaceTests.java

@@ -67,6 +67,7 @@ public class ThreadPoolMergeExecutorServiceDiskSpaceTests extends ESTestCase {
     private static Settings settings;
     private static TestCapturingThreadPool testThreadPool;
     private static NodeEnvironment nodeEnvironment;
+    private static boolean setThreadPoolMergeSchedulerSetting;
 
     @BeforeClass
     public static void installMockUsableSpaceFS() throws Exception {
@@ -86,7 +87,8 @@ public static void installMockUsableSpaceFS() throws Exception {
             // the default of "5s" slows down testing
             .put(ThreadPoolMergeExecutorService.INDICES_MERGE_DISK_CHECK_INTERVAL_SETTING.getKey(), "50ms")
             .put(EsExecutors.NODE_PROCESSORS_SETTING.getKey(), mergeExecutorThreadCount);
-        if (randomBoolean()) {
+        setThreadPoolMergeSchedulerSetting = randomBoolean();
+        if (setThreadPoolMergeSchedulerSetting) {
             settingsBuilder.put(ThreadPoolMergeScheduler.USE_THREAD_POOL_MERGE_SCHEDULER_SETTING.getKey(), true);
         }
         settings = settingsBuilder.build();
@@ -520,6 +522,12 @@ public void testAvailableDiskSpaceMonitorSettingsUpdate() throws Exception {
                 }
             }, 5, TimeUnit.SECONDS);
         }
+        if (setThreadPoolMergeSchedulerSetting) {
+            assertWarnings(
+                "[indices.merge.scheduler.use_thread_pool] setting was deprecated in Elasticsearch "
+                    + "and will be removed in a future release. See the breaking changes documentation for the next major version."
+            );
+        }
     }
 
     public void testAbortingOrRunningMergeTaskHoldsUpBudget() throws Exception {
@@ -593,6 +601,12 @@ public void testAbortingOrRunningMergeTaskHoldsUpBudget() throws Exception {
                 assertThat(threadPoolMergeExecutorService.allDone(), is(true));
             });
         }
+        if (setThreadPoolMergeSchedulerSetting) {
+            assertWarnings(
+                "[indices.merge.scheduler.use_thread_pool] setting was deprecated in Elasticsearch "
+                    + "and will be removed in a future release. See the breaking changes documentation for the next major version."
+            );
+        }
     }
 
     public void testBackloggedMergeTasksDoNotHoldUpBudget() throws Exception {
@@ -731,6 +745,12 @@ && randomBoolean()) {
                 assertThat(threadPoolMergeExecutorService.allDone(), is(true));
             });
         }
+        if (setThreadPoolMergeSchedulerSetting) {
+            assertWarnings(
+                "[indices.merge.scheduler.use_thread_pool] setting was deprecated in Elasticsearch "
+                    + "and will be removed in a future release. See the breaking changes documentation for the next major version."
+            );
+        }
     }
 
     public void testUnavailableBudgetBlocksNewMergeTasksFromStartingExecution() throws Exception {
@@ -868,6 +888,12 @@ public void testUnavailableBudgetBlocksNewMergeTasksFromStartingExecution() thro
                 assertThat(threadPoolMergeExecutorService.allDone(), is(true));
             });
         }
+        if (setThreadPoolMergeSchedulerSetting) {
+            assertWarnings(
+                "[indices.merge.scheduler.use_thread_pool] setting was deprecated in Elasticsearch "
+                    + "and will be removed in a future release. See the breaking changes documentation for the next major version."
+            );
+        }
     }
 
     public void testMergeTasksAreUnblockedWhenMoreDiskSpaceBecomesAvailable() throws Exception {
@@ -1019,5 +1045,11 @@ public void testMergeTasksAreUnblockedWhenMoreDiskSpaceBecomesAvailable() throws
                 );
             });
         }
+        if (setThreadPoolMergeSchedulerSetting) {
+            assertWarnings(
+                "[indices.merge.scheduler.use_thread_pool] setting was deprecated in Elasticsearch "
+                    + "and will be removed in a future release. See the breaking changes documentation for the next major version."
+            );
+        }
     }
 }

server/src/test/java/org/elasticsearch/index/engine/ThreadPoolMergeExecutorServiceTests.java

@@ -211,6 +211,10 @@ public void testEnqueuedAndBackloggedMergesAreStillExecutedWhenThreadPoolIsShutd
         });
         assertThat(countingListener.aborted.get() + countingListener.completed.get(), equalTo(doneMergesCount.get()));
         assertThat(countingListener.aborted.get(), equalTo(abortedMergesCount.get()));
+        assertWarnings(
+            "[indices.merge.scheduler.use_thread_pool] setting was deprecated in Elasticsearch and will be removed in a future release. "
+                + "See the breaking changes documentation for the next major version."
+        );
     }
 
     public void testTargetIORateChangesWhenSubmittingMergeTasks() throws Exception {
@@ -298,6 +302,10 @@ public void testTargetIORateChangesWhenSubmittingMergeTasks() throws Exception {
             }
             assertBusy(() -> assertTrue(threadPoolMergeExecutorService.allDone()));
         }
+        assertWarnings(
+            "[indices.merge.scheduler.use_thread_pool] setting was deprecated in Elasticsearch and will be removed in a future release. "
+                + "See the breaking changes documentation for the next major version."
+        );
     }
 
     public void testIORateIsAdjustedForAllRunningMergeTasks() throws Exception {
@@ -386,6 +394,10 @@ public void testIORateIsAdjustedForAllRunningMergeTasks() throws Exception {
             }
             assertBusy(() -> assertTrue(threadPoolMergeExecutorService.allDone()));
         }
+        assertWarnings(
+            "[indices.merge.scheduler.use_thread_pool] setting was deprecated in Elasticsearch and will be removed in a future release. "
+                + "See the breaking changes documentation for the next major version."
+        );
     }
 
     public void testIORateAdjustedForSubmittedTasksWhenExecutionRateIsSpeedy() throws IOException {
@@ -567,6 +579,10 @@ public void testMergeTasksRunConcurrently() throws Exception {
             }
             assertBusy(() -> assertTrue(threadPoolMergeExecutorService.allDone()));
         }
+        assertWarnings(
+            "[indices.merge.scheduler.use_thread_pool] setting was deprecated in Elasticsearch and will be removed in a future release. "
+                + "See the breaking changes documentation for the next major version."
+        );
     }
 
     public void testThreadPoolStatsWithBackloggedMergeTasks() throws Exception {
@@ -626,6 +642,10 @@ public void testThreadPoolStatsWithBackloggedMergeTasks() throws Exception {
                 assertTrue(threadPoolMergeExecutorService.allDone());
             });
         }
+        assertWarnings(
+            "[indices.merge.scheduler.use_thread_pool] setting was deprecated in Elasticsearch and will be removed in a future release. "
+                + "See the breaking changes documentation for the next major version."
+        );
     }
 
     public void testBackloggedMergeTasksExecuteExactlyOnce() throws Exception {
@@ -697,6 +717,10 @@ public void testBackloggedMergeTasksExecuteExactlyOnce() throws Exception {
                 assertTrue(threadPoolMergeExecutorService.allDone());
             });
         }
+        assertWarnings(
+            "[indices.merge.scheduler.use_thread_pool] setting was deprecated in Elasticsearch and will be removed in a future release. "
+                + "See the breaking changes documentation for the next major version."
+        );
     }
 
     public void testMergeTasksExecuteInSizeOrder() throws IOException {

server/src/test/java/org/elasticsearch/index/replication/IndexLevelReplicationTests.java

@@ -666,6 +666,10 @@ public void testLateDeliveryAfterGCTriggeredOnReplica() throws Exception {
             indexOnReplica(indexRequest, shards, replica);  // index arrives on replica lately.
             shards.assertAllEqual(0);
         }
+        assertWarnings(
+            "[indices.merge.scheduler.use_thread_pool] setting was deprecated in Elasticsearch and will be removed in a future release. "
+                + "See the breaking changes documentation for the next major version."
+        );
     }
 
     private void updateGCDeleteCycle(IndexShard shard, TimeValue interval) {

server/src/test/java/org/elasticsearch/index/shard/RefreshListenersTests.java

@@ -186,6 +186,10 @@ public void onFailedEngine(String reason, @Nullable Exception e) {
 
     @After
     public void tearDownListeners() throws Exception {
+        assertWarnings(
+            "[indices.merge.scheduler.use_thread_pool] setting was deprecated in Elasticsearch and will be removed in a future release. "
+                + "See the breaking changes documentation for the next major version."
+        );
         IOUtils.close(engine, store, nodeEnvironment, () -> terminate(threadPool));
     }
 

x-pack/plugin/ccr/src/test/java/org/elasticsearch/xpack/ccr/repository/CcrRestoreSourceServiceTests.java

@@ -24,6 +24,7 @@
 import org.elasticsearch.index.store.Store;
 import org.elasticsearch.index.store.StoreFileMetadata;
 import org.elasticsearch.xpack.ccr.CcrSettings;
+import org.junit.After;
 import org.junit.Before;
 
 import java.io.IOException;
@@ -49,6 +50,14 @@ public void setUp() throws Exception {
         restoreSourceService = new CcrRestoreSourceService(taskQueue.getThreadPool(), new CcrSettings(Settings.EMPTY, clusterSettings));
     }
 
+    @After
+    public void assertWarnings() {
+        assertWarnings(
+            "[indices.merge.scheduler.use_thread_pool] setting was deprecated in Elasticsearch and will be removed in a future release. "
+                + "See the breaking changes documentation for the next major version."
+        );
+    }
+
     public void testOpenSession() throws IOException {
         IndexShard indexShard1 = newStartedShard(true);
         IndexShard indexShard2 = newStartedShard(true);