Handle -1 gc_threshold settings explicitly (#54546)

Because -1 is technically a valid TimeValue (as a sentinel value), that is now
explicitly checked for when validating gc_thresholds. The tests are also
adjusted to test this case separately from other negative values.
This commit is contained in:
Gordon Brown 2020-04-01 13:56:50 -06:00 committed by GitHub
parent bd6b383926
commit f0cb8a56a9
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 16 additions and 1 deletions

View File

@ -177,6 +177,10 @@ public class JvmGcMonitorService extends AbstractLifecycleComponent {
if (threshold == null) {
throw new IllegalArgumentException("missing gc_threshold for [" + getThresholdName(key, level) + "]");
} else if (threshold.nanos() < 0) {
final String settingValue = settings.get(level);
throw new IllegalArgumentException("invalid gc_threshold [" + getThresholdName(key, level) + "] value [" +
settingValue + "]: value cannot be negative");
}
return threshold;

View File

@ -62,7 +62,7 @@ public class JvmGcMonitorServiceSettingsTests extends ESTestCase {
public void testNegativeSetting() throws InterruptedException {
String collector = randomAlphaOfLength(5);
final String timeValue = "-" + randomTimeValue();
final String timeValue = "-" + randomTimeValue(2,1000); // -1 is handled separately
Settings settings = Settings.builder().put("monitor.jvm.gc.collector." + collector + ".warn", timeValue).build();
execute(settings, (command, interval, name) -> null, e -> {
assertThat(e, instanceOf(IllegalArgumentException.class));
@ -71,6 +71,17 @@ public class JvmGcMonitorServiceSettingsTests extends ESTestCase {
}, true, null);
}
public void testNegativeOneSetting() throws InterruptedException {
String collector = randomAlphaOfLength(5);
final String timeValue = "-1" + randomFrom("", "d", "h", "m", "s", "ms", "nanos");
Settings settings = Settings.builder().put("monitor.jvm.gc.collector." + collector + ".warn", timeValue).build();
execute(settings, (command, interval, name) -> null, e -> {
assertThat(e, instanceOf(IllegalArgumentException.class));
assertThat(e.getMessage(), equalTo("invalid gc_threshold [monitor.jvm.gc.collector." + collector + ".warn] " +
"value [" + timeValue + "]: value cannot be negative"));
}, true, null);
}
public void testMissingSetting() throws InterruptedException {
String collector = randomAlphaOfLength(5);
Set<AbstractMap.SimpleEntry<String, String>> entries = new HashSet<>();