diff --git a/docs/changelog/129464.yaml b/docs/changelog/129464.yaml new file mode 100644 index 0000000000000..c4724242454d5 --- /dev/null +++ b/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). diff --git a/server/src/main/java/org/elasticsearch/index/engine/ElasticsearchConcurrentMergeScheduler.java b/server/src/main/java/org/elasticsearch/index/engine/ElasticsearchConcurrentMergeScheduler.java index 90f8e6adab73d..0603308ff52b1 100644 --- a/server/src/main/java/org/elasticsearch/index/engine/ElasticsearchConcurrentMergeScheduler.java +++ b/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; diff --git a/server/src/main/java/org/elasticsearch/index/engine/ThreadPoolMergeScheduler.java b/server/src/main/java/org/elasticsearch/index/engine/ThreadPoolMergeScheduler.java index 653ad508a9057..c01325e095354 100644 --- a/server/src/main/java/org/elasticsearch/index/engine/ThreadPoolMergeScheduler.java +++ b/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 deprecated 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 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; diff --git a/server/src/test/java/org/elasticsearch/action/support/replication/TransportReplicationAllPermitsAcquisitionTests.java b/server/src/test/java/org/elasticsearch/action/support/replication/TransportReplicationAllPermitsAcquisitionTests.java index 7e0d7063c417e..eb1683970e2df 100644 --- a/server/src/test/java/org/elasticsearch/action/support/replication/TransportReplicationAllPermitsAcquisitionTests.java +++ b/server/src/test/java/org/elasticsearch/action/support/replication/TransportReplicationAllPermitsAcquisitionTests.java @@ -391,6 +391,9 @@ 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." + ); } private void assertSuccessfulOperation(final TestAction action, final Response response) { diff --git a/server/src/test/java/org/elasticsearch/index/engine/ThreadPoolMergeExecutorServiceDiskSpaceTests.java b/server/src/test/java/org/elasticsearch/index/engine/ThreadPoolMergeExecutorServiceDiskSpaceTests.java index 77b0ea0517364..e8f7c847da063 100644 --- a/server/src/test/java/org/elasticsearch/index/engine/ThreadPoolMergeExecutorServiceDiskSpaceTests.java +++ b/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,11 @@ 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." + ); + } } public void testAbortingOrRunningMergeTaskHoldsUpBudget() throws Exception { @@ -593,6 +600,11 @@ 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." + ); + } } public void testBackloggedMergeTasksDoNotHoldUpBudget() throws Exception { @@ -731,6 +743,11 @@ && 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." + ); + } } public void testUnavailableBudgetBlocksNewMergeTasksFromStartingExecution() throws Exception { @@ -868,6 +885,11 @@ 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." + ); + } } public void testMergeTasksAreUnblockedWhenMoreDiskSpaceBecomesAvailable() throws Exception { @@ -1019,6 +1041,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." + ); + } } private static T getLast(final Iterable elements) { diff --git a/server/src/test/java/org/elasticsearch/index/engine/ThreadPoolMergeExecutorServiceTests.java b/server/src/test/java/org/elasticsearch/index/engine/ThreadPoolMergeExecutorServiceTests.java index e3c40acf07cb7..8ea1d0aa742ec 100644 --- a/server/src/test/java/org/elasticsearch/index/engine/ThreadPoolMergeExecutorServiceTests.java +++ b/server/src/test/java/org/elasticsearch/index/engine/ThreadPoolMergeExecutorServiceTests.java @@ -211,6 +211,9 @@ 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." + ); } public void testTargetIORateChangesWhenSubmittingMergeTasks() throws Exception { @@ -298,6 +301,9 @@ 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." + ); } public void testIORateIsAdjustedForAllRunningMergeTasks() throws Exception { @@ -386,6 +392,9 @@ 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." + ); } public void testIORateAdjustedForSubmittedTasksWhenExecutionRateIsSpeedy() throws IOException { @@ -567,6 +576,9 @@ 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." + ); } public void testThreadPoolStatsWithBackloggedMergeTasks() throws Exception { @@ -626,6 +638,9 @@ 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." + ); } public void testBackloggedMergeTasksExecuteExactlyOnce() throws Exception { @@ -697,6 +712,9 @@ 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." + ); } public void testMergeTasksExecuteInSizeOrder() throws IOException { diff --git a/server/src/test/java/org/elasticsearch/index/replication/IndexLevelReplicationTests.java b/server/src/test/java/org/elasticsearch/index/replication/IndexLevelReplicationTests.java index 4c6b912e5d3a3..14edd6bab8862 100644 --- a/server/src/test/java/org/elasticsearch/index/replication/IndexLevelReplicationTests.java +++ b/server/src/test/java/org/elasticsearch/index/replication/IndexLevelReplicationTests.java @@ -666,6 +666,9 @@ 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." + ); } private void updateGCDeleteCycle(IndexShard shard, TimeValue interval) { diff --git a/server/src/test/java/org/elasticsearch/index/shard/RefreshListenersTests.java b/server/src/test/java/org/elasticsearch/index/shard/RefreshListenersTests.java index 3643659047e78..85e2965116109 100644 --- a/server/src/test/java/org/elasticsearch/index/shard/RefreshListenersTests.java +++ b/server/src/test/java/org/elasticsearch/index/shard/RefreshListenersTests.java @@ -185,6 +185,9 @@ 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." + ); IOUtils.close(engine, store, nodeEnvironment, () -> terminate(threadPool)); } diff --git a/x-pack/plugin/ccr/src/test/java/org/elasticsearch/xpack/ccr/repository/CcrRestoreSourceServiceTests.java b/x-pack/plugin/ccr/src/test/java/org/elasticsearch/xpack/ccr/repository/CcrRestoreSourceServiceTests.java index f577ccd4e5a44..796dae5373252 100644 --- a/x-pack/plugin/ccr/src/test/java/org/elasticsearch/xpack/ccr/repository/CcrRestoreSourceServiceTests.java +++ b/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,13 @@ 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." + ); + } + public void testOpenSession() throws IOException { IndexShard indexShard1 = newStartedShard(true); IndexShard indexShard2 = newStartedShard(true);