honeymoose
/
OpenSearch
mirror of https://github.com/honeymoose/OpenSearch.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Fix AutoQueueAdjustingExecutorBuilder settings validation (#33922) 

Settings validation in AutoQueueAdjustingExecutorBuilder always checked against
a default value which means that we never can change a max queue size that is lower
than the default. This change adds tests and fixes this validation.
This commit is contained in:
Simon Willnauer 2018-09-24 07:45:50 +02:00 committed by GitHub
parent ddd5ce5740
commit 7d703c2f92
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 109 additions and 15 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

54
server/src/main/java/org/elasticsearch/threadpool/AutoQueueAdjustingExecutorBuilder.java
View File

@ -28,8 +28,10 @@ import org.elasticsearch.common.util.concurrent.ThreadContext;
import org.elasticsearch.node.Node;
import java.util.Arrays;
import java.util.Iterator;
import java.util.List;
import java.util.Locale;
import java.util.Map;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.ThreadFactory;
@ -71,14 +73,42 @@ public final class AutoQueueAdjustingExecutorBuilder extends ExecutorBuilder<Aut
Setting<Integer> tempMinQueueSizeSetting = Setting.intSetting(minSizeKey, minQueueSize, Setting.Property.NodeScope);
this.minQueueSizeSetting = new Setting<>(
minSizeKey,
(s) -> Integer.toString(minQueueSize),
(s) -> Setting.parseInt(s, 0, tempMaxQueueSizeSetting.get(settings), minSizeKey),
Setting.Property.NodeScope);
minSizeKey,
Integer.toString(minQueueSize),
(s) -> Setting.parseInt(s, 0, minSizeKey),
new Setting.Validator<Integer>() {
@Override
public void validate(Integer value, Map<Setting<Integer>, Integer> settings) {
if (value > settings.get(tempMaxQueueSizeSetting)) {
throw new IllegalArgumentException("Failed to parse value [" + value + "] for setting [" + minSizeKey
+ "] must be <= " + settings.get(tempMaxQueueSizeSetting));
}
}
@Override
public Iterator<Setting<Integer>> settings() {
return Arrays.asList(tempMaxQueueSizeSetting).iterator();
}
},
Setting.Property.NodeScope);
this.maxQueueSizeSetting = new Setting<>(
maxSizeKey,
(s) -> Integer.toString(maxQueueSize),
(s) -> Setting.parseInt(s, tempMinQueueSizeSetting.get(settings), Integer.MAX_VALUE, maxSizeKey),
Integer.toString(maxQueueSize),
(s) -> Setting.parseInt(s, 0, maxSizeKey),
new Setting.Validator<Integer>() {
@Override
public void validate(Integer value, Map<Setting<Integer>, Integer> settings) {
if (value < settings.get(tempMinQueueSizeSetting)) {
throw new IllegalArgumentException("Failed to parse value [" + value + "] for setting [" + minSizeKey
+ "] must be >= " + settings.get(tempMinQueueSizeSetting));
}
}
@Override
public Iterator<Setting<Integer>> settings() {
return Arrays.asList(tempMinQueueSizeSetting).iterator();
}
},
Setting.Property.NodeScope);
this.frameSizeSetting = Setting.intSetting(frameSizeKey, frameSize, 100, Setting.Property.NodeScope);
}
@ -141,12 +171,12 @@ public final class AutoQueueAdjustingExecutorBuilder extends ExecutorBuilder<Aut
static final class AutoExecutorSettings extends ExecutorBuilder.ExecutorSettings {
private final int size;
private final int initialQueueSize;
private final int minQueueSize;
private final int maxQueueSize;
private final int frameSize;
private final TimeValue targetedResponseTime;
final int size;
final int initialQueueSize;
final int minQueueSize;
final int maxQueueSize;
final int frameSize;
final TimeValue targetedResponseTime;
AutoExecutorSettings(final String nodeName, final int size, final int initialQueueSize,
final int minQueueSize, final int maxQueueSize, final int frameSize,

70
server/src/test/java/org/elasticsearch/threadpool/AutoQueueAdjustingExecutorBuilderTests.java
View File

@ -25,10 +25,10 @@ import static org.hamcrest.CoreMatchers.containsString;
public class AutoQueueAdjustingExecutorBuilderTests extends ESThreadPoolTestCase {
public void testValidatingMinMaxSettings() throws Exception {
public void testValidatingMinMaxSettings() {
Settings settings = Settings.builder()
.put("thread_pool.search.min_queue_size", randomIntBetween(30, 100))
.put("thread_pool.search.max_queue_size", randomIntBetween(1,25))
.put("thread_pool.test.min_queue_size", randomIntBetween(30, 100))
.put("thread_pool.test.max_queue_size", randomIntBetween(1,25))
.build();
try {
new AutoQueueAdjustingExecutorBuilder(settings, "test", 1, 15, 1, 100, 10);
@ -36,6 +36,70 @@ public class AutoQueueAdjustingExecutorBuilderTests extends ESThreadPoolTestCase
} catch (IllegalArgumentException e) {
assertThat(e.getMessage(), containsString("Failed to parse value"));
}
settings = Settings.builder()
.put("thread_pool.test.min_queue_size", 10)
.put("thread_pool.test.max_queue_size", 9)
.build();
try {
new AutoQueueAdjustingExecutorBuilder(settings, "test", 1, 15, 1, 100, 2000).getSettings(settings);
fail("should have thrown an exception");
} catch (IllegalArgumentException e) {
assertEquals(e.getMessage(), "Failed to parse value [10] for setting [thread_pool.test.min_queue_size] must be <= 9");
}
settings = Settings.builder()
.put("thread_pool.test.min_queue_size", 11)
.put("thread_pool.test.max_queue_size", 10)
.build();
try {
new AutoQueueAdjustingExecutorBuilder(settings, "test", 1, 15, 1, 100, 2000).getSettings(settings);
fail("should have thrown an exception");
} catch (IllegalArgumentException e) {
assertEquals(e.getMessage(), "Failed to parse value [11] for setting [thread_pool.test.min_queue_size] must be <= 10");
}
settings = Settings.builder()
.put("thread_pool.test.min_queue_size", 101)
.build();
try {
new AutoQueueAdjustingExecutorBuilder(settings, "test", 1, 15, 100, 100, 2000).getSettings(settings);
fail("should have thrown an exception");
} catch (IllegalArgumentException e) {
assertEquals(e.getMessage(), "Failed to parse value [101] for setting [thread_pool.test.min_queue_size] must be <= 100");
}
settings = Settings.builder()
.put("thread_pool.test.max_queue_size", 99)
.build();
try {
new AutoQueueAdjustingExecutorBuilder(settings, "test", 1, 15, 100, 100, 2000).getSettings(settings);
fail("should have thrown an exception");
} catch (IllegalArgumentException e) {
assertEquals(e.getMessage(), "Failed to parse value [100] for setting [thread_pool.test.min_queue_size] must be <= 99");
}
}
public void testSetLowerSettings() {
Settings settings = Settings.builder()
.put("thread_pool.test.min_queue_size", 10)
.put("thread_pool.test.max_queue_size", 10)
.build();
AutoQueueAdjustingExecutorBuilder test = new AutoQueueAdjustingExecutorBuilder(settings, "test", 1, 1000, 1000, 1000, 2000);
AutoQueueAdjustingExecutorBuilder.AutoExecutorSettings s = test.getSettings(settings);
assertEquals(10, s.maxQueueSize);
assertEquals(10, s.minQueueSize);
}
public void testSetHigherSettings() {
Settings settings = Settings.builder()
.put("thread_pool.test.min_queue_size", 2000)
.put("thread_pool.test.max_queue_size", 3000)
.build();
AutoQueueAdjustingExecutorBuilder test = new AutoQueueAdjustingExecutorBuilder(settings, "test", 1, 1000, 1000, 1000, 2000);
AutoQueueAdjustingExecutorBuilder.AutoExecutorSettings s = test.getSettings(settings);
assertEquals(3000, s.maxQueueSize);
assertEquals(2000, s.minQueueSize);
}
}