From 7415fe453972fb23debd4f570599b5eeb0376ecb Mon Sep 17 00:00:00 2001 From: Noble Paul Date: Mon, 19 Aug 2019 19:48:06 +1000 Subject: [PATCH] SOLR-13677: All Metrics Gauges should be unregistered by the objects that registered them (#836) * SOLR-13677: All Metrics Gauges should be unregistered by the objects that registered them --- .../handler/dataimport/DataImportHandler.java | 8 +- .../solr/cloud/ReplicateFromLeader.java | 2 +- .../java/org/apache/solr/core/PluginBag.java | 3 +- .../java/org/apache/solr/core/SolrCore.java | 2 +- .../solr/handler/ReplicationHandler.java | 47 +++--- .../solr/handler/RequestHandlerBase.java | 48 +++--- .../solr/handler/admin/CoreAdminHandler.java | 9 +- .../handler/component/SuggestComponent.java | 23 +-- .../solr/metrics/SolrMetricManager.java | 78 +++++++--- .../solr/metrics/SolrMetricProducer.java | 52 ++++++- .../org/apache/solr/metrics/SolrMetrics.java | 94 +++++++++++ .../org/apache/solr/search/FastLRUCache.java | 58 +++---- .../java/org/apache/solr/search/LFUCache.java | 23 ++- .../java/org/apache/solr/search/LRUCache.java | 31 ++-- .../apache/solr/search/SolrCacheHolder.java | 7 +- .../solr/security/AuthenticationPlugin.java | 55 ++++--- .../handler/admin/MetricsHandlerTest.java | 146 ++++++++++++++++-- .../apache/solr/common/util/ExecutorUtil.java | 1 + 18 files changed, 503 insertions(+), 184 deletions(-) create mode 100644 solr/core/src/java/org/apache/solr/metrics/SolrMetrics.java diff --git a/solr/contrib/dataimporthandler/src/java/org/apache/solr/handler/dataimport/DataImportHandler.java b/solr/contrib/dataimporthandler/src/java/org/apache/solr/handler/dataimport/DataImportHandler.java index 50938e4c380..80d374eb505 100644 --- a/solr/contrib/dataimporthandler/src/java/org/apache/solr/handler/dataimport/DataImportHandler.java +++ b/solr/contrib/dataimporthandler/src/java/org/apache/solr/handler/dataimport/DataImportHandler.java @@ -36,7 +36,7 @@ import org.apache.solr.core.SolrCore; import org.apache.solr.core.SolrResourceLoader; import org.apache.solr.handler.RequestHandlerBase; import org.apache.solr.metrics.MetricsMap; -import org.apache.solr.metrics.SolrMetricManager; +import org.apache.solr.metrics.SolrMetrics; import org.apache.solr.request.SolrQueryRequest; import org.apache.solr.response.RawResponseWriter; import org.apache.solr.response.SolrQueryResponse; @@ -275,8 +275,8 @@ public class DataImportHandler extends RequestHandlerBase implements } @Override - public void initializeMetrics(SolrMetricManager manager, String registryName, String tag, String scope) { - super.initializeMetrics(manager, registryName, tag, scope); + public void initializeMetrics(SolrMetrics m) { + super.initializeMetrics(m); metrics = new MetricsMap((detailed, map) -> { if (importer != null) { DocBuilder.Statistics cumulative = importer.cumulativeStatistics; @@ -299,7 +299,7 @@ public class DataImportHandler extends RequestHandlerBase implements map.put(DataImporter.MSG.TOTAL_DOCS_SKIPPED, cumulative.skipDocCount); } }); - manager.registerGauge(this, registryName, metrics, tag, true, "importer", getCategory().toString(), scope); + solrMetrics.gauge(this, metrics, true, "importer", getCategory().toString()); } // //////////////////////SolrInfoMBeans methods ////////////////////// diff --git a/solr/core/src/java/org/apache/solr/cloud/ReplicateFromLeader.java b/solr/core/src/java/org/apache/solr/cloud/ReplicateFromLeader.java index 957b3212a8a..17a6ec38b97 100644 --- a/solr/core/src/java/org/apache/solr/cloud/ReplicateFromLeader.java +++ b/solr/core/src/java/org/apache/solr/cloud/ReplicateFromLeader.java @@ -133,7 +133,7 @@ public class ReplicateFromLeader { public void stopReplication() { if (replicationProcess != null) { - replicationProcess.close(); + replicationProcess.shutdown(); } } } diff --git a/solr/core/src/java/org/apache/solr/core/PluginBag.java b/solr/core/src/java/org/apache/solr/core/PluginBag.java index b5271b0b7e6..1a5e98717ea 100644 --- a/solr/core/src/java/org/apache/solr/core/PluginBag.java +++ b/solr/core/src/java/org/apache/solr/core/PluginBag.java @@ -195,10 +195,11 @@ public class PluginBag implements AutoCloseable { PluginHolder pluginHolder = new PluginHolder<>(null, plugin); pluginHolder.registerAPI = false; PluginHolder old = put(name, pluginHolder); + if(old != null) closeQuietly(old); return old == null ? null : old.get(); } - PluginHolder put(String name, PluginHolder plugin) { + public PluginHolder put(String name, PluginHolder plugin) { Boolean registerApi = null; Boolean disableHandler = null; if (plugin.pluginInfo != null) { diff --git a/solr/core/src/java/org/apache/solr/core/SolrCore.java b/solr/core/src/java/org/apache/solr/core/SolrCore.java index 19fcd14b04e..3a6f10a163b 100644 --- a/solr/core/src/java/org/apache/solr/core/SolrCore.java +++ b/solr/core/src/java/org/apache/solr/core/SolrCore.java @@ -233,7 +233,7 @@ public final class SolrCore implements SolrInfoBean, SolrMetricProducer, Closeab private final CoreContainer coreContainer; private Set metricNames = ConcurrentHashMap.newKeySet(); - private String metricTag = Integer.toHexString(hashCode()); + private final String metricTag = getUniqueMetricTag(null); public volatile boolean searchEnabled = true; public volatile boolean indexEnabled = true; diff --git a/solr/core/src/java/org/apache/solr/handler/ReplicationHandler.java b/solr/core/src/java/org/apache/solr/handler/ReplicationHandler.java index dc1d1b1bf09..76ecbecd9f2 100644 --- a/solr/core/src/java/org/apache/solr/handler/ReplicationHandler.java +++ b/solr/core/src/java/org/apache/solr/handler/ReplicationHandler.java @@ -92,7 +92,7 @@ import org.apache.solr.core.backup.repository.BackupRepository; import org.apache.solr.core.backup.repository.LocalFileSystemRepository; import org.apache.solr.handler.IndexFetcher.IndexFetchResult; import org.apache.solr.metrics.MetricsMap; -import org.apache.solr.metrics.SolrMetricManager; +import org.apache.solr.metrics.SolrMetrics; import org.apache.solr.request.SolrQueryRequest; import org.apache.solr.response.SolrQueryResponse; import org.apache.solr.search.SolrIndexSearcher; @@ -863,21 +863,20 @@ public class ReplicationHandler extends RequestHandlerBase implements SolrCoreAw } @Override - public void initializeMetrics(SolrMetricManager manager, String registry, String tag, String scope) { - super.initializeMetrics(manager, registry, tag, scope); - - manager.registerGauge(this, registry, () -> (core != null && !core.isClosed() ? NumberUtils.readableSize(core.getIndexSize()) : ""), - tag, true, "indexSize", getCategory().toString(), scope); - manager.registerGauge(this, registry, () -> (core != null && !core.isClosed() ? getIndexVersion().toString() : ""), - tag, true, "indexVersion", getCategory().toString(), scope); - manager.registerGauge(this, registry, () -> (core != null && !core.isClosed() ? getIndexVersion().generation : 0), - tag, true, GENERATION, getCategory().toString(), scope); - manager.registerGauge(this, registry, () -> (core != null && !core.isClosed() ? core.getIndexDir() : ""), - tag, true, "indexPath", getCategory().toString(), scope); - manager.registerGauge(this, registry, () -> isMaster, - tag, true, "isMaster", getCategory().toString(), scope); - manager.registerGauge(this, registry, () -> isSlave, - tag, true, "isSlave", getCategory().toString(), scope); + public void initializeMetrics(SolrMetrics m) { + super.initializeMetrics(m); + solrMetrics.gauge(this, () -> (core != null && !core.isClosed() ? NumberUtils.readableSize(core.getIndexSize()) : ""), + true, "indexSize", getCategory().toString()); + solrMetrics.gauge(this, () -> (core != null && !core.isClosed() ? getIndexVersion().toString() : ""), + true, "indexVersion", getCategory().toString()); + solrMetrics.gauge(this, () -> (core != null && !core.isClosed() ? getIndexVersion().generation : 0), + true, GENERATION, getCategory().toString()); + solrMetrics.gauge(this, () -> (core != null && !core.isClosed() ? core.getIndexDir() : ""), + true, "indexPath", getCategory().toString()); + solrMetrics.gauge(this, () -> isMaster, + true, "isMaster", getCategory().toString()); + solrMetrics.gauge(this, () -> isSlave, + true, "isSlave", getCategory().toString()); final MetricsMap fetcherMap = new MetricsMap((detailed, map) -> { IndexFetcher fetcher = currentIndexFetcher; if (fetcher != null) { @@ -906,13 +905,13 @@ public class ReplicationHandler extends RequestHandlerBase implements SolrCoreAw addVal(map, IndexFetcher.CONF_FILES_REPLICATED, props, String.class); } }); - manager.registerGauge(this, registry, fetcherMap, tag, true, "fetcher", getCategory().toString(), scope); - manager.registerGauge(this, registry, () -> isMaster && includeConfFiles != null ? includeConfFiles : "", - tag, true, "confFilesToReplicate", getCategory().toString(), scope); - manager.registerGauge(this, registry, () -> isMaster ? getReplicateAfterStrings() : Collections.emptyList(), - tag, true, REPLICATE_AFTER, getCategory().toString(), scope); - manager.registerGauge(this, registry, () -> isMaster && replicationEnabled.get(), - tag, true, "replicationEnabled", getCategory().toString(), scope); + solrMetrics.gauge(this , fetcherMap, true, "fetcher", getCategory().toString()); + solrMetrics.gauge(this, () -> isMaster && includeConfFiles != null ? includeConfFiles : "", + true, "confFilesToReplicate", getCategory().toString()); + solrMetrics.gauge(this, () -> isMaster ? getReplicateAfterStrings() : Collections.emptyList(), + true, REPLICATE_AFTER, getCategory().toString()); + solrMetrics.gauge(this, () -> isMaster && replicationEnabled.get(), + true, "replicationEnabled", getCategory().toString()); } //TODO Should a failure retrieving any piece of info mark the overall request as a failure? Is there a core set of values that are required to make a response here useful? @@ -1387,7 +1386,7 @@ public class ReplicationHandler extends RequestHandlerBase implements SolrCoreAw }); } - public void close() { + public void shutdown() { if (executorService != null) executorService.shutdown(); if (pollingIndexFetcher != null) { pollingIndexFetcher.destroy(); diff --git a/solr/core/src/java/org/apache/solr/handler/RequestHandlerBase.java b/solr/core/src/java/org/apache/solr/handler/RequestHandlerBase.java index eca391b4989..499db959e2e 100644 --- a/solr/core/src/java/org/apache/solr/handler/RequestHandlerBase.java +++ b/solr/core/src/java/org/apache/solr/handler/RequestHandlerBase.java @@ -22,11 +22,14 @@ import java.util.Map; import java.util.Set; import java.util.concurrent.ConcurrentHashMap; -import com.codahale.metrics.MetricRegistry; -import com.google.common.collect.ImmutableList; import com.codahale.metrics.Counter; import com.codahale.metrics.Meter; +import com.codahale.metrics.MetricRegistry; import com.codahale.metrics.Timer; +import com.google.common.collect.ImmutableList; +import org.apache.solr.api.Api; +import org.apache.solr.api.ApiBag; +import org.apache.solr.api.ApiSupport; import org.apache.solr.common.SolrException; import org.apache.solr.common.params.ShardParams; import org.apache.solr.common.params.SolrParams; @@ -36,16 +39,13 @@ import org.apache.solr.core.PluginBag; import org.apache.solr.core.PluginInfo; import org.apache.solr.core.SolrInfoBean; import org.apache.solr.metrics.MetricsMap; -import org.apache.solr.metrics.SolrMetricManager; import org.apache.solr.metrics.SolrMetricProducer; +import org.apache.solr.metrics.SolrMetrics; import org.apache.solr.request.SolrQueryRequest; import org.apache.solr.request.SolrRequestHandler; import org.apache.solr.response.SolrQueryResponse; import org.apache.solr.search.SyntaxError; import org.apache.solr.util.SolrPluginUtils; -import org.apache.solr.api.Api; -import org.apache.solr.api.ApiBag; -import org.apache.solr.api.ApiSupport; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -79,9 +79,6 @@ public abstract class RequestHandlerBase implements SolrRequestHandler, SolrInfo private PluginInfo pluginInfo; private Set metricNames = ConcurrentHashMap.newKeySet(); - private MetricRegistry registry; - protected String registryName; - protected SolrMetricManager metricManager; @SuppressForbidden(reason = "Need currentTimeMillis, used only for stats output") @@ -143,22 +140,27 @@ public abstract class RequestHandlerBase implements SolrRequestHandler, SolrInfo } + protected SolrMetrics solrMetrics; + @Override - public void initializeMetrics(SolrMetricManager manager, String registryName, String tag, final String scope) { - this.metricManager = manager; - this.registryName = registryName; - this.registry = manager.registry(registryName); - numErrors = manager.meter(this, registryName, "errors", getCategory().toString(), scope); - numServerErrors = manager.meter(this, registryName, "serverErrors", getCategory().toString(), scope); - numClientErrors = manager.meter(this, registryName, "clientErrors", getCategory().toString(), scope); - numTimeouts = manager.meter(this, registryName, "timeouts", getCategory().toString(), scope); - requests = manager.counter(this, registryName, "requests", getCategory().toString(), scope); + public SolrMetrics getMetrics() { + return solrMetrics; + } + + @Override + public void initializeMetrics(SolrMetrics m) { + this.solrMetrics = m.getChildInfo(this); + numErrors = solrMetrics.meter(this, "errors", getCategory().toString()); + numServerErrors = solrMetrics.meter(this, "serverErrors", getCategory().toString()); + numClientErrors = solrMetrics.meter(this, "clientErrors", getCategory().toString()); + numTimeouts = solrMetrics.meter(this, "timeouts", getCategory().toString()); + requests = solrMetrics.counter(this, "requests", getCategory().toString()); MetricsMap metricsMap = new MetricsMap((detail, map) -> shardPurposes.forEach((k, v) -> map.put(k, v.getCount()))); - manager.registerGauge(this, registryName, metricsMap, tag, true, "shardRequests", getCategory().toString(), scope); - requestTimes = manager.timer(this, registryName, "requestTimes", getCategory().toString(), scope); - totalTime = manager.counter(this, registryName, "totalTime", getCategory().toString(), scope); - manager.registerGauge(this, registryName, () -> handlerStart, tag, true, "handlerStart", getCategory().toString(), scope); + solrMetrics.gauge(this, metricsMap, true, "shardRequests", getCategory().toString()); + requestTimes = solrMetrics.timer(this,"requestTimes", getCategory().toString()); + totalTime = solrMetrics.counter(this, "totalTime", getCategory().toString()); + solrMetrics.gauge(this, () -> handlerStart, true, "handlerStart", getCategory().toString()); } public static SolrParams getSolrParamsFromNamedList(NamedList args, String key) { @@ -274,7 +276,7 @@ public abstract class RequestHandlerBase implements SolrRequestHandler, SolrInfo @Override public MetricRegistry getMetricRegistry() { - return registry; + return solrMetrics.getRegistry(); } @Override diff --git a/solr/core/src/java/org/apache/solr/handler/admin/CoreAdminHandler.java b/solr/core/src/java/org/apache/solr/handler/admin/CoreAdminHandler.java index 45cb0631e87..ae225556b13 100644 --- a/solr/core/src/java/org/apache/solr/handler/admin/CoreAdminHandler.java +++ b/solr/core/src/java/org/apache/solr/handler/admin/CoreAdminHandler.java @@ -46,6 +46,7 @@ import org.apache.solr.core.CoreDescriptor; import org.apache.solr.handler.RequestHandlerBase; import org.apache.solr.logging.MDCLoggingContext; import org.apache.solr.metrics.SolrMetricManager; +import org.apache.solr.metrics.SolrMetrics; import org.apache.solr.request.SolrQueryRequest; import org.apache.solr.response.SolrQueryResponse; import org.apache.solr.security.AuthorizationContext; @@ -120,10 +121,10 @@ public class CoreAdminHandler extends RequestHandlerBase implements PermissionNa } @Override - public void initializeMetrics(SolrMetricManager manager, String registryName, String tag, String scope) { - super.initializeMetrics(manager, registryName, tag, scope); - parallelExecutor = MetricUtils.instrumentedExecutorService(parallelExecutor, this, manager.registry(registryName), - SolrMetricManager.mkName("parallelCoreAdminExecutor", getCategory().name(),scope, "threadPool")); + public void initializeMetrics(SolrMetrics m) { + super.initializeMetrics(m); + parallelExecutor = MetricUtils.instrumentedExecutorService(parallelExecutor, this, solrMetrics.getRegistry(), + SolrMetricManager.mkName("parallelCoreAdminExecutor", getCategory().name(), solrMetrics.scope, "threadPool")); } @Override public Boolean registerV2() { diff --git a/solr/core/src/java/org/apache/solr/handler/component/SuggestComponent.java b/solr/core/src/java/org/apache/solr/handler/component/SuggestComponent.java index 2d6fdb15594..00d86970e51 100644 --- a/solr/core/src/java/org/apache/solr/handler/component/SuggestComponent.java +++ b/solr/core/src/java/org/apache/solr/handler/component/SuggestComponent.java @@ -48,8 +48,8 @@ import org.apache.solr.common.util.SimpleOrderedMap; import org.apache.solr.core.SolrCore; import org.apache.solr.core.SolrEventListener; import org.apache.solr.metrics.MetricsMap; -import org.apache.solr.metrics.SolrMetricManager; import org.apache.solr.metrics.SolrMetricProducer; +import org.apache.solr.metrics.SolrMetrics; import org.apache.solr.search.SolrIndexSearcher; import org.apache.solr.spelling.suggest.SolrSuggester; import org.apache.solr.spelling.suggest.SuggesterOptions; @@ -88,9 +88,6 @@ public class SuggestComponent extends SearchComponent implements SolrCoreAware, @SuppressWarnings("unchecked") protected NamedList initParams; - protected SolrMetricManager metricManager; - protected String registryName; - /** * Key is the dictionary name used in SolrConfig, value is the corresponding {@link SolrSuggester} */ @@ -350,19 +347,25 @@ public class SuggestComponent extends SearchComponent implements SolrCoreAware, return "Suggester component"; } + protected SolrMetrics metricsInfo; + @Override - public void initializeMetrics(SolrMetricManager manager, String registryName, String tag, String scope) { - this.registryName = registryName; - this.metricManager = manager; - registry = manager.registry(registryName); - manager.registerGauge(this, registryName, () -> ramBytesUsed(), tag, true, "totalSizeInBytes", getCategory().toString(), scope); + public SolrMetrics getMetrics() { + return metricsInfo; + } + + @Override + public void initializeMetrics(SolrMetrics info) { + this.metricsInfo = info.getChildInfo(this); + + metricsInfo.metricManager.registerGauge(this, info.registry, () -> ramBytesUsed(), metricsInfo.tag, true, "totalSizeInBytes", getCategory().toString(), metricsInfo.scope); MetricsMap suggestersMap = new MetricsMap((detailed, map) -> { for (Map.Entry entry : suggesters.entrySet()) { SolrSuggester suggester = entry.getValue(); map.put(entry.getKey(), suggester.toString()); } }); - manager.registerGauge(this, registryName, suggestersMap, tag, true, "suggesters", getCategory().toString(), scope); + metricsInfo.metricManager.registerGauge(this, metricsInfo.registry, suggestersMap, metricsInfo.tag, true, "suggesters", getCategory().toString(), metricsInfo.scope); } @Override diff --git a/solr/core/src/java/org/apache/solr/metrics/SolrMetricManager.java b/solr/core/src/java/org/apache/solr/metrics/SolrMetricManager.java index 23a8bf14177..a8a1a4fbc0f 100644 --- a/solr/core/src/java/org/apache/solr/metrics/SolrMetricManager.java +++ b/solr/core/src/java/org/apache/solr/metrics/SolrMetricManager.java @@ -599,10 +599,14 @@ public class SolrMetricManager { */ public Meter meter(SolrInfoBean info, String registry, String metricName, String... metricPath) { final String name = mkName(metricName, metricPath); + return meter(info, registry(registry), name); + } + + public Meter meter(SolrInfoBean info, MetricRegistry registry, String metricsPath) { if (info != null) { - info.registerMetricName(name); + info.registerMetricName(metricsPath); } - return registry(registry).meter(name, meterSupplier); + return registry.meter(metricsPath, meterSupplier); } /** @@ -622,6 +626,14 @@ public class SolrMetricManager { return registry(registry).timer(name, timerSupplier); } + public Timer timer(SolrInfoBean info, MetricRegistry registry, String name) { + if (info != null) { + info.registerMetricName(name); + } + return registry.timer(name, timerSupplier); + + } + /** * Create or get an existing named {@link Counter} * @@ -633,10 +645,15 @@ public class SolrMetricManager { */ public Counter counter(SolrInfoBean info, String registry, String metricName, String... metricPath) { final String name = mkName(metricName, metricPath); + return counter(info, registry(registry), name); + } + + public Counter counter(SolrInfoBean info, MetricRegistry registry, String name) { if (info != null) { info.registerMetricName(name); } - return registry(registry).counter(name, counterSupplier); + return registry.counter(name, counterSupplier); + } /** @@ -682,13 +699,27 @@ public class SolrMetricManager { } } - /** - * This is a wrapper for {@link Gauge} metrics, which are usually implemented as - * lambdas that often keep a reference to their parent instance. In order to make sure that - * all such metrics are removed when their parent instance is removed / closed the - * metric is associated with an instance tag, which can be used then to remove - * wrappers with the matching tag using {@link #unregisterGauges(String, String)}. - */ + public void registerGauge(SolrInfoBean info, MetricRegistry registry, Gauge g, boolean force, String name) { + if (info != null) { + info.registerMetricName(name); + } + synchronized (registry) { + if (force && registry.getMetrics().containsKey(name)) { + registry.remove(name); + } + registry.register(name, g); + } + + } + + + /** + * This is a wrapper for {@link Gauge} metrics, which are usually implemented as + * lambdas that often keep a reference to their parent instance. In order to make sure that + * all such metrics are removed when their parent instance is removed / closed the + * metric is associated with an instance tag, which can be used then to remove + * wrappers with the matching tag using {@link #unregisterGauges(String, String)}. + */ public static class GaugeWrapper implements Gauge { private final Gauge gauge; private final String tag; @@ -716,20 +747,22 @@ public class SolrMetricManager { registerMetric(info, registry, new GaugeWrapper(gauge, tag), force, metricName, metricPath); } - public int unregisterGauges(String registryName, String tag) { - if (tag == null) { + public int unregisterGauges(String registryName, String tagSegment) { + if (tagSegment == null) { return 0; } MetricRegistry registry = registry(registryName); + if (registry == null) return 0; AtomicInteger removed = new AtomicInteger(); registry.removeMatching((name, metric) -> { - if (metric instanceof GaugeWrapper && - tag.equals(((GaugeWrapper) metric).getTag())) { - removed.incrementAndGet(); - return true; - } else { - return false; + if (metric instanceof GaugeWrapper) { + GaugeWrapper wrapper = (GaugeWrapper) metric; + boolean toRemove = tagSegment.equals(wrapper.getTag()) || wrapper.getTag().contains(tagSegment); + if (toRemove) removed.incrementAndGet(); + return toRemove; } + return false; + }); return removed.get(); } @@ -744,10 +777,16 @@ public class SolrMetricManager { * segments prepended to the name. */ public static String mkName(String name, String... path) { + return makeName(path == null || path.length == 0 ? Collections.emptyList() : Arrays.asList(path), + name); + + } + + public static String makeName(List path, String name) { if (name == null || name.isEmpty()) { throw new IllegalArgumentException("name must not be empty"); } - if (path == null || path.length == 0) { + if (path == null || path.size() == 0) { return name; } else { StringBuilder sb = new StringBuilder(); @@ -766,6 +805,7 @@ public class SolrMetricManager { sb.append(name); return sb.toString(); } + } /** diff --git a/solr/core/src/java/org/apache/solr/metrics/SolrMetricProducer.java b/solr/core/src/java/org/apache/solr/metrics/SolrMetricProducer.java index d5c23b55a52..cb534aca5dc 100644 --- a/solr/core/src/java/org/apache/solr/metrics/SolrMetricProducer.java +++ b/solr/core/src/java/org/apache/solr/metrics/SolrMetricProducer.java @@ -19,17 +19,53 @@ package org.apache.solr.metrics; /** * Used by objects that expose metrics through {@link SolrCoreMetricManager}. */ -public interface SolrMetricProducer { +public interface SolrMetricProducer extends AutoCloseable { + + /** + * Unique metric name is in the format of A.B.C + * A is the parent of B is the parent of C and so on. + * If object "B" is unregistered , C also must get unregistered. + * If object "A" is unregistered , B , C also must get unregistered. + */ + default String getUniqueMetricTag(String parentName) { + String name = getClass().getSimpleName() + "@" + Integer.toHexString(hashCode()); + if (parentName != null && parentName.contains(name)) return parentName; + return parentName == null ? + name : + parentName + ":" + name; + } + /** * Initializes metrics specific to this producer - * @param manager an instance of {@link SolrMetricManager} + * + * @param manager an instance of {@link SolrMetricManager} * @param registry registry name where metrics are registered - * @param tag a symbolic tag that represents this instance of the producer, - * or a group of related instances that have the same life-cycle. This tag is - * used when managing life-cycle of some metrics and is set when - * {@link #initializeMetrics(SolrMetricManager, String, String, String)} is called. - * @param scope scope of the metrics (eg. handler name) to separate metrics of + * @param tag a symbolic tag that represents this instance of the producer, + * or a group of related instances that have the same life-cycle. This tag is + * used when managing life-cycle of some metrics and is set when + * {@link #initializeMetrics(SolrMetricManager, String, String, String)} is called. + * @param scope scope of the metrics (eg. handler name) to separate metrics of */ - void initializeMetrics(SolrMetricManager manager, String registry, String tag, String scope); + default void initializeMetrics(SolrMetricManager manager, String registry, String tag, String scope) { + initializeMetrics(new SolrMetrics(manager, registry, tag, scope)); + + } + + default void initializeMetrics(SolrMetrics info) { + throw new RuntimeException("This means , the class has not implemented both of these methods"); + + } + + default SolrMetrics getMetrics() { + return null; + } + + @Override + default void close() throws Exception { + SolrMetrics info = getMetrics(); + if (info == null || info.tag.indexOf(':') == -1) return;//this will end up unregistering the root itself + info.unregister(); + } + } diff --git a/solr/core/src/java/org/apache/solr/metrics/SolrMetrics.java b/solr/core/src/java/org/apache/solr/metrics/SolrMetrics.java new file mode 100644 index 00000000000..28aea4e4675 --- /dev/null +++ b/solr/core/src/java/org/apache/solr/metrics/SolrMetrics.java @@ -0,0 +1,94 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF 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.apache.solr.metrics; + +import java.util.ArrayList; +import java.util.Collections; + +import com.codahale.metrics.Counter; +import com.codahale.metrics.Gauge; +import com.codahale.metrics.Meter; +import com.codahale.metrics.MetricRegistry; +import com.codahale.metrics.Timer; +import org.apache.solr.core.SolrInfoBean; + +import static org.apache.solr.metrics.SolrMetricManager.makeName; + +public class SolrMetrics { + public final String registry; + public final SolrMetricManager metricManager; + public final String tag; + public final String scope; + private SolrMetrics parent; + + public SolrMetrics(SolrMetricManager metricManager, String registry, String tag, String scope) { + this.registry = registry; + this.metricManager = metricManager; + this.tag = tag; + this.scope = scope; + } + + public String getTag() { + return tag; + } + + public void unregister() { + metricManager.unregisterGauges(registry, tag); + } + + public SolrMetrics getChildInfo(SolrMetricProducer producer) { + SolrMetrics metricsInfo = new SolrMetrics(metricManager, registry, producer.getUniqueMetricTag(tag), scope); + metricsInfo.parent = this; + return metricsInfo; + } + + public Meter meter(SolrInfoBean info, String metricName, String... metricpath) { + return metricManager.meter(info, getRegistry(), createName(metricName, metricpath)); + } + + private String createName(String metricName, String... metricpath) { + ArrayList l = new ArrayList<>(); + if(metricpath != null ) { + Collections.addAll(l, metricpath); + } + l.add(scope); + return makeName(l, metricName); + } + + public Counter counter(SolrInfoBean info, String metricName, String... metricpath) { + return metricManager.counter(info, getRegistry(), createName(metricName, metricpath)); + + } + + public void gauge(SolrInfoBean info, Gauge gauge, boolean force, String metricName, String... metricpath) { + metricManager.registerGauge(info, getRegistry(), new SolrMetricManager.GaugeWrapper<>(gauge, tag), force, createName(metricName, metricpath)); + } + + public Timer timer(SolrInfoBean info, String metricName, String... metricpath) { + return metricManager.timer(info, getRegistry(), createName(metricName, metricpath)); + + } + + public SolrMetrics getParent() { + return parent; + } + + public MetricRegistry getRegistry() { + return metricManager.registry(registry); + } +} diff --git a/solr/core/src/java/org/apache/solr/search/FastLRUCache.java b/solr/core/src/java/org/apache/solr/search/FastLRUCache.java index 6c16783f433..5487987b057 100644 --- a/solr/core/src/java/org/apache/solr/search/FastLRUCache.java +++ b/solr/core/src/java/org/apache/solr/search/FastLRUCache.java @@ -16,24 +16,24 @@ */ package org.apache.solr.search; +import java.lang.invoke.MethodHandles; +import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.CopyOnWriteArrayList; +import java.util.concurrent.TimeUnit; + import com.codahale.metrics.MetricRegistry; import org.apache.lucene.util.Accountable; import org.apache.lucene.util.RamUsageEstimator; import org.apache.solr.common.SolrException; import org.apache.solr.metrics.MetricsMap; -import org.apache.solr.metrics.SolrMetricManager; +import org.apache.solr.metrics.SolrMetrics; import org.apache.solr.util.ConcurrentLRUCache; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import java.lang.invoke.MethodHandles; -import java.util.concurrent.ConcurrentHashMap; -import java.util.List; -import java.util.Map; -import java.util.Set; -import java.util.concurrent.CopyOnWriteArrayList; -import java.util.concurrent.TimeUnit; - /** * SolrCache based on ConcurrentLRUCache implementation. *

@@ -42,12 +42,11 @@ import java.util.concurrent.TimeUnit; *

* Also see SolrCaching * - * * @see org.apache.solr.util.ConcurrentLRUCache * @see org.apache.solr.search.SolrCache * @since solr 1.4 */ -public class FastLRUCache extends SolrCacheBase implements SolrCache, Accountable { +public class FastLRUCache extends SolrCacheBase implements SolrCache, Accountable { private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass()); private static final long BASE_RAM_BYTES_USED = RamUsageEstimator.shallowSizeOfInstance(FastLRUCache.class); @@ -64,7 +63,7 @@ public class FastLRUCache extends SolrCacheBase implements SolrCache, private long warmupTime = 0; private String description = "Concurrent LRU Cache"; - private ConcurrentLRUCache cache; + private ConcurrentLRUCache cache; private int showItems = 0; private long maxRamBytes; @@ -77,7 +76,6 @@ public class FastLRUCache extends SolrCacheBase implements SolrCache, private MetricsMap cacheMap; private Set metricNames = ConcurrentHashMap.newKeySet(); - private MetricRegistry registry; @Override public Object init(Map args, Object persistence, CacheRegenerator regenerator) { @@ -112,11 +110,11 @@ public class FastLRUCache extends SolrCacheBase implements SolrCache, str = (String) args.get(MAX_RAM_MB_PARAM); long maxRamMB = str == null ? -1 : (long) Double.parseDouble(str); this.maxRamBytes = maxRamMB < 0 ? Long.MAX_VALUE : maxRamMB * 1024L * 1024L; - if (maxRamBytes != Long.MAX_VALUE) { + if (maxRamBytes != Long.MAX_VALUE) { ramLowerWatermark = Math.round(maxRamBytes * 0.8); description = generateDescription(maxRamBytes, ramLowerWatermark, cleanupThread); cache = new ConcurrentLRUCache<>(ramLowerWatermark, maxRamBytes, cleanupThread, null); - } else { + } else { ramLowerWatermark = -1L; description = generateDescription(maxSize, initialSize, minSizeLimit, acceptableSize, cleanupThread); cache = new ConcurrentLRUCache<>(maxSize, minSizeLimit, acceptableSize, initialSize, cleanupThread, false, null); @@ -151,7 +149,7 @@ public class FastLRUCache extends SolrCacheBase implements SolrCache, */ protected String generateDescription(int limit, int initialSize, int minLimit, int acceptableLimit, boolean newThread) { String description = "Concurrent LRU Cache(maxSize=" + limit + ", initialSize=" + initialSize + - ", minSize="+minLimit + ", acceptableSize="+acceptableLimit+", cleanupThread="+newThread; + ", minSize=" + minLimit + ", acceptableSize=" + acceptableLimit + ", cleanupThread=" + newThread; if (isAutowarmingOn()) { description += ", " + getAutowarmDescription(); } @@ -212,10 +210,9 @@ public class FastLRUCache extends SolrCacheBase implements SolrCache, for (int i = itemsArr.length - 1; i >= 0; i--) { try { boolean continueRegen = regenerator.regenerateItem(searcher, - this, old, itemsArr[i].getKey(), itemsArr[i].getValue()); + this, old, itemsArr[i].getKey(), itemsArr[i].getValue()); if (!continueRegen) break; - } - catch (Exception e) { + } catch (Exception e) { SolrException.log(log, "Error during auto-warming of key:" + itemsArr[i].getKey(), e); } } @@ -226,6 +223,7 @@ public class FastLRUCache extends SolrCacheBase implements SolrCache, @Override public void close() { + if (metricsInfo != null) metricsInfo.unregister(); // add the stats to the cumulative stats object (the first in the statsList) statsList.get(0).add(cache.getStats()); statsList.remove(cache.getStats()); @@ -248,9 +246,17 @@ public class FastLRUCache extends SolrCacheBase implements SolrCache, return metricNames; } + + SolrMetrics metricsInfo; + @Override - public void initializeMetrics(SolrMetricManager manager, String registryName, String tag, String scope) { - registry = manager.registry(registryName); + public SolrMetrics getMetrics() { + return metricsInfo; + } + + @Override + public void initializeMetrics(SolrMetrics info) { + metricsInfo = info.getChildInfo(this); cacheMap = new MetricsMap((detailed, map) -> { if (cache != null) { ConcurrentLRUCache.Stats stats = cache.getStats(); @@ -290,20 +296,20 @@ public class FastLRUCache extends SolrCacheBase implements SolrCache, map.put("cumulative_evictions", cevictions); if (detailed && showItems != 0) { - Map items = cache.getLatestAccessedItems( showItems == -1 ? Integer.MAX_VALUE : showItems ); - for (Map.Entry e : (Set )items.entrySet()) { + Map items = cache.getLatestAccessedItems(showItems == -1 ? Integer.MAX_VALUE : showItems); + for (Map.Entry e : (Set) items.entrySet()) { Object k = e.getKey(); Object v = e.getValue(); String ks = "item_" + k; String vs = v.toString(); - map.put(ks,vs); + map.put(ks, vs); } } } }); - manager.registerGauge(this, registryName, cacheMap, tag, true, scope, getCategory().toString()); + metricsInfo.metricManager.registerGauge(this, metricsInfo.registry, cacheMap, metricsInfo.tag, true, metricsInfo.scope, getCategory().toString()); } @@ -314,7 +320,7 @@ public class FastLRUCache extends SolrCacheBase implements SolrCache, @Override public MetricRegistry getMetricRegistry() { - return registry; + return metricsInfo ==null ?null:metricsInfo.getRegistry(); } @Override diff --git a/solr/core/src/java/org/apache/solr/search/LFUCache.java b/solr/core/src/java/org/apache/solr/search/LFUCache.java index f1c30d057c4..0d69e4fbd1b 100644 --- a/solr/core/src/java/org/apache/solr/search/LFUCache.java +++ b/solr/core/src/java/org/apache/solr/search/LFUCache.java @@ -17,10 +17,10 @@ package org.apache.solr.search; import java.lang.invoke.MethodHandles; -import java.util.concurrent.ConcurrentHashMap; import java.util.List; import java.util.Map; import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.CopyOnWriteArrayList; import java.util.concurrent.TimeUnit; @@ -29,7 +29,7 @@ import org.apache.lucene.util.Accountable; import org.apache.lucene.util.RamUsageEstimator; import org.apache.solr.common.SolrException; import org.apache.solr.metrics.MetricsMap; -import org.apache.solr.metrics.SolrMetricManager; +import org.apache.solr.metrics.SolrMetrics; import org.apache.solr.util.ConcurrentLFUCache; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -78,7 +78,6 @@ public class LFUCache implements SolrCache, Accountable { private Boolean timeDecay = true; private MetricsMap cacheMap; private Set metricNames = ConcurrentHashMap.newKeySet(); - private MetricRegistry registry; private int maxSize; private int minSizeLimit; @@ -226,6 +225,7 @@ public class LFUCache implements SolrCache, Accountable { statsList.get(0).add(cache.getStats()); statsList.remove(cache.getStats()); cache.destroy(); + if (solrMetrics != null) solrMetrics.unregister(); } //////////////////////// SolrInfoMBeans methods ////////////////////// @@ -253,9 +253,17 @@ public class LFUCache implements SolrCache, Accountable { return "0." + hundredths; } + + private SolrMetrics solrMetrics; + @Override - public void initializeMetrics(SolrMetricManager manager, String registryName, String tag, String scope) { - registry = manager.registry(registryName); + public SolrMetrics getMetrics() { + return solrMetrics; + } + + @Override + public void initializeMetrics(SolrMetrics info) { + solrMetrics = info.getChildInfo(this); cacheMap = new MetricsMap((detailed, map) -> { if (cache != null) { ConcurrentLFUCache.Stats stats = cache.getStats(); @@ -316,7 +324,7 @@ public class LFUCache implements SolrCache, Accountable { } }); - manager.registerGauge(this, registryName, cacheMap, tag, true, scope, getCategory().toString()); + solrMetrics.metricManager.registerGauge(this, solrMetrics.registry, cacheMap, solrMetrics.getTag(), true, solrMetrics.scope, getCategory().toString()); } // for unit tests only @@ -331,7 +339,8 @@ public class LFUCache implements SolrCache, Accountable { @Override public MetricRegistry getMetricRegistry() { - return registry; + return solrMetrics == null ? null : solrMetrics.getRegistry(); + } @Override diff --git a/solr/core/src/java/org/apache/solr/search/LRUCache.java b/solr/core/src/java/org/apache/solr/search/LRUCache.java index 82e2700a516..e26563025b3 100644 --- a/solr/core/src/java/org/apache/solr/search/LRUCache.java +++ b/solr/core/src/java/org/apache/solr/search/LRUCache.java @@ -18,11 +18,11 @@ package org.apache.solr.search; import java.lang.invoke.MethodHandles; import java.util.Collection; -import java.util.concurrent.ConcurrentHashMap; import java.util.Iterator; import java.util.LinkedHashMap; import java.util.Map; import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.LongAdder; @@ -32,7 +32,7 @@ import org.apache.lucene.util.Accountables; import org.apache.lucene.util.RamUsageEstimator; import org.apache.solr.common.SolrException; import org.apache.solr.metrics.MetricsMap; -import org.apache.solr.metrics.SolrMetricManager; +import org.apache.solr.metrics.SolrMetrics; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -74,7 +74,6 @@ public class LRUCache extends SolrCacheBase implements SolrCache, Acco private String description="LRU Cache"; private MetricsMap cacheMap; private Set metricNames = ConcurrentHashMap.newKeySet(); - private MetricRegistry registry; private int maxSize; private int initialSize; @@ -153,8 +152,8 @@ public class LRUCache extends SolrCacheBase implements SolrCache, Acco } /** - * - * @return Returns the description of this cache. + * + * @return Returns the description of this cache. */ private String generateDescription() { String description = "LRU Cache(maxSize=" + getMaxSize() + ", initialSize=" + initialSize; @@ -242,9 +241,9 @@ public class LRUCache extends SolrCacheBase implements SolrCache, Acco // Don't do the autowarming in the synchronized block, just pull out the keys and values. synchronized (other.map) { - + int sz = autowarm.getWarmCount(other.map.size()); - + keys = new Object[sz]; vals = new Object[sz]; @@ -281,10 +280,9 @@ public class LRUCache extends SolrCacheBase implements SolrCache, Acco @Override public void close() { - + if(solrMetrics != null) solrMetrics.unregister(); } - //////////////////////// SolrInfoMBeans methods ////////////////////// @@ -303,9 +301,16 @@ public class LRUCache extends SolrCacheBase implements SolrCache, Acco return metricNames; } + SolrMetrics solrMetrics; + @Override - public void initializeMetrics(SolrMetricManager manager, String registryName, String tag, String scope) { - registry = manager.registry(registryName); + public SolrMetrics getMetrics() { + return solrMetrics; + } + + @Override + public void initializeMetrics(SolrMetrics m) { + solrMetrics = m.getChildInfo(this); cacheMap = new MetricsMap((detailed, res) -> { synchronized (map) { res.put(LOOKUPS_PARAM, lookups); @@ -330,7 +335,7 @@ public class LRUCache extends SolrCacheBase implements SolrCache, Acco res.put("cumulative_evictions", stats.evictions.longValue()); res.put("cumulative_evictionsRamUsage", stats.evictionsRamUsage.longValue()); }); - manager.registerGauge(this, registryName, cacheMap, tag, true, scope, getCategory().toString()); + solrMetrics.metricManager.registerGauge(this, solrMetrics.registry, cacheMap, solrMetrics.tag, true, solrMetrics.scope, getCategory().toString()); } // for unit tests only @@ -340,7 +345,7 @@ public class LRUCache extends SolrCacheBase implements SolrCache, Acco @Override public MetricRegistry getMetricRegistry() { - return registry; + return solrMetrics ==null ?null: solrMetrics.getRegistry(); } @Override diff --git a/solr/core/src/java/org/apache/solr/search/SolrCacheHolder.java b/solr/core/src/java/org/apache/solr/search/SolrCacheHolder.java index 82a882c94d3..86f6b2d46cc 100644 --- a/solr/core/src/java/org/apache/solr/search/SolrCacheHolder.java +++ b/solr/core/src/java/org/apache/solr/search/SolrCacheHolder.java @@ -22,7 +22,7 @@ import java.util.Map; import java.util.Set; import com.codahale.metrics.MetricRegistry; -import org.apache.solr.metrics.SolrMetricManager; +import org.apache.solr.metrics.SolrMetrics; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -141,9 +141,8 @@ public class SolrCacheHolder implements SolrCache { } @Override - public void initializeMetrics(SolrMetricManager manager, String registry, String tag, String scope) { - - delegate.initializeMetrics(manager, registry, tag,scope); + public void initializeMetrics(SolrMetrics info) { + delegate.initializeMetrics(info); } diff --git a/solr/core/src/java/org/apache/solr/security/AuthenticationPlugin.java b/solr/core/src/java/org/apache/solr/security/AuthenticationPlugin.java index 5fd18a1ab79..8044af9f5f7 100644 --- a/solr/core/src/java/org/apache/solr/security/AuthenticationPlugin.java +++ b/solr/core/src/java/org/apache/solr/security/AuthenticationPlugin.java @@ -19,7 +19,6 @@ package org.apache.solr.security; import javax.servlet.FilterChain; import javax.servlet.ServletRequest; import javax.servlet.ServletResponse; -import java.io.Closeable; import java.util.Arrays; import java.util.Map; import java.util.Set; @@ -29,29 +28,25 @@ import com.codahale.metrics.Counter; import com.codahale.metrics.Meter; import com.codahale.metrics.MetricRegistry; import com.codahale.metrics.Timer; -import org.apache.solr.core.SolrInfoBean; -import org.apache.solr.metrics.SolrMetricManager; -import org.apache.solr.metrics.SolrMetricProducer; - import org.apache.http.HttpRequest; import org.apache.http.protocol.HttpContext; +import org.apache.solr.core.SolrInfoBean; +import org.apache.solr.metrics.SolrMetricProducer; +import org.apache.solr.metrics.SolrMetrics; import org.eclipse.jetty.client.api.Request; /** * * @lucene.experimental */ -public abstract class AuthenticationPlugin implements Closeable, SolrInfoBean, SolrMetricProducer { +public abstract class AuthenticationPlugin implements SolrInfoBean, SolrMetricProducer { final public static String AUTHENTICATION_PLUGIN_PROP = "authenticationPlugin"; final public static String HTTP_HEADER_X_SOLR_AUTHDATA = "X-Solr-AuthData"; // Metrics private Set metricNames = ConcurrentHashMap.newKeySet(); - private MetricRegistry registry; - protected String registryName; - protected SolrMetricManager metricManager; protected Meter numErrors = new Meter(); protected Counter requests = new Counter(); protected Timer requestTimes = new Timer(); @@ -66,7 +61,7 @@ public abstract class AuthenticationPlugin implements Closeable, SolrInfoBean, S * @param pluginConfig Config parameters, possibly from a ZK source */ public abstract void init(Map pluginConfig); - + /** * This method attempts to authenticate the request. Upon a successful authentication, this * must call the next filter in the filter chain and set the user principal of the request, @@ -107,10 +102,10 @@ public abstract class AuthenticationPlugin implements Closeable, SolrInfoBean, S * delegate to {@link PKIAuthenticationPlugin}. Return true to indicate that your plugin * did handle the request, or false to signal that PKI plugin should handle it. This method * will be called by {@link PKIAuthenticationPlugin}'s interceptor. - * + * *

* If not overridden, this method will return true for plugins implementing {@link HttpClientBuilderPlugin}. - * This method can be overridden by subclasses e.g. to set HTTP headers, even if you don't use a clientBuilder. + * This method can be overridden by subclasses e.g. to set HTTP headers, even if you don't use a clientBuilder. *

* @param httpRequest the httpRequest that is about to be sent to another internal Solr node * @param httpContext the context of that request. @@ -137,31 +132,35 @@ public abstract class AuthenticationPlugin implements Closeable, SolrInfoBean, S protected boolean interceptInternodeRequest(Request request) { return this instanceof HttpClientBuilderPlugin; } - + /** * Cleanup any per request data */ public void closeRequest() { } + protected SolrMetrics metrics; @Override - public void initializeMetrics(SolrMetricManager manager, String registryName, String tag, final String scope) { - this.metricManager = manager; - this.registryName = registryName; + public SolrMetrics getMetrics() { + return metrics; + } + + @Override + public void initializeMetrics(SolrMetrics metrics) { + this.metrics = metrics.getChildInfo(this); // Metrics - registry = manager.registry(registryName); - numErrors = manager.meter(this, registryName, "errors", getCategory().toString(), scope); - requests = manager.counter(this, registryName, "requests", getCategory().toString(), scope); - numAuthenticated = manager.counter(this, registryName, "authenticated", getCategory().toString(), scope); - numPassThrough = manager.counter(this, registryName, "passThrough", getCategory().toString(), scope); - numWrongCredentials = manager.counter(this, registryName, "failWrongCredentials", getCategory().toString(), scope); - numMissingCredentials = manager.counter(this, registryName, "failMissingCredentials", getCategory().toString(), scope); - requestTimes = manager.timer(this, registryName, "requestTimes", getCategory().toString(), scope); - totalTime = manager.counter(this, registryName, "totalTime", getCategory().toString(), scope); + numErrors = this.metrics.meter(this, "errors", getCategory().toString()); + requests = this.metrics.counter(this, "requests", getCategory().toString()); + numAuthenticated = this.metrics.counter(this, "authenticated",getCategory().toString()); + numPassThrough = this.metrics.counter(this, "passThrough", getCategory().toString()); + numWrongCredentials = this.metrics.counter(this, "failWrongCredentials",getCategory().toString()); + numMissingCredentials = this.metrics.counter(this, "failMissingCredentials",getCategory().toString()); + requestTimes = this.metrics.timer(this,"requestTimes", getCategory().toString()); + totalTime = this.metrics.counter(this,"totalTime", getCategory().toString()); metricNames.addAll(Arrays.asList("errors", "requests", "authenticated", "passThrough", "failWrongCredentials", "failMissingCredentials", "requestTimes", "totalTime")); } - + @Override public String getName() { return this.getClass().getName(); @@ -184,7 +183,7 @@ public abstract class AuthenticationPlugin implements Closeable, SolrInfoBean, S @Override public MetricRegistry getMetricRegistry() { - return registry; + return metrics == null ? null : metrics.getRegistry(); } - + } diff --git a/solr/core/src/test/org/apache/solr/handler/admin/MetricsHandlerTest.java b/solr/core/src/test/org/apache/solr/handler/admin/MetricsHandlerTest.java index a6dbd9ecd54..3c16ce65a5f 100644 --- a/solr/core/src/test/org/apache/solr/handler/admin/MetricsHandlerTest.java +++ b/solr/core/src/test/org/apache/solr/handler/admin/MetricsHandlerTest.java @@ -17,6 +17,7 @@ package org.apache.solr.handler.admin; +import java.util.Arrays; import java.util.Map; import com.codahale.metrics.Counter; @@ -24,6 +25,15 @@ import org.apache.solr.SolrTestCaseJ4; import org.apache.solr.common.params.CommonParams; import org.apache.solr.common.util.NamedList; import org.apache.solr.common.util.SimpleOrderedMap; +import org.apache.solr.common.util.Utils; +import org.apache.solr.core.PluginBag; +import org.apache.solr.core.PluginInfo; +import org.apache.solr.core.SolrCore; +import org.apache.solr.handler.RequestHandlerBase; +import org.apache.solr.metrics.MetricsMap; +import org.apache.solr.metrics.SolrMetrics; +import org.apache.solr.request.SolrQueryRequest; +import org.apache.solr.request.SolrRequestHandler; import org.apache.solr.response.SolrQueryResponse; import org.junit.AfterClass; import org.junit.BeforeClass; @@ -53,12 +63,12 @@ public class MetricsHandlerTest extends SolrTestCaseJ4 { @AfterClass public static void cleanupMetrics() throws Exception { if (null != h) { - h.getCoreContainer().getMetricManager().registry("solr.jvm" ).remove("solrtest_foo"); + h.getCoreContainer().getMetricManager().registry("solr.jvm").remove("solrtest_foo"); h.getCoreContainer().getMetricManager().registry("solr.jetty").remove("solrtest_foo"); h.getCoreContainer().getMetricManager().registry("solr.jetty").remove("solrtest_foo:bar"); } } - + @Test public void test() throws Exception { MetricsHandler handler = new MetricsHandler(h.getCoreContainer()); @@ -145,7 +155,7 @@ public class MetricsHandlerTest extends SolrTestCaseJ4 { assertNotNull(values.get("metrics")); values = (NamedList) values.get("metrics"); assertEquals(1, values.size()); - assertEquals(13, ((NamedList)values.get("solr.node")).size()); + assertEquals(13, ((NamedList) values.get("solr.node")).size()); assertNotNull(values.get("solr.node")); values = (NamedList) values.get("solr.node"); assertNotNull(values.get("CONTAINER.cores.lazy")); // this is a gauge node @@ -171,7 +181,7 @@ public class MetricsHandlerTest extends SolrTestCaseJ4 { assertNotNull(values.get("solr.core.collection1")); values = (NamedList) values.get("solr.core.collection1"); assertEquals(1, values.size()); - Map m = (Map)values.get("CACHE.core.fieldCache"); + Map m = (Map) values.get("CACHE.core.fieldCache"); assertNotNull(m); assertNotNull(m.get("entries_count")); @@ -223,7 +233,7 @@ public class MetricsHandlerTest extends SolrTestCaseJ4 { assertTrue(nl.size() > 0); nl.forEach((k, v) -> { assertTrue(v instanceof Map); - Map map = (Map)v; + Map map = (Map) v; assertTrue(map.size() > 2); }); @@ -238,7 +248,7 @@ public class MetricsHandlerTest extends SolrTestCaseJ4 { assertTrue(nl.size() > 0); nl.forEach((k, v) -> { assertTrue(v instanceof Map); - Map map = (Map)v; + Map map = (Map) v; assertEquals(2, map.size()); assertNotNull(map.get("inserts")); assertNotNull(map.get("size")); @@ -257,7 +267,7 @@ public class MetricsHandlerTest extends SolrTestCaseJ4 { Object val = values.findRecursive("metrics", key1); assertNotNull(val); assertTrue(val instanceof Map); - assertTrue(((Map)val).size() >= 2); + assertTrue(((Map) val).size() >= 2); String key2 = "solr.core.collection1:CACHE.core.fieldCache:entries_count"; resp = new SolrQueryResponse(); @@ -276,7 +286,7 @@ public class MetricsHandlerTest extends SolrTestCaseJ4 { val = values.findRecursive("metrics", key3); assertNotNull(val); assertTrue(val instanceof Number); - assertEquals(3, ((Number)val).intValue()); + assertEquals(3, ((Number) val).intValue()); // test multiple keys resp = new SolrQueryResponse(); @@ -306,7 +316,7 @@ public class MetricsHandlerTest extends SolrTestCaseJ4 { handler.handleRequestBody(req(CommonParams.QT, "/admin/metrics", CommonParams.WT, "json", MetricsHandler.KEY_PARAM, "foo", MetricsHandler.KEY_PARAM, "foo:bar:baz:xyz"), resp); values = resp.getValues(); - NamedList metrics = (NamedList)values.get("metrics"); + NamedList metrics = (NamedList) values.get("metrics"); assertEquals(0, metrics.size()); assertNotNull(values.findRecursive("errors", "foo")); assertNotNull(values.findRecursive("errors", "foo:bar:baz:xyz")); @@ -316,7 +326,7 @@ public class MetricsHandlerTest extends SolrTestCaseJ4 { handler.handleRequestBody(req(CommonParams.QT, "/admin/metrics", CommonParams.WT, "json", MetricsHandler.KEY_PARAM, "foo:bar:baz"), resp); values = resp.getValues(); - metrics = (NamedList)values.get("metrics"); + metrics = (NamedList) values.get("metrics"); assertEquals(0, metrics.size()); assertNotNull(values.findRecursive("errors", "foo:bar:baz")); @@ -325,8 +335,122 @@ public class MetricsHandlerTest extends SolrTestCaseJ4 { handler.handleRequestBody(req(CommonParams.QT, "/admin/metrics", CommonParams.WT, "json", MetricsHandler.KEY_PARAM, "solr.jetty:unknown:baz"), resp); values = resp.getValues(); - metrics = (NamedList)values.get("metrics"); + metrics = (NamedList) values.get("metrics"); assertEquals(0, metrics.size()); assertNotNull(values.findRecursive("errors", "solr.jetty:unknown:baz")); } + + @Test + public void testMetricsUnload() throws Exception { + + SolrCore core = h.getCoreContainer().getCore("collection1");//;.getRequestHandlers().put("/dumphandler", new DumpRequestHandler()); + RefreshablePluginHolder pluginHolder =null; + try { + PluginInfo info = new PluginInfo(SolrRequestHandler.TYPE, Utils.makeMap("name", "/dumphandler", "class", DumpRequestHandler.class.getName())); + DumpRequestHandler requestHandler = new DumpRequestHandler(); + requestHandler.gaugevals = Utils.makeMap("d_k1","v1", "d_k2","v2"); + pluginHolder = new RefreshablePluginHolder(info, requestHandler); + core.getRequestHandlers().put("/dumphandler", + + pluginHolder); + } finally { + core.close(); + } + + + + MetricsHandler handler = new MetricsHandler(h.getCoreContainer()); + + SolrQueryResponse resp = new SolrQueryResponse(); + handler.handleRequestBody(req(CommonParams.QT, "/admin/metrics", CommonParams.WT, "json", MetricsHandler.COMPACT_PARAM, "true", "key", "solr.core.collection1:QUERY./dumphandler.dumphandlergauge"), + resp); + + assertEquals("v1", resp.getValues()._getStr(Arrays.asList("metrics", "solr.core.collection1:QUERY./dumphandler.dumphandlergauge","d_k1"), null)); + assertEquals("v2", resp.getValues()._getStr(Arrays.asList("metrics","solr.core.collection1:QUERY./dumphandler.dumphandlergauge","d_k2"), null)); + pluginHolder.closeHandler(); + resp = new SolrQueryResponse(); + handler.handleRequestBody(req(CommonParams.QT, "/admin/metrics", CommonParams.WT, "json", MetricsHandler.COMPACT_PARAM, "true", "key", "solr.core.collection1:QUERY./dumphandler.dumphandlergauge"), + resp); + + assertEquals(null, resp.getValues()._getStr(Arrays.asList("metrics", "solr.core.collection1:QUERY./dumphandler.dumphandlergauge","d_k1"), null)); + assertEquals(null, resp.getValues()._getStr(Arrays.asList("metrics","solr.core.collection1:QUERY./dumphandler.dumphandlergauge","d_k2"), null)); + + DumpRequestHandler requestHandler = new DumpRequestHandler(); + requestHandler.gaugevals = Utils.makeMap("d_k1","v1.1", "d_k2","v2.1"); + pluginHolder.reset(requestHandler); + resp = new SolrQueryResponse(); + handler.handleRequestBody(req(CommonParams.QT, "/admin/metrics", CommonParams.WT, "json", MetricsHandler.COMPACT_PARAM, "true", "key", "solr.core.collection1:QUERY./dumphandler.dumphandlergauge"), + resp); + + assertEquals("v1.1", resp.getValues()._getStr(Arrays.asList("metrics", "solr.core.collection1:QUERY./dumphandler.dumphandlergauge","d_k1"), null)); + assertEquals("v2.1", resp.getValues()._getStr(Arrays.asList("metrics","solr.core.collection1:QUERY./dumphandler.dumphandlergauge","d_k2"), null)); + + + } + + static class RefreshablePluginHolder extends PluginBag.PluginHolder { + + private DumpRequestHandler rh; + private SolrMetrics metricsInfo; + + public RefreshablePluginHolder(PluginInfo info, DumpRequestHandler rh) { + super(info); + this.rh = rh; + } + + @Override + public boolean isLoaded() { + return true; + } + + void closeHandler() throws Exception { + this.metricsInfo = rh.getMetrics(); + if(metricsInfo.tag.contains(String.valueOf(rh.hashCode()))){ + //this created a new child metrics + metricsInfo = metricsInfo.getParent(); + } + this.rh.close(); + } + + void reset(DumpRequestHandler rh) throws Exception { + this.rh = rh; + if(metricsInfo != null) + this.rh.initializeMetrics(metricsInfo); + } + + + @Override + public SolrRequestHandler get() { + return rh; + } + } + + public static class DumpRequestHandler extends RequestHandlerBase { + + static String key = DumpRequestHandler.class.getName(); + Map gaugevals ; + @Override + public void handleRequestBody(SolrQueryRequest req, SolrQueryResponse rsp) throws Exception { + rsp.add("key", key); + } + + @Override + public String getDescription() { + return "DO nothing"; + } + + @Override + public void initializeMetrics(SolrMetrics m) { + super.initializeMetrics(m); + MetricsMap metrics = new MetricsMap((detailed, map) -> map.putAll(gaugevals)); + solrMetrics.gauge(this, + metrics, true, "dumphandlergauge", getCategory().toString()); + + } + + @Override + public Boolean registerV2() { + return Boolean.FALSE; + } + } } diff --git a/solr/solrj/src/java/org/apache/solr/common/util/ExecutorUtil.java b/solr/solrj/src/java/org/apache/solr/common/util/ExecutorUtil.java index a053a180109..e5bad27ceb5 100644 --- a/solr/solrj/src/java/org/apache/solr/common/util/ExecutorUtil.java +++ b/solr/solrj/src/java/org/apache/solr/common/util/ExecutorUtil.java @@ -72,6 +72,7 @@ public class ExecutorUtil { } public static void shutdownAndAwaitTermination(ExecutorService pool) { + if(pool == null) return; pool.shutdown(); // Disable new tasks from being submitted awaitTermination(pool); }