SOLR-14274 Do not register multiple sets of JVM metrics (#1299)

This commit is contained in:
Mike 2020-03-25 11:48:17 -07:00 committed by GitHub
parent 075adac598
commit 8d937c1dc3
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 44 additions and 21 deletions

View File

@ -199,12 +199,12 @@ public class SimCloudManager implements SolrCloudManager {
// register common metrics
metricTag = Integer.toHexString(hashCode());
String registryName = SolrMetricManager.getRegistryName(SolrInfoBean.Group.jvm);
metricManager.registerAll(registryName, new AltBufferPoolMetricSet(), true, "buffers");
metricManager.registerAll(registryName, new ClassLoadingGaugeSet(), true, "classes");
metricManager.registerAll(registryName, new OperatingSystemMetricSet(), true, "os");
metricManager.registerAll(registryName, new GarbageCollectorMetricSet(), true, "gc");
metricManager.registerAll(registryName, new MemoryUsageGaugeSet(), true, "memory");
metricManager.registerAll(registryName, new ThreadStatesGaugeSet(), true, "threads"); // todo should we use CachedThreadStatesGaugeSet instead?
metricManager.registerAll(registryName, new AltBufferPoolMetricSet(), SolrMetricManager.ResolutionStrategy.REPLACE, "buffers");
metricManager.registerAll(registryName, new ClassLoadingGaugeSet(), SolrMetricManager.ResolutionStrategy.REPLACE, "classes");
metricManager.registerAll(registryName, new OperatingSystemMetricSet(), SolrMetricManager.ResolutionStrategy.REPLACE, "os");
metricManager.registerAll(registryName, new GarbageCollectorMetricSet(), SolrMetricManager.ResolutionStrategy.REPLACE, "gc");
metricManager.registerAll(registryName, new MemoryUsageGaugeSet(), SolrMetricManager.ResolutionStrategy.REPLACE, "memory");
metricManager.registerAll(registryName, new ThreadStatesGaugeSet(), SolrMetricManager.ResolutionStrategy.REPLACE, "threads"); // todo should we use CachedThreadStatesGaugeSet instead?
MetricsMap sysprops = new MetricsMap((detailed, map) -> {
System.getProperties().forEach((k, v) -> {
map.put(String.valueOf(k), v);

View File

@ -522,26 +522,46 @@ public class SolrMetricManager {
}
}
/**
* Potential conflict resolution strategies when attempting to register a new metric that already exists
*/
public enum ResolutionStrategy {
/**
* The existing metric will be kept and the new metric will be ignored
*/
IGNORE,
/**
* The existing metric will be removed and replaced with the new metric
*/
REPLACE,
/**
* An exception will be thrown. This is the default implementation behavior.
*/
ERROR
}
/**
* Register all metrics in the provided {@link MetricSet}, optionally skipping those that
* already exist.
*
* @param registry registry name
* @param metrics metric set to register
* @param force if true then already existing metrics with the same name will be replaced.
* When false and a metric with the same name already exists an exception
* will be thrown.
* @param strategy the conflict resolution strategy to use if the named metric already exists.
* @param metricPath (optional) additional top-most metric name path elements
* @throws Exception if a metric with this name already exists.
*/
public void registerAll(String registry, MetricSet metrics, boolean force, String... metricPath) throws Exception {
public void registerAll(String registry, MetricSet metrics, ResolutionStrategy strategy, String... metricPath) throws Exception {
MetricRegistry metricRegistry = registry(registry);
synchronized (metricRegistry) {
Map<String, Metric> existingMetrics = metricRegistry.getMetrics();
for (Map.Entry<String, Metric> entry : metrics.getMetrics().entrySet()) {
String fullName = mkName(entry.getKey(), metricPath);
if (force && existingMetrics.containsKey(fullName)) {
metricRegistry.remove(fullName);
if (existingMetrics.containsKey(fullName)) {
if (strategy == ResolutionStrategy.REPLACE) {
metricRegistry.remove(fullName);
} else if (strategy == ResolutionStrategy.IGNORE) {
continue;
} // strategy == ERROR will fail when we try to register later
}
metricRegistry.register(fullName, entry.getValue());
}

View File

@ -211,12 +211,12 @@ public class SolrDispatchFilter extends BaseSolrFilter {
registryName = SolrMetricManager.getRegistryName(SolrInfoBean.Group.jvm);
final Set<String> hiddenSysProps = coresInit.getConfig().getMetricsConfig().getHiddenSysProps();
try {
metricManager.registerAll(registryName, new AltBufferPoolMetricSet(), true, "buffers");
metricManager.registerAll(registryName, new ClassLoadingGaugeSet(), true, "classes");
metricManager.registerAll(registryName, new OperatingSystemMetricSet(), true, "os");
metricManager.registerAll(registryName, new GarbageCollectorMetricSet(), true, "gc");
metricManager.registerAll(registryName, new MemoryUsageGaugeSet(), true, "memory");
metricManager.registerAll(registryName, new ThreadStatesGaugeSet(), true, "threads"); // todo should we use CachedThreadStatesGaugeSet instead?
metricManager.registerAll(registryName, new AltBufferPoolMetricSet(), SolrMetricManager.ResolutionStrategy.IGNORE, "buffers");
metricManager.registerAll(registryName, new ClassLoadingGaugeSet(), SolrMetricManager.ResolutionStrategy.IGNORE, "classes");
metricManager.registerAll(registryName, new OperatingSystemMetricSet(), SolrMetricManager.ResolutionStrategy.IGNORE, "os");
metricManager.registerAll(registryName, new GarbageCollectorMetricSet(), SolrMetricManager.ResolutionStrategy.IGNORE, "gc");
metricManager.registerAll(registryName, new MemoryUsageGaugeSet(), SolrMetricManager.ResolutionStrategy.IGNORE, "memory");
metricManager.registerAll(registryName, new ThreadStatesGaugeSet(), SolrMetricManager.ResolutionStrategy.IGNORE, "threads"); // todo should we use CachedThreadStatesGaugeSet instead?
MetricsMap sysprops = new MetricsMap((detailed, map) -> {
System.getProperties().forEach((k, v) -> {
if (!hiddenSysProps.contains(k)) {

View File

@ -89,11 +89,14 @@ public class SolrMetricManagerTest extends SolrTestCaseJ4 {
String registryName = TestUtil.randomSimpleString(r, 1, 10);
assertEquals(0, metricManager.registry(registryName).getMetrics().size());
metricManager.registerAll(registryName, mr, false);
// There is nothing registered so we should be error-free on the first pass
metricManager.registerAll(registryName, mr, SolrMetricManager.ResolutionStrategy.ERROR);
// this should simply skip existing names
metricManager.registerAll(registryName, mr, true);
metricManager.registerAll(registryName, mr, SolrMetricManager.ResolutionStrategy.IGNORE);
// this should re-register everything, and no errors
metricManager.registerAll(registryName, mr, SolrMetricManager.ResolutionStrategy.REPLACE);
// this should produce error
expectThrows(IllegalArgumentException.class, () -> metricManager.registerAll(registryName, mr, false));
expectThrows(IllegalArgumentException.class, () -> metricManager.registerAll(registryName, mr, SolrMetricManager.ResolutionStrategy.ERROR));
}
@Test