From 931503cc04e83990a3f3e98c75b7104cca12b780 Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Fri, 26 Jul 2019 17:10:36 +0900 Subject: [PATCH 1/6] Limit processors by available processors This commit limits the processors setting to be more than the number of available processors. --- .../migration/migrate_8_0/settings.asciidoc | 9 +++++++++ .../common/util/concurrent/EsExecutors.java | 20 ++++--------------- .../util/concurrent/EsExecutorsTests.java | 14 +++++++++++++ 3 files changed, 27 insertions(+), 16 deletions(-) diff --git a/docs/reference/migration/migrate_8_0/settings.asciidoc b/docs/reference/migration/migrate_8_0/settings.asciidoc index 0c21ae4021aa7..660101c16fcd7 100644 --- a/docs/reference/migration/migrate_8_0/settings.asciidoc +++ b/docs/reference/migration/migrate_8_0/settings.asciidoc @@ -11,3 +11,12 @@ provided automatic upgrading of these settings to their `cluster.remote` counterparts. In 8.0.0, these settings have been removed. Elasticsearch will refuse to start if you have these settings in your configuration or cluster state. + +[float] +==== `processors` can no longer exceed the available number of processors + +Previously it was possible to set the number of processors used to set the +default sizes for the thread pools to be more than the number of available +processors. As this leads to more context switches and more threads but without +an increase in the number of physical CPUs on which to schedule these additional +threads, the `processors` setting is now bounded by the number of available processors. 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 561a820d49078..66d65337237f7 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 @@ -48,26 +48,14 @@ public class EsExecutors { - private static final DeprecationLogger deprecationLogger = new DeprecationLogger(LogManager.getLogger(EsExecutors.class)); - /** * 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 = new Setting<>( + public static final Setting PROCESSORS_SETTING = Setting.intSetting( "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; - }, + Runtime.getRuntime().availableProcessors(), + 1, + Runtime.getRuntime().availableProcessors(), Property.NodeScope); /** diff --git a/server/src/test/java/org/elasticsearch/common/util/concurrent/EsExecutorsTests.java b/server/src/test/java/org/elasticsearch/common/util/concurrent/EsExecutorsTests.java index 0f0350c48210c..e15404bb28464 100644 --- a/server/src/test/java/org/elasticsearch/common/util/concurrent/EsExecutorsTests.java +++ b/server/src/test/java/org/elasticsearch/common/util/concurrent/EsExecutorsTests.java @@ -32,7 +32,10 @@ import static org.hamcrest.Matchers.anyOf; import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.hasToString; import static org.hamcrest.Matchers.lessThan; +import static org.hamcrest.Matchers.matchesPattern; +import static org.hamcrest.Matchers.matchesRegex; /** * Tests for EsExecutors and its components like EsAbortPolicy. @@ -388,4 +391,15 @@ public void testGetTasks() throws InterruptedException { } } + public void testProcessorsBound() { + final int available = Runtime.getRuntime().availableProcessors(); + final int processors = randomIntBetween(available + 1, Integer.MAX_VALUE); + final Settings settings = Settings.builder().put("processors", processors).build(); + final IllegalArgumentException e = + expectThrows(IllegalArgumentException.class, () -> EsExecutors.PROCESSORS_SETTING.get(settings)); + assertThat( + e, + hasToString(containsString("Failed to parse value [" + processors + "] for setting [processors] must be <= " + available))); + } + } From 230fa553e2735a64b68f4820ddf7c7568ecaa24e Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Fri, 26 Jul 2019 17:21:08 +0900 Subject: [PATCH 2/6] Fix imports --- .../elasticsearch/common/util/concurrent/EsExecutorsTests.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/server/src/test/java/org/elasticsearch/common/util/concurrent/EsExecutorsTests.java b/server/src/test/java/org/elasticsearch/common/util/concurrent/EsExecutorsTests.java index e15404bb28464..6a64a93227747 100644 --- a/server/src/test/java/org/elasticsearch/common/util/concurrent/EsExecutorsTests.java +++ b/server/src/test/java/org/elasticsearch/common/util/concurrent/EsExecutorsTests.java @@ -34,8 +34,6 @@ import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.hasToString; import static org.hamcrest.Matchers.lessThan; -import static org.hamcrest.Matchers.matchesPattern; -import static org.hamcrest.Matchers.matchesRegex; /** * Tests for EsExecutors and its components like EsAbortPolicy. From df62e774e2987f7fafeb843cbd0a0af2ed47b09c Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Fri, 26 Jul 2019 17:21:18 +0900 Subject: [PATCH 3/6] More import fixes --- .../org/elasticsearch/common/util/concurrent/EsExecutors.java | 2 -- 1 file changed, 2 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 66d65337237f7..1623ffdf82565 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,10 +19,8 @@ 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; From 5e86b029d9f1201a1ea7f228b4fcc784fd827edb Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Fri, 26 Jul 2019 22:13:11 +0900 Subject: [PATCH 4/6] Fix wrapping --- docs/reference/migration/migrate_8_0/settings.asciidoc | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/docs/reference/migration/migrate_8_0/settings.asciidoc b/docs/reference/migration/migrate_8_0/settings.asciidoc index 660101c16fcd7..4eac119538dd7 100644 --- a/docs/reference/migration/migrate_8_0/settings.asciidoc +++ b/docs/reference/migration/migrate_8_0/settings.asciidoc @@ -19,4 +19,5 @@ Previously it was possible to set the number of processors used to set the default sizes for the thread pools to be more than the number of available processors. As this leads to more context switches and more threads but without an increase in the number of physical CPUs on which to schedule these additional -threads, the `processors` setting is now bounded by the number of available processors. +threads, the `processors` setting is now bounded by the number of available +processors. From 9f8a69113bed5d3815b88e2b75d7ab49e57efefa Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Fri, 26 Jul 2019 22:17:27 +0900 Subject: [PATCH 5/6] Fix test --- .../threadpool/ScalingThreadPoolTests.java | 18 ++---------------- 1 file changed, 2 insertions(+), 16 deletions(-) diff --git a/server/src/test/java/org/elasticsearch/threadpool/ScalingThreadPoolTests.java b/server/src/test/java/org/elasticsearch/threadpool/ScalingThreadPoolTests.java index d4e6f3693b712..56d9ce6ed5940 100644 --- a/server/src/test/java/org/elasticsearch/threadpool/ScalingThreadPoolTests.java +++ b/server/src/test/java/org/elasticsearch/threadpool/ScalingThreadPoolTests.java @@ -48,18 +48,8 @@ public void testScalingThreadPoolConfiguration() throws InterruptedException { core = "generic".equals(threadPoolName) ? 4 : 1; // the defaults } - final int availableProcessors = Runtime.getRuntime().availableProcessors(); - final int maxBasedOnNumberOfProcessors; - final int processorsUsed; - if (randomBoolean()) { - final int processors = randomIntBetween(1, 64); - maxBasedOnNumberOfProcessors = expectedSize(threadPoolName, processors); - builder.put("processors", processors); - processorsUsed = processors; - } else { - maxBasedOnNumberOfProcessors = expectedSize(threadPoolName, availableProcessors); - processorsUsed = availableProcessors; - } + final int processors = randomIntBetween(1, 1 + Runtime.getRuntime().availableProcessors()); + final int maxBasedOnNumberOfProcessors = expectedSize(threadPoolName, processors); final int expectedMax; if (maxBasedOnNumberOfProcessors < core || randomBoolean()) { @@ -98,10 +88,6 @@ public void testScalingThreadPoolConfiguration() throws InterruptedException { assertThat(esThreadPoolExecutor.getMaximumPoolSize(), equalTo(expectedMax)); }); - if (processorsUsed > availableProcessors) { - assertWarnings("setting processors to value [" + processorsUsed + - "] which is more than available processors [" + availableProcessors + "] is deprecated"); - } } private int expectedSize(final String threadPoolName, final int numberOfProcessors) { From 9d27086a49750b7010f73ac3952d1329df99f626 Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Fri, 26 Jul 2019 22:21:57 +0900 Subject: [PATCH 6/6] Fix off by one error --- .../org/elasticsearch/threadpool/ScalingThreadPoolTests.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/test/java/org/elasticsearch/threadpool/ScalingThreadPoolTests.java b/server/src/test/java/org/elasticsearch/threadpool/ScalingThreadPoolTests.java index 56d9ce6ed5940..097d856f06289 100644 --- a/server/src/test/java/org/elasticsearch/threadpool/ScalingThreadPoolTests.java +++ b/server/src/test/java/org/elasticsearch/threadpool/ScalingThreadPoolTests.java @@ -48,7 +48,7 @@ public void testScalingThreadPoolConfiguration() throws InterruptedException { core = "generic".equals(threadPoolName) ? 4 : 1; // the defaults } - final int processors = randomIntBetween(1, 1 + Runtime.getRuntime().availableProcessors()); + final int processors = randomIntBetween(1, Runtime.getRuntime().availableProcessors()); final int maxBasedOnNumberOfProcessors = expectedSize(threadPoolName, processors); final int expectedMax;