From 6ea2b5dec073ebcd873cf8cfb8b535367310bea5 Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Fri, 26 Jul 2019 17:06:10 +0900 Subject: [PATCH] Deprecate setting processors to more than available (#44889) Today the processors setting is permitted to be set to more than the number of processors available to the JVM. The processors setting directly sizes the number of threads in the various thread pools, with most of these sizes being a linear function in the number of processors. It doesn't make any sense to set processors very high as the overhead from context switching amongst all the threads will overwhelm, and changing the setting does not control how many physical CPU resources there are on which to schedule the additional threads. We have to draw a line somewhere and this commit deprecates setting processors to more than the number of available processors. This is the right place to draw the line given the linear growth as a function of processors in most of the thread pools, and that some are capped at the number of available processors already. --- .../common/util/concurrent/EsExecutors.java | 25 ++++++++++++++--- .../test/InternalTestCluster.java | 2 +- .../elasticsearch/xpack/watcher/Watcher.java | 10 ++++--- .../xpack/watcher/WatcherPluginTests.java | 28 ++++++++----------- 4 files changed, 40 insertions(+), 25 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/common/util/concurrent/EsExecutors.java b/server/src/main/java/org/elasticsearch/common/util/concurrent/EsExecutors.java index 7b304cd092a..561a820d490 100644 --- a/server/src/main/java/org/elasticsearch/common/util/concurrent/EsExecutors.java +++ b/server/src/main/java/org/elasticsearch/common/util/concurrent/EsExecutors.java @@ -19,8 +19,10 @@ package org.elasticsearch.common.util.concurrent; +import org.apache.logging.log4j.LogManager; import org.elasticsearch.ExceptionsHelper; import org.elasticsearch.common.SuppressForbidden; +import org.elasticsearch.common.logging.DeprecationLogger; import org.elasticsearch.common.settings.Setting; import org.elasticsearch.common.settings.Setting.Property; import org.elasticsearch.common.settings.Settings; @@ -46,12 +48,27 @@ import java.util.stream.Collectors; public class EsExecutors { + private static final DeprecationLogger deprecationLogger = new DeprecationLogger(LogManager.getLogger(EsExecutors.class)); + /** - * Settings key to manually set the number of available processors. - * This is used to adjust thread pools sizes etc. per node. + * Setting to manually set the number of available processors. This setting is used to adjust thread pool sizes per node. */ - public static final Setting PROCESSORS_SETTING = - Setting.intSetting("processors", Runtime.getRuntime().availableProcessors(), 1, Property.NodeScope); + public static final Setting PROCESSORS_SETTING = new Setting<>( + "processors", + s -> Integer.toString(Runtime.getRuntime().availableProcessors()), + s -> { + final int value = Setting.parseInt(s, 1, "processors"); + final int availableProcessors = Runtime.getRuntime().availableProcessors(); + if (value > availableProcessors) { + deprecationLogger.deprecatedAndMaybeLog( + "processors", + "setting processors to value [{}] which is more than available processors [{}] is deprecated", + value, + availableProcessors); + } + return value; + }, + Property.NodeScope); /** * Returns the number of available processors. Defaults to diff --git a/test/framework/src/main/java/org/elasticsearch/test/InternalTestCluster.java b/test/framework/src/main/java/org/elasticsearch/test/InternalTestCluster.java index 26908aeb2fc..90e82107bca 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/InternalTestCluster.java +++ b/test/framework/src/main/java/org/elasticsearch/test/InternalTestCluster.java @@ -470,7 +470,7 @@ public final class InternalTestCluster extends TestCluster { builder.put(SearchService.DEFAULT_KEEPALIVE_SETTING.getKey(), timeValueSeconds(100 + random.nextInt(5 * 60)).getStringRep()); } - builder.put(EsExecutors.PROCESSORS_SETTING.getKey(), 1 + random.nextInt(3)); + builder.put(EsExecutors.PROCESSORS_SETTING.getKey(), 1 + random.nextInt(Math.min(4, Runtime.getRuntime().availableProcessors()))); if (random.nextBoolean()) { if (random.nextBoolean()) { builder.put("indices.fielddata.cache.size", 1 + random.nextInt(1000), ByteSizeUnit.MB); diff --git a/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/Watcher.java b/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/Watcher.java index 8ce5901b353..3712629d018 100644 --- a/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/Watcher.java +++ b/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/Watcher.java @@ -532,11 +532,13 @@ public class Watcher extends Plugin implements ActionPlugin, ScriptPlugin, Reloa * @param settings The current settings * @return A number between 5 and the number of processors */ - static int getWatcherThreadPoolSize(Settings settings) { - boolean isDataNode = Node.NODE_DATA_SETTING.get(settings); + static int getWatcherThreadPoolSize(final Settings settings) { + return getWatcherThreadPoolSize(Node.NODE_DATA_SETTING.get(settings), EsExecutors.numberOfProcessors(settings)); + } + + static int getWatcherThreadPoolSize(final boolean isDataNode, final int numberOfProcessors) { if (isDataNode) { - int numberOfProcessors = EsExecutors.numberOfProcessors(settings); - long size = Math.max(Math.min(5 * numberOfProcessors, 50), numberOfProcessors); + final long size = Math.max(Math.min(5 * numberOfProcessors, 50), numberOfProcessors); return Math.toIntExact(size); } else { return 1; diff --git a/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/WatcherPluginTests.java b/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/WatcherPluginTests.java index a4131889f84..49dc0a8b82e 100644 --- a/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/WatcherPluginTests.java +++ b/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/WatcherPluginTests.java @@ -87,23 +87,19 @@ public class WatcherPluginTests extends ESTestCase { public void testThreadPoolSize() { // old calculation was 5 * number of processors - assertThat(Watcher.getWatcherThreadPoolSize(Settings.builder().put("processors", 1).build()), is(5)); - assertThat(Watcher.getWatcherThreadPoolSize(Settings.builder().put("processors", 2).build()), is(10)); - assertThat(Watcher.getWatcherThreadPoolSize(Settings.builder().put("processors", 4).build()), is(20)); - assertThat(Watcher.getWatcherThreadPoolSize(Settings.builder().put("processors", 8).build()), is(40)); - assertThat(Watcher.getWatcherThreadPoolSize(Settings.builder().put("processors", 9).build()), is(45)); - assertThat(Watcher.getWatcherThreadPoolSize(Settings.builder().put("processors", 10).build()), is(50)); - assertThat(Watcher.getWatcherThreadPoolSize(Settings.builder().put("processors", 16).build()), is(50)); - assertThat(Watcher.getWatcherThreadPoolSize(Settings.builder().put("processors", 24).build()), is(50)); - assertThat(Watcher.getWatcherThreadPoolSize(Settings.builder().put("processors", 50).build()), is(50)); - assertThat(Watcher.getWatcherThreadPoolSize(Settings.builder().put("processors", 51).build()), is(51)); - assertThat(Watcher.getWatcherThreadPoolSize(Settings.builder().put("processors", 96).build()), is(96)); + assertThat(Watcher.getWatcherThreadPoolSize(true, 1), is(5)); + assertThat(Watcher.getWatcherThreadPoolSize(true, 2), is(10)); + assertThat(Watcher.getWatcherThreadPoolSize(true, 4), is(20)); + assertThat(Watcher.getWatcherThreadPoolSize(true, 8), is(40)); + assertThat(Watcher.getWatcherThreadPoolSize(true, 9), is(45)); + assertThat(Watcher.getWatcherThreadPoolSize(true, 10), is(50)); + assertThat(Watcher.getWatcherThreadPoolSize(true, 16), is(50)); + assertThat(Watcher.getWatcherThreadPoolSize(true, 24), is(50)); + assertThat(Watcher.getWatcherThreadPoolSize(true, 50), is(50)); + assertThat(Watcher.getWatcherThreadPoolSize(true, 51), is(51)); + assertThat(Watcher.getWatcherThreadPoolSize(true, 96), is(96)); - Settings noDataNodeSettings = Settings.builder() - .put("processors", scaledRandomIntBetween(1, 100)) - .put("node.data", false) - .build(); - assertThat(Watcher.getWatcherThreadPoolSize(noDataNodeSettings), is(1)); + assertThat(Watcher.getWatcherThreadPoolSize(false, scaledRandomIntBetween(1, 100)), is(1)); } public void testReload() {