No leniency in parsing JVM GC monitor settings

This commit removes some leniency in the parsing of the JVM GC monitor
settings, and adds tests for the JVM GC monitor settings.
This commit is contained in:
Jason Tedor 2016-01-29 09:39:02 -05:00
parent ea56bfad9a
commit a1fd01d4cc
2 changed files with 135 additions and 9 deletions

View File

@ -50,7 +50,9 @@ public class JvmGcMonitorService extends AbstractLifecycleComponent<JvmGcMonitor
public final static Setting<Boolean> ENABLED_SETTING = Setting.boolSetting("monitor.jvm.gc.enabled", true, false, Scope.CLUSTER);
public final static Setting<TimeValue> REFRESH_INTERVAL_SETTING =
Setting.timeSetting("monitor.jvm.gc.refresh_interval", TimeValue.timeValueSeconds(1), TimeValue.timeValueSeconds(1), false, Scope.CLUSTER);
public final static Setting<Settings> GC_SETTING = Setting.groupSetting("monitor.jvm.gc.collector.", false, Scope.CLUSTER);
private static String GC_COLLECTOR_PREFIX = "monitor.jvm.gc.collector.";
public final static Setting<Settings> GC_SETTING = Setting.groupSetting(GC_COLLECTOR_PREFIX, false, Scope.CLUSTER);
static class GcThreshold {
public final String name;
@ -87,14 +89,10 @@ public class JvmGcMonitorService extends AbstractLifecycleComponent<JvmGcMonitor
Map<String, Settings> gcThresholdGroups = GC_SETTING.get(settings).getAsGroups();
for (Map.Entry<String, Settings> entry : gcThresholdGroups.entrySet()) {
String name = entry.getKey();
TimeValue warn = entry.getValue().getAsTime("warn", null);
TimeValue info = entry.getValue().getAsTime("info", null);
TimeValue debug = entry.getValue().getAsTime("debug", null);
if (warn == null || info == null || debug == null) {
logger.warn("ignoring gc_threshold for [{}], missing warn/info/debug values", name);
} else {
gcThresholds.put(name, new GcThreshold(name, warn.millis(), info.millis(), debug.millis()));
}
TimeValue warn = getValidThreshold(entry.getValue(), entry.getKey(), "warn");
TimeValue info = getValidThreshold(entry.getValue(), entry.getKey(), "info");
TimeValue debug = getValidThreshold(entry.getValue(), entry.getKey(), "debug");
gcThresholds.put(name, new GcThreshold(name, warn.millis(), info.millis(), debug.millis()));
}
gcThresholds.putIfAbsent(GcNames.YOUNG, new GcThreshold(GcNames.YOUNG, 1000, 700, 400));
gcThresholds.putIfAbsent(GcNames.OLD, new GcThreshold(GcNames.OLD, 10000, 5000, 2000));
@ -104,6 +102,21 @@ public class JvmGcMonitorService extends AbstractLifecycleComponent<JvmGcMonitor
logger.debug("enabled [{}], interval [{}], gc_threshold [{}]", enabled, interval, this.gcThresholds);
}
private static TimeValue getValidThreshold(Settings settings, String key, String level) {
TimeValue threshold = settings.getAsTime(level, null);
if (threshold == null) {
throw new IllegalArgumentException("missing gc_threshold for [" + getThresholdName(key, level) + "]");
}
if (threshold.nanos() <= 0) {
throw new IllegalArgumentException("invalid gc_threshold [" + threshold + "] for [" + getThresholdName(key, level) + "]");
}
return threshold;
}
private static String getThresholdName(String key, String level) {
return GC_COLLECTOR_PREFIX + key + "." + level;
}
@Override
protected void doStart() {
if (!enabled) {

View File

@ -0,0 +1,113 @@
/*
* Licensed to Elasticsearch under one or more contributor
* license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright
* ownership. Elasticsearch licenses this file to you under
* the Apache License, Version 2.0 (the "License"); you may
* not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/
package org.elasticsearch.monitor.jvm;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.unit.TimeValue;
import org.elasticsearch.test.ESTestCase;
import org.elasticsearch.threadpool.ThreadPool;
import java.util.AbstractMap;
import java.util.HashSet;
import java.util.Set;
import java.util.concurrent.ScheduledFuture;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.function.BiFunction;
import java.util.function.Consumer;
import static org.hamcrest.CoreMatchers.allOf;
import static org.hamcrest.CoreMatchers.containsString;
import static org.hamcrest.CoreMatchers.instanceOf;
public class JvmGcMonitorServiceSettingsTests extends ESTestCase {
public void testEmptySettingsAreOkay() throws InterruptedException {
AtomicBoolean scheduled = new AtomicBoolean();
execute(Settings.EMPTY, (command, interval) -> { scheduled.set(true); return null; }, () -> assertTrue(scheduled.get()));
}
public void testDisabledSetting() throws InterruptedException {
Settings settings = Settings.builder().put("monitor.jvm.gc.enabled", "false").build();
AtomicBoolean scheduled = new AtomicBoolean();
execute(settings, (command, interval) -> { scheduled.set(true); return null; }, () -> assertFalse(scheduled.get()));
}
public void testNegativeSetting() throws InterruptedException {
String collector = randomAsciiOfLength(5);
Settings settings = Settings.builder().put("monitor.jvm.gc.collector." + collector + ".warn", "-" + randomTimeValue()).build();
execute(settings, (command, interval) -> null, t -> {
assertThat(t, instanceOf(IllegalArgumentException.class));
assertThat(t.getMessage(), allOf(containsString("invalid gc_threshold"), containsString("for [monitor.jvm.gc.collector." + collector + ".")));
}, true, null);
}
public void testMissingSetting() throws InterruptedException {
String collector = randomAsciiOfLength(5);
Set<AbstractMap.SimpleEntry<String, String>> entries = new HashSet<>();
entries.add(new AbstractMap.SimpleEntry<>("monitor.jvm.gc.collector." + collector + ".warn", randomTimeValue()));
entries.add(new AbstractMap.SimpleEntry<>("monitor.jvm.gc.collector." + collector + ".info", randomTimeValue()));
entries.add(new AbstractMap.SimpleEntry<>("monitor.jvm.gc.collector." + collector + ".debug", randomTimeValue()));
Settings.Builder builder = Settings.builder();
// drop a random setting or two
for (@SuppressWarnings("unchecked") AbstractMap.SimpleEntry<String, String> entry : randomSubsetOf(randomIntBetween(1, 2), entries.toArray(new AbstractMap.SimpleEntry[0]))) {
builder.put(entry.getKey(), entry.getValue());
}
// we should get an exception that a setting is missing
execute(builder.build(), (command, interval) -> null, t -> {
assertThat(t, instanceOf(IllegalArgumentException.class));
assertThat(t.getMessage(), containsString("missing gc_threshold for [monitor.jvm.gc.collector." + collector + "."));
}, true, null);
}
private static void execute(Settings settings, BiFunction<Runnable, TimeValue, ScheduledFuture<?>> scheduler, Runnable asserts) throws InterruptedException {
execute(settings, scheduler, null, false, asserts);
}
private static void execute(Settings settings, BiFunction<Runnable, TimeValue, ScheduledFuture<?>> scheduler, Consumer<Throwable> consumer, boolean constructionShouldFail, Runnable asserts) throws InterruptedException {
assert constructionShouldFail == (consumer != null);
assert constructionShouldFail == (asserts == null);
ThreadPool threadPool = null;
try {
threadPool = new ThreadPool(JvmGcMonitorServiceSettingsTests.class.getCanonicalName()) {
@Override
public ScheduledFuture<?> scheduleWithFixedDelay(Runnable command, TimeValue interval) {
return scheduler.apply(command, interval);
}
};
try {
JvmGcMonitorService service = new JvmGcMonitorService(settings, threadPool);
if (constructionShouldFail) {
fail("construction of jvm gc service should have failed");
}
service.doStart();
asserts.run();
service.doStop();
} catch (Throwable t) {
consumer.accept(t);
}
} finally {
ThreadPool.terminate(threadPool, 30, TimeUnit.SECONDS);
}
}
}