You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by ab...@apache.org on 2019/10/15 10:52:05 UTC
[lucene-solr] 02/03: SOLR-13677: Clean up some issues from review.
This is an automated email from the ASF dual-hosted git repository.
ab pushed a commit to branch jira/solr-13677-final
in repository https://gitbox.apache.org/repos/asf/lucene-solr.git
commit dccbbc6051ec05b2f1e5d6ffbc8d46dcff562363
Author: Andrzej Bialecki <ab...@apache.org>
AuthorDate: Tue Oct 15 11:37:33 2019 +0200
SOLR-13677: Clean up some issues from review.
---
.../solr/handler/dataimport/DataImportHandler.java | 6 +--
.../src/java/org/apache/solr/core/PluginBag.java | 17 +++++--
.../apache/solr/handler/ReplicationHandler.java | 24 +++++-----
.../apache/solr/handler/RequestHandlerBase.java | 32 ++++++-------
.../solr/handler/admin/CoreAdminHandler.java | 8 ++--
.../solr/handler/component/SuggestComponent.java | 18 ++++----
.../org/apache/solr/metrics/SolrMetricManager.java | 54 +++++-----------------
.../apache/solr/metrics/SolrMetricProducer.java | 26 +++++++----
.../{SolrMetrics.java => SolrMetricsContext.java} | 23 ++++-----
.../java/org/apache/solr/search/CaffeineCache.java | 3 +-
.../java/org/apache/solr/search/FastLRUCache.java | 13 +++---
.../src/java/org/apache/solr/search/LFUCache.java | 20 ++++----
.../src/java/org/apache/solr/search/LRUCache.java | 21 ++++-----
.../src/java/org/apache/solr/search/SolrCache.java | 4 +-
.../org/apache/solr/search/SolrCacheHolder.java | 6 +--
.../apache/solr/security/AuthenticationPlugin.java | 8 ++--
.../solr/handler/admin/MetricsHandlerTest.java | 19 ++++----
17 files changed, 140 insertions(+), 162 deletions(-)
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 80d374e..3d5c4d5 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.SolrMetrics;
+import org.apache.solr.metrics.SolrMetricsContext;
import org.apache.solr.request.SolrQueryRequest;
import org.apache.solr.response.RawResponseWriter;
import org.apache.solr.response.SolrQueryResponse;
@@ -275,7 +275,7 @@ public class DataImportHandler extends RequestHandlerBase implements
}
@Override
- public void initializeMetrics(SolrMetrics m) {
+ public void initializeMetrics(SolrMetricsContext m) {
super.initializeMetrics(m);
metrics = new MetricsMap((detailed, map) -> {
if (importer != null) {
@@ -299,7 +299,7 @@ public class DataImportHandler extends RequestHandlerBase implements
map.put(DataImporter.MSG.TOTAL_DOCS_SKIPPED, cumulative.skipDocCount);
}
});
- solrMetrics.gauge(this, metrics, true, "importer", getCategory().toString());
+ solrMetricsContext.gauge(this, metrics, true, "importer", getCategory().toString());
}
// //////////////////////SolrInfoMBeans methods //////////////////////
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 a05f18e..fa2c3e3 100644
--- a/solr/core/src/java/org/apache/solr/core/PluginBag.java
+++ b/solr/core/src/java/org/apache/solr/core/PluginBag.java
@@ -192,7 +192,6 @@ public class PluginBag<T> implements AutoCloseable {
PluginHolder<T> pluginHolder = new PluginHolder<>(null, plugin);
pluginHolder.registerAPI = false;
PluginHolder<T> old = put(name, pluginHolder);
- if(old != null) closeQuietly(old);
return old == null ? null : old.get();
}
@@ -232,11 +231,15 @@ public class PluginBag<T> implements AutoCloseable {
apiBag.registerLazy((PluginHolder<SolrRequestHandler>) plugin, plugin.pluginInfo);
}
}
- if(disableHandler == null) disableHandler = Boolean.FALSE;
+ if (disableHandler == null) disableHandler = Boolean.FALSE;
PluginHolder<T> old = null;
- if(!disableHandler) old = registry.put(name, plugin);
+ if (!disableHandler) old = registry.put(name, plugin);
if (plugin.pluginInfo != null && plugin.pluginInfo.isDefault()) setDefault(name);
if (plugin.isLoaded()) registerMBean(plugin.get(), core, name);
+ // old instance has been replaced - close it to prevent mem leaks
+ if (old != null && old != plugin) {
+ closeQuietly(old);
+ }
return old;
}
@@ -325,6 +328,14 @@ public class PluginBag<T> implements AutoCloseable {
}
}
+ public static void closeQuietly(Object inst) {
+ try {
+ if (inst != null && inst instanceof AutoCloseable) ((AutoCloseable) inst).close();
+ } catch (Exception e) {
+ log.error("Error closing "+ inst , e);
+ }
+ }
+
/**
* An indirect reference to a plugin. It just wraps a plugin instance.
* subclasses may choose to lazily load the plugin
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 76ecbec..d78490a 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.SolrMetrics;
+import org.apache.solr.metrics.SolrMetricsContext;
import org.apache.solr.request.SolrQueryRequest;
import org.apache.solr.response.SolrQueryResponse;
import org.apache.solr.search.SolrIndexSearcher;
@@ -863,19 +863,19 @@ public class ReplicationHandler extends RequestHandlerBase implements SolrCoreAw
}
@Override
- public void initializeMetrics(SolrMetrics m) {
+ public void initializeMetrics(SolrMetricsContext m) {
super.initializeMetrics(m);
- solrMetrics.gauge(this, () -> (core != null && !core.isClosed() ? NumberUtils.readableSize(core.getIndexSize()) : ""),
+ solrMetricsContext.gauge(this, () -> (core != null && !core.isClosed() ? NumberUtils.readableSize(core.getIndexSize()) : ""),
true, "indexSize", getCategory().toString());
- solrMetrics.gauge(this, () -> (core != null && !core.isClosed() ? getIndexVersion().toString() : ""),
+ solrMetricsContext.gauge(this, () -> (core != null && !core.isClosed() ? getIndexVersion().toString() : ""),
true, "indexVersion", getCategory().toString());
- solrMetrics.gauge(this, () -> (core != null && !core.isClosed() ? getIndexVersion().generation : 0),
+ solrMetricsContext.gauge(this, () -> (core != null && !core.isClosed() ? getIndexVersion().generation : 0),
true, GENERATION, getCategory().toString());
- solrMetrics.gauge(this, () -> (core != null && !core.isClosed() ? core.getIndexDir() : ""),
+ solrMetricsContext.gauge(this, () -> (core != null && !core.isClosed() ? core.getIndexDir() : ""),
true, "indexPath", getCategory().toString());
- solrMetrics.gauge(this, () -> isMaster,
+ solrMetricsContext.gauge(this, () -> isMaster,
true, "isMaster", getCategory().toString());
- solrMetrics.gauge(this, () -> isSlave,
+ solrMetricsContext.gauge(this, () -> isSlave,
true, "isSlave", getCategory().toString());
final MetricsMap fetcherMap = new MetricsMap((detailed, map) -> {
IndexFetcher fetcher = currentIndexFetcher;
@@ -905,12 +905,12 @@ public class ReplicationHandler extends RequestHandlerBase implements SolrCoreAw
addVal(map, IndexFetcher.CONF_FILES_REPLICATED, props, String.class);
}
});
- solrMetrics.gauge(this , fetcherMap, true, "fetcher", getCategory().toString());
- solrMetrics.gauge(this, () -> isMaster && includeConfFiles != null ? includeConfFiles : "",
+ solrMetricsContext.gauge(this , fetcherMap, true, "fetcher", getCategory().toString());
+ solrMetricsContext.gauge(this, () -> isMaster && includeConfFiles != null ? includeConfFiles : "",
true, "confFilesToReplicate", getCategory().toString());
- solrMetrics.gauge(this, () -> isMaster ? getReplicateAfterStrings() : Collections.<String>emptyList(),
+ solrMetricsContext.gauge(this, () -> isMaster ? getReplicateAfterStrings() : Collections.<String>emptyList(),
true, REPLICATE_AFTER, getCategory().toString());
- solrMetrics.gauge(this, () -> isMaster && replicationEnabled.get(),
+ solrMetricsContext.gauge(this, () -> isMaster && replicationEnabled.get(),
true, "replicationEnabled", getCategory().toString());
}
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 499db95..9ea64b9 100644
--- a/solr/core/src/java/org/apache/solr/handler/RequestHandlerBase.java
+++ b/solr/core/src/java/org/apache/solr/handler/RequestHandlerBase.java
@@ -40,7 +40,7 @@ import org.apache.solr.core.PluginInfo;
import org.apache.solr.core.SolrInfoBean;
import org.apache.solr.metrics.MetricsMap;
import org.apache.solr.metrics.SolrMetricProducer;
-import org.apache.solr.metrics.SolrMetrics;
+import org.apache.solr.metrics.SolrMetricsContext;
import org.apache.solr.request.SolrQueryRequest;
import org.apache.solr.request.SolrRequestHandler;
import org.apache.solr.response.SolrQueryResponse;
@@ -140,27 +140,27 @@ public abstract class RequestHandlerBase implements SolrRequestHandler, SolrInfo
}
- protected SolrMetrics solrMetrics;
+ protected SolrMetricsContext solrMetricsContext;
@Override
- public SolrMetrics getMetrics() {
- return solrMetrics;
+ public SolrMetricsContext getSolrMetricsContext() {
+ return solrMetricsContext;
}
@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());
+ public void initializeMetrics(SolrMetricsContext m) {
+ this.solrMetricsContext = m.getChildInfo(this);
+ numErrors = solrMetricsContext.meter(this, "errors", getCategory().toString());
+ numServerErrors = solrMetricsContext.meter(this, "serverErrors", getCategory().toString());
+ numClientErrors = solrMetricsContext.meter(this, "clientErrors", getCategory().toString());
+ numTimeouts = solrMetricsContext.meter(this, "timeouts", getCategory().toString());
+ requests = solrMetricsContext.counter(this, "requests", getCategory().toString());
MetricsMap metricsMap = new MetricsMap((detail, map) ->
shardPurposes.forEach((k, v) -> map.put(k, v.getCount())));
- 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());
+ solrMetricsContext.gauge(this, metricsMap, true, "shardRequests", getCategory().toString());
+ requestTimes = solrMetricsContext.timer(this,"requestTimes", getCategory().toString());
+ totalTime = solrMetricsContext.counter(this, "totalTime", getCategory().toString());
+ solrMetricsContext.gauge(this, () -> handlerStart, true, "handlerStart", getCategory().toString());
}
public static SolrParams getSolrParamsFromNamedList(NamedList args, String key) {
@@ -276,7 +276,7 @@ public abstract class RequestHandlerBase implements SolrRequestHandler, SolrInfo
@Override
public MetricRegistry getMetricRegistry() {
- return solrMetrics.getRegistry();
+ return solrMetricsContext != null ? solrMetricsContext.getRegistry() : null;
}
@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 ae22555..15bf4e4 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,7 +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.metrics.SolrMetricsContext;
import org.apache.solr.request.SolrQueryRequest;
import org.apache.solr.response.SolrQueryResponse;
import org.apache.solr.security.AuthorizationContext;
@@ -121,10 +121,10 @@ public class CoreAdminHandler extends RequestHandlerBase implements PermissionNa
}
@Override
- public void initializeMetrics(SolrMetrics m) {
+ public void initializeMetrics(SolrMetricsContext m) {
super.initializeMetrics(m);
- parallelExecutor = MetricUtils.instrumentedExecutorService(parallelExecutor, this, solrMetrics.getRegistry(),
- SolrMetricManager.mkName("parallelCoreAdminExecutor", getCategory().name(), solrMetrics.scope, "threadPool"));
+ parallelExecutor = MetricUtils.instrumentedExecutorService(parallelExecutor, this, solrMetricsContext.getRegistry(),
+ SolrMetricManager.mkName("parallelCoreAdminExecutor", getCategory().name(), solrMetricsContext.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 00d8697..ca02867 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
@@ -49,7 +49,7 @@ import org.apache.solr.core.SolrCore;
import org.apache.solr.core.SolrEventListener;
import org.apache.solr.metrics.MetricsMap;
import org.apache.solr.metrics.SolrMetricProducer;
-import org.apache.solr.metrics.SolrMetrics;
+import org.apache.solr.metrics.SolrMetricsContext;
import org.apache.solr.search.SolrIndexSearcher;
import org.apache.solr.spelling.suggest.SolrSuggester;
import org.apache.solr.spelling.suggest.SuggesterOptions;
@@ -88,6 +88,8 @@ public class SuggestComponent extends SearchComponent implements SolrCoreAware,
@SuppressWarnings("unchecked")
protected NamedList initParams;
+ protected SolrMetricsContext metricsContext;
+
/**
* Key is the dictionary name used in SolrConfig, value is the corresponding {@link SolrSuggester}
*/
@@ -347,25 +349,23 @@ public class SuggestComponent extends SearchComponent implements SolrCoreAware,
return "Suggester component";
}
- protected SolrMetrics metricsInfo;
-
@Override
- public SolrMetrics getMetrics() {
- return metricsInfo;
+ public SolrMetricsContext getSolrMetricsContext() {
+ return metricsContext;
}
@Override
- public void initializeMetrics(SolrMetrics info) {
- this.metricsInfo = info.getChildInfo(this);
+ public void initializeMetrics(SolrMetricsContext metricsContext) {
+ this.metricsContext = metricsContext.getChildInfo(this);
- metricsInfo.metricManager.registerGauge(this, info.registry, () -> ramBytesUsed(), metricsInfo.tag, true, "totalSizeInBytes", getCategory().toString(), metricsInfo.scope);
+ this.metricsContext.gauge(this, () -> ramBytesUsed(), true, "totalSizeInBytes", getCategory().toString());
MetricsMap suggestersMap = new MetricsMap((detailed, map) -> {
for (Map.Entry<String, SolrSuggester> entry : suggesters.entrySet()) {
SolrSuggester suggester = entry.getValue();
map.put(entry.getKey(), suggester.toString());
}
});
- metricsInfo.metricManager.registerGauge(this, metricsInfo.registry, suggestersMap, metricsInfo.tag, true, "suggesters", getCategory().toString(), metricsInfo.scope);
+ this.metricsContext.gauge(this, suggestersMap, true, "suggesters", getCategory().toString());
}
@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 187598d..201dc08 100644
--- a/solr/core/src/java/org/apache/solr/metrics/SolrMetricManager.java
+++ b/solr/core/src/java/org/apache/solr/metrics/SolrMetricManager.java
@@ -607,14 +607,10 @@ 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(metricsPath);
+ info.registerMetricName(name);
}
- return registry.meter(metricsPath, meterSupplier);
+ return registry(registry).meter(name, meterSupplier);
}
/**
@@ -634,14 +630,6 @@ 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}
*
@@ -653,15 +641,10 @@ 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.counter(name, counterSupplier);
-
+ return registry(registry).counter(name, counterSupplier);
}
/**
@@ -707,27 +690,13 @@ public class SolrMetricManager {
}
}
- 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)}.
- */
+ /**
+ * 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<T> implements Gauge<T> {
private final Gauge<T> gauge;
private final String tag;
@@ -765,7 +734,7 @@ public class SolrMetricManager {
registry.removeMatching((name, metric) -> {
if (metric instanceof GaugeWrapper) {
GaugeWrapper wrapper = (GaugeWrapper) metric;
- boolean toRemove = tagSegment.equals(wrapper.getTag()) || wrapper.getTag().contains(tagSegment);
+ boolean toRemove = wrapper.getTag().contains(tagSegment);
if (toRemove) removed.incrementAndGet();
return toRemove;
}
@@ -813,7 +782,6 @@ 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 0069734..369cff6 100644
--- a/solr/core/src/java/org/apache/solr/metrics/SolrMetricProducer.java
+++ b/solr/core/src/java/org/apache/solr/metrics/SolrMetricProducer.java
@@ -22,14 +22,17 @@ package org.apache.solr.metrics;
public interface SolrMetricProducer extends AutoCloseable {
/**
- * Unique metric name is in the format of A.B.C
+ * Unique metric tag identifies components with the same life-cycle, which should
+ * be registered / unregistered together. It is in the format of A:B:C, where
* 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.
+ * If object "B" is unregistered C also must get unregistered.
+ * If object "A" is unregistered B and 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;
+ if (parentName != null && parentName.contains(name)) {
+ throw new RuntimeException("Parent already includes this component? parent=" + parentName + ", this=" + name);
+ }
return parentName == null ?
name :
parentName + ":" + name;
@@ -45,25 +48,28 @@ public interface SolrMetricProducer extends AutoCloseable {
* 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 scope scope of the metrics (eg. handler name) to separate metrics of components with
+ * the same implementation but different scope
+ * @deprecated use {@link #initializeMetrics(SolrMetricsContext)} instead
*/
default void initializeMetrics(SolrMetricManager manager, String registry, String tag, String scope) {
- initializeMetrics(new SolrMetrics(manager, registry, tag, scope));
+ initializeMetrics(new SolrMetricsContext(manager, registry, tag, scope));
}
- default void initializeMetrics(SolrMetrics info) {
- throw new RuntimeException("This means , the class has not implemented both of these methods");
+ default void initializeMetrics(SolrMetricsContext context) {
+ throw new RuntimeException("You must implement either initializeMetrics(SolrMetricsContext) or " +
+ "initializeMetrics(SolrMetricManager, String, String, String)");
}
- default SolrMetrics getMetrics() {
+ default SolrMetricsContext getSolrMetricsContext() {
return null;
}
@Override
default void close() throws Exception {
- SolrMetrics info = getMetrics();
+ SolrMetricsContext info = getSolrMetricsContext();
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/SolrMetricsContext.java
similarity index 73%
rename from solr/core/src/java/org/apache/solr/metrics/SolrMetrics.java
rename to solr/core/src/java/org/apache/solr/metrics/SolrMetricsContext.java
index 28aea4e..3cb2554 100644
--- a/solr/core/src/java/org/apache/solr/metrics/SolrMetrics.java
+++ b/solr/core/src/java/org/apache/solr/metrics/SolrMetricsContext.java
@@ -29,14 +29,13 @@ import org.apache.solr.core.SolrInfoBean;
import static org.apache.solr.metrics.SolrMetricManager.makeName;
-public class SolrMetrics {
+public class SolrMetricsContext {
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) {
+ public SolrMetricsContext(SolrMetricManager metricManager, String registry, String tag, String scope) {
this.registry = registry;
this.metricManager = metricManager;
this.tag = tag;
@@ -51,16 +50,16 @@ public class SolrMetrics {
metricManager.unregisterGauges(registry, tag);
}
- public SolrMetrics getChildInfo(SolrMetricProducer producer) {
- SolrMetrics metricsInfo = new SolrMetrics(metricManager, registry, producer.getUniqueMetricTag(tag), scope);
- metricsInfo.parent = this;
+ public SolrMetricsContext getChildInfo(SolrMetricProducer producer) {
+ SolrMetricsContext metricsInfo = new SolrMetricsContext(metricManager, registry, producer.getUniqueMetricTag(tag), scope);
return metricsInfo;
}
public Meter meter(SolrInfoBean info, String metricName, String... metricpath) {
- return metricManager.meter(info, getRegistry(), createName(metricName, metricpath));
+ return metricManager.meter(info, registry, createName(metricName, metricpath));
}
+ // adds the scope
private String createName(String metricName, String... metricpath) {
ArrayList<String> l = new ArrayList<>();
if(metricpath != null ) {
@@ -71,23 +70,19 @@ public class SolrMetrics {
}
public Counter counter(SolrInfoBean info, String metricName, String... metricpath) {
- return metricManager.counter(info, getRegistry(), createName(metricName, metricpath));
+ return metricManager.counter(info, registry, 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));
+ metricManager.registerGauge(info, registry, gauge, tag, force, createName(metricName, metricpath));
}
public Timer timer(SolrInfoBean info, String metricName, String... metricpath) {
- return metricManager.timer(info, getRegistry(), createName(metricName, metricpath));
+ return metricManager.timer(info, registry, 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/CaffeineCache.java b/solr/core/src/java/org/apache/solr/search/CaffeineCache.java
index 71eb86f..8e66eb8 100644
--- a/solr/core/src/java/org/apache/solr/search/CaffeineCache.java
+++ b/solr/core/src/java/org/apache/solr/search/CaffeineCache.java
@@ -202,7 +202,8 @@ public class CaffeineCache<K, V> extends SolrCacheBase implements SolrCache<K, V
}
@Override
- public void close() {
+ public void close() throws Exception {
+ SolrCache.super.close();
cache.invalidateAll();
cache.cleanUp();
if (executor instanceof ExecutorService) {
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 af802e2..414389b 100644
--- a/solr/core/src/java/org/apache/solr/search/FastLRUCache.java
+++ b/solr/core/src/java/org/apache/solr/search/FastLRUCache.java
@@ -29,7 +29,8 @@ 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.SolrMetrics;
+import org.apache.solr.metrics.SolrMetricProducer;
+import org.apache.solr.metrics.SolrMetricsContext;
import org.apache.solr.util.ConcurrentLRUCache;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -284,8 +285,8 @@ public class FastLRUCache<K, V> extends SolrCacheBase implements SolrCache<K, V>
@Override
- public void close() {
- if (metricsInfo != null) metricsInfo.unregister();
+ public void close() throws Exception {
+ SolrCache.super.close();
// add the stats to the cumulative stats object (the first in the statsList)
statsList.get(0).add(cache.getStats());
statsList.remove(cache.getStats());
@@ -309,15 +310,15 @@ public class FastLRUCache<K, V> extends SolrCacheBase implements SolrCache<K, V>
}
- SolrMetrics metricsInfo;
+ SolrMetricsContext metricsInfo;
@Override
- public SolrMetrics getMetrics() {
+ public SolrMetricsContext getSolrMetricsContext() {
return metricsInfo;
}
@Override
- public void initializeMetrics(SolrMetrics metricsInfo) {
+ public void initializeMetrics(SolrMetricsContext metricsInfo) {
this.metricsInfo = metricsInfo;
metricsInfo.metricManager.registerGauge(this, metricsInfo.registry, cacheMap, metricsInfo.tag, true, metricsInfo.scope, getCategory().toString());
}
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 a309d73..13e02e4 100644
--- a/solr/core/src/java/org/apache/solr/search/LFUCache.java
+++ b/solr/core/src/java/org/apache/solr/search/LFUCache.java
@@ -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.SolrMetrics;
+import org.apache.solr.metrics.SolrMetricsContext;
import org.apache.solr.util.ConcurrentLFUCache;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -229,12 +229,12 @@ public class LFUCache<K, V> implements SolrCache<K, V>, Accountable {
@Override
- public void close() {
+ public void close() throws Exception {
+ SolrCache.super.close();
// add the stats to the cumulative stats object (the first in the statsList)
statsList.get(0).add(cache.getStats());
statsList.remove(cache.getStats());
cache.destroy();
- if (solrMetrics != null) solrMetrics.unregister();
}
//////////////////////// SolrInfoMBeans methods //////////////////////
@@ -263,16 +263,16 @@ public class LFUCache<K, V> implements SolrCache<K, V>, Accountable {
}
- private SolrMetrics solrMetrics;
+ private SolrMetricsContext solrMetricsContext;
@Override
- public SolrMetrics getMetrics() {
- return solrMetrics;
+ public SolrMetricsContext getSolrMetricsContext() {
+ return solrMetricsContext;
}
@Override
- public void initializeMetrics(SolrMetrics info) {
- solrMetrics = info.getChildInfo(this);
+ public void initializeMetrics(SolrMetricsContext info) {
+ solrMetricsContext = info.getChildInfo(this);
cacheMap = new MetricsMap((detailed, map) -> {
if (cache != null) {
ConcurrentLFUCache.Stats stats = cache.getStats();
@@ -338,7 +338,7 @@ public class LFUCache<K, V> implements SolrCache<K, V>, Accountable {
}
});
- solrMetrics.metricManager.registerGauge(this, solrMetrics.registry, cacheMap, solrMetrics.getTag(), true, solrMetrics.scope, getCategory().toString());
+ solrMetricsContext.metricManager.registerGauge(this, solrMetricsContext.registry, cacheMap, solrMetricsContext.getTag(), true, solrMetricsContext.scope, getCategory().toString());
}
// for unit tests only
@@ -353,7 +353,7 @@ public class LFUCache<K, V> implements SolrCache<K, V>, Accountable {
@Override
public MetricRegistry getMetricRegistry() {
- return solrMetrics == null ? null : solrMetrics.getRegistry();
+ return solrMetricsContext == null ? null : solrMetricsContext.getRegistry();
}
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 ae3e81c..c8c5cdd 100644
--- a/solr/core/src/java/org/apache/solr/search/LRUCache.java
+++ b/solr/core/src/java/org/apache/solr/search/LRUCache.java
@@ -33,7 +33,7 @@ import org.apache.lucene.util.RamUsageEstimator;
import org.apache.solr.common.SolrException;
import org.apache.solr.common.util.TimeSource;
import org.apache.solr.metrics.MetricsMap;
-import org.apache.solr.metrics.SolrMetrics;
+import org.apache.solr.metrics.SolrMetricsContext;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -377,11 +377,6 @@ public class LRUCache<K,V> extends SolrCacheBase implements SolrCache<K,V>, Acco
warmupTime = TimeUnit.MILLISECONDS.convert(System.nanoTime() - warmingStartTime, TimeUnit.NANOSECONDS);
}
- @Override
- public void close() {
- if(solrMetrics != null) solrMetrics.unregister();
- }
-
//////////////////////// SolrInfoMBeans methods //////////////////////
@@ -400,16 +395,16 @@ public class LRUCache<K,V> extends SolrCacheBase implements SolrCache<K,V>, Acco
return metricNames;
}
- SolrMetrics solrMetrics;
+ SolrMetricsContext solrMetricsContext;
@Override
- public SolrMetrics getMetrics() {
- return solrMetrics;
+ public SolrMetricsContext getSolrMetricsContext() {
+ return solrMetricsContext;
}
@Override
- public void initializeMetrics(SolrMetrics m) {
- solrMetrics = m.getChildInfo(this);
+ public void initializeMetrics(SolrMetricsContext m) {
+ solrMetricsContext = m.getChildInfo(this);
cacheMap = new MetricsMap((detailed, res) -> {
synchronized (map) {
res.put(LOOKUPS_PARAM, lookups);
@@ -438,7 +433,7 @@ public class LRUCache<K,V> extends SolrCacheBase implements SolrCache<K,V>, Acco
res.put("cumulative_evictionsRamUsage", stats.evictionsRamUsage.longValue());
res.put("cumulative_evictionsIdleTime", stats.evictionsIdleTime.longValue());
});
- solrMetrics.metricManager.registerGauge(this, solrMetrics.registry, cacheMap, solrMetrics.tag, true, solrMetrics.scope, getCategory().toString());
+ solrMetricsContext.metricManager.registerGauge(this, solrMetricsContext.registry, cacheMap, solrMetricsContext.tag, true, solrMetricsContext.scope, getCategory().toString());
}
// for unit tests only
@@ -448,7 +443,7 @@ public class LRUCache<K,V> extends SolrCacheBase implements SolrCache<K,V>, Acco
@Override
public MetricRegistry getMetricRegistry() {
- return solrMetrics ==null ?null: solrMetrics.getRegistry();
+ return solrMetricsContext ==null ?null: solrMetricsContext.getRegistry();
}
@Override
diff --git a/solr/core/src/java/org/apache/solr/search/SolrCache.java b/solr/core/src/java/org/apache/solr/search/SolrCache.java
index 4a16b39..55f57ec 100644
--- a/solr/core/src/java/org/apache/solr/search/SolrCache.java
+++ b/solr/core/src/java/org/apache/solr/search/SolrCache.java
@@ -137,7 +137,9 @@ public interface SolrCache<K,V> extends SolrInfoBean, SolrMetricProducer {
/** Frees any non-memory resources */
- public void close();
+ default void close() throws Exception {
+ SolrMetricProducer.super.close();
+ }
/** Returns maximum size limit (number of items) if set and supported, -1 otherwise. */
int getMaxSize();
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 58a43eb..8ac1b10 100644
--- a/solr/core/src/java/org/apache/solr/search/SolrCacheHolder.java
+++ b/solr/core/src/java/org/apache/solr/search/SolrCacheHolder.java
@@ -22,9 +22,7 @@ import java.util.Map;
import java.util.Set;
import com.codahale.metrics.MetricRegistry;
-import org.apache.solr.common.util.Utils;
-import org.apache.solr.metrics.SolrMetricManager;
-import org.apache.solr.metrics.SolrMetrics;
+import org.apache.solr.metrics.SolrMetricsContext;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -143,7 +141,7 @@ public class SolrCacheHolder<K, V> implements SolrCache<K,V> {
}
@Override
- public void initializeMetrics(SolrMetrics info) {
+ public void initializeMetrics(SolrMetricsContext 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 8044af9..cf30a2e 100644
--- a/solr/core/src/java/org/apache/solr/security/AuthenticationPlugin.java
+++ b/solr/core/src/java/org/apache/solr/security/AuthenticationPlugin.java
@@ -32,7 +32,7 @@ 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.apache.solr.metrics.SolrMetricsContext;
import org.eclipse.jetty.client.api.Request;
/**
@@ -138,15 +138,15 @@ public abstract class AuthenticationPlugin implements SolrInfoBean, SolrMetricPr
*/
public void closeRequest() {
}
- protected SolrMetrics metrics;
+ protected SolrMetricsContext metrics;
@Override
- public SolrMetrics getMetrics() {
+ public SolrMetricsContext getSolrMetricsContext() {
return metrics;
}
@Override
- public void initializeMetrics(SolrMetrics metrics) {
+ public void initializeMetrics(SolrMetricsContext metrics) {
this.metrics = metrics.getChildInfo(this);
// Metrics
numErrors = this.metrics.meter(this, "errors", getCategory().toString());
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 3c16ce6..9a3856d 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
@@ -31,7 +31,7 @@ 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.metrics.SolrMetricsContext;
import org.apache.solr.request.SolrQueryRequest;
import org.apache.solr.request.SolrRequestHandler;
import org.apache.solr.response.SolrQueryResponse;
@@ -391,7 +391,7 @@ public class MetricsHandlerTest extends SolrTestCaseJ4 {
static class RefreshablePluginHolder extends PluginBag.PluginHolder<SolrRequestHandler> {
private DumpRequestHandler rh;
- private SolrMetrics metricsInfo;
+ private SolrMetricsContext metricsInfo;
public RefreshablePluginHolder(PluginInfo info, DumpRequestHandler rh) {
super(info);
@@ -404,11 +404,12 @@ public class MetricsHandlerTest extends SolrTestCaseJ4 {
}
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.metricsInfo = rh.getSolrMetricsContext();
+ // nocommit
+// if(metricsInfo.tag.contains(String.valueOf(rh.hashCode()))){
+// //this created a new child metrics
+// metricsInfo = metricsInfo.getParent();
+// }
this.rh.close();
}
@@ -440,10 +441,10 @@ public class MetricsHandlerTest extends SolrTestCaseJ4 {
}
@Override
- public void initializeMetrics(SolrMetrics m) {
+ public void initializeMetrics(SolrMetricsContext m) {
super.initializeMetrics(m);
MetricsMap metrics = new MetricsMap((detailed, map) -> map.putAll(gaugevals));
- solrMetrics.gauge(this,
+ solrMetricsContext.gauge(this,
metrics, true, "dumphandlergauge", getCategory().toString());
}