From 042478cfa795dd537dcd4863a0524a73bad9a740 Mon Sep 17 00:00:00 2001 From: Noble Paul Date: Sun, 8 Sep 2019 14:57:47 +1000 Subject: [PATCH] SOLR-13677: reverting the last commit --- .../handler/dataimport/DataImportHandler.java | 8 +- .../java/org/apache/solr/core/PluginBag.java | 1 - .../java/org/apache/solr/core/SolrCore.java | 2 +- .../solr/handler/ReplicationHandler.java | 45 +++---- .../solr/handler/RequestHandlerBase.java | 38 +++--- .../solr/handler/admin/CoreAdminHandler.java | 9 +- .../handler/component/SuggestComponent.java | 23 ++-- .../solr/metrics/SolrMetricManager.java | 63 ++------- .../solr/metrics/SolrMetricProducer.java | 40 +----- .../org/apache/solr/metrics/SolrMetrics.java | 95 -------------- .../org/apache/solr/search/FastLRUCache.java | 22 +--- .../java/org/apache/solr/search/LFUCache.java | 22 +--- .../java/org/apache/solr/search/LRUCache.java | 21 +-- .../apache/solr/search/SolrCacheHolder.java | 38 +++++- .../solr/security/AuthenticationPlugin.java | 41 +++--- .../handler/admin/MetricsHandlerTest.java | 124 ------------------ 16 files changed, 144 insertions(+), 448 deletions(-) delete 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 80d374eb505..50938e4c380 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.SolrMetricManager; 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(SolrMetrics m) { - super.initializeMetrics(m); + public void initializeMetrics(SolrMetricManager manager, String registryName, String tag, String scope) { + super.initializeMetrics(manager, registryName, tag, scope); 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); } }); - solrMetrics.gauge(this, metrics, true, "importer", getCategory().toString()); + manager.registerGauge(this, registryName, metrics, tag, true, "importer", getCategory().toString(), scope); } // //////////////////////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 dd15a1a26c2..a9ea65b0f8a 100644 --- a/solr/core/src/java/org/apache/solr/core/PluginBag.java +++ b/solr/core/src/java/org/apache/solr/core/PluginBag.java @@ -197,7 +197,6 @@ 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(); } 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 3729ff13bfc..ef5fbb7bca5 100644 --- a/solr/core/src/java/org/apache/solr/core/SolrCore.java +++ b/solr/core/src/java/org/apache/solr/core/SolrCore.java @@ -234,7 +234,7 @@ public final class SolrCore implements SolrInfoBean, SolrMetricProducer, Closeab private final CoreContainer coreContainer; private Set metricNames = ConcurrentHashMap.newKeySet(); - private final String metricTag = getUniqueMetricTag(null); + private String metricTag = Integer.toHexString(hashCode()); 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 76ecbecd9f2..5098a0d786d 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.SolrMetricManager; import org.apache.solr.request.SolrQueryRequest; import org.apache.solr.response.SolrQueryResponse; import org.apache.solr.search.SolrIndexSearcher; @@ -863,20 +863,21 @@ public class ReplicationHandler extends RequestHandlerBase implements SolrCoreAw } @Override - 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()); + 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); final MetricsMap fetcherMap = new MetricsMap((detailed, map) -> { IndexFetcher fetcher = currentIndexFetcher; if (fetcher != null) { @@ -905,13 +906,13 @@ 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 : "", - 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()); + 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); } //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? 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 499db959e2e..212c30c033f 100644 --- a/solr/core/src/java/org/apache/solr/handler/RequestHandlerBase.java +++ b/solr/core/src/java/org/apache/solr/handler/RequestHandlerBase.java @@ -39,8 +39,8 @@ 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; @@ -79,6 +79,9 @@ 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") @@ -140,27 +143,22 @@ public abstract class RequestHandlerBase implements SolrRequestHandler, SolrInfo } - protected SolrMetrics solrMetrics; - @Override - 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()); + 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); 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()); + 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); } public static SolrParams getSolrParamsFromNamedList(NamedList args, String key) { @@ -276,7 +274,7 @@ public abstract class RequestHandlerBase implements SolrRequestHandler, SolrInfo @Override public MetricRegistry getMetricRegistry() { - return solrMetrics.getRegistry(); + return registry; } @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 ae225556b13..45cb0631e87 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,6 @@ 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; @@ -121,10 +120,10 @@ public class CoreAdminHandler extends RequestHandlerBase implements PermissionNa } @Override - public void initializeMetrics(SolrMetrics m) { - super.initializeMetrics(m); - parallelExecutor = MetricUtils.instrumentedExecutorService(parallelExecutor, this, solrMetrics.getRegistry(), - SolrMetricManager.mkName("parallelCoreAdminExecutor", getCategory().name(), solrMetrics.scope, "threadPool")); + 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")); } @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 00d86970e51..3ede10dc1e8 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,6 +88,9 @@ 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} */ @@ -347,25 +350,19 @@ public class SuggestComponent extends SearchComponent implements SolrCoreAware, return "Suggester component"; } - protected SolrMetrics metricsInfo; - @Override - 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); + 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); MetricsMap suggestersMap = new MetricsMap((detailed, map) -> { for (Map.Entry 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); + manager.registerGauge(this, registryName, suggestersMap, tag, true, "suggesters", getCategory().toString(), 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 187598d30ca..f029e603309 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,20 +690,6 @@ 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 @@ -755,22 +724,20 @@ public class SolrMetricManager { registerMetric(info, registry, new GaugeWrapper(gauge, tag), force, metricName, metricPath); } - public int unregisterGauges(String registryName, String tagSegment) { - if (tagSegment == null) { + public int unregisterGauges(String registryName, String tag) { + if (tag == null) { return 0; } MetricRegistry registry = registry(registryName); - if (registry == null) return 0; AtomicInteger removed = new AtomicInteger(); registry.removeMatching((name, metric) -> { - if (metric instanceof GaugeWrapper) { - GaugeWrapper wrapper = (GaugeWrapper) metric; - boolean toRemove = tagSegment.equals(wrapper.getTag()) || wrapper.getTag().contains(tagSegment); - if (toRemove) removed.incrementAndGet(); - return toRemove; - } + if (metric instanceof GaugeWrapper && + tag.equals(((GaugeWrapper) metric).getTag())) { + removed.incrementAndGet(); + return true; + } else { return false; - + } }); return removed.get(); } @@ -785,16 +752,10 @@ 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.size() == 0) { + if (path == null || path.length == 0) { return name; } else { StringBuilder sb = new StringBuilder(); 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 cb534aca5dc..deb2b1809be 100644 --- a/solr/core/src/java/org/apache/solr/metrics/SolrMetricProducer.java +++ b/solr/core/src/java/org/apache/solr/metrics/SolrMetricProducer.java @@ -19,26 +19,10 @@ package org.apache.solr.metrics; /** * Used by objects that expose metrics through {@link SolrCoreMetricManager}. */ -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; - } - +public interface SolrMetricProducer { /** * Initializes metrics specific to this producer - * * @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, @@ -47,25 +31,5 @@ public interface SolrMetricProducer extends AutoCloseable { * {@link #initializeMetrics(SolrMetricManager, String, String, String)} is called. * @param scope scope of the metrics (eg. handler name) to separate metrics of */ - 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(); - } - + void initializeMetrics(SolrMetricManager manager, String registry, String tag, String scope); } diff --git a/solr/core/src/java/org/apache/solr/metrics/SolrMetrics.java b/solr/core/src/java/org/apache/solr/metrics/SolrMetrics.java deleted file mode 100644 index d73b04b33c9..00000000000 --- a/solr/core/src/java/org/apache/solr/metrics/SolrMetrics.java +++ /dev/null @@ -1,95 +0,0 @@ -/* - * 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) { - String name = metricpath == null || metricpath.length == 0 ? metricName : createName(metricName, metricpath); - metricManager.registerGauge(info, getRegistry(), new SolrMetricManager.GaugeWrapper<>(gauge, tag), force, name); - } - - 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 5a5ea01b197..dee2e40faa1 100644 --- a/solr/core/src/java/org/apache/solr/search/FastLRUCache.java +++ b/solr/core/src/java/org/apache/solr/search/FastLRUCache.java @@ -25,13 +25,11 @@ import java.util.concurrent.CopyOnWriteArrayList; import java.util.concurrent.TimeUnit; import com.codahale.metrics.MetricRegistry; -import com.google.common.collect.ImmutableList; 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; @@ -44,6 +42,7 @@ import org.slf4j.LoggerFactory; *

* Also see SolrCaching * + * * @see org.apache.solr.util.ConcurrentLRUCache * @see org.apache.solr.search.SolrCache * @since solr 1.4 @@ -78,6 +77,7 @@ 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) { @@ -225,7 +225,6 @@ public class FastLRUCache extends SolrCacheBase implements SolrCache @Override public void close() { - if (solrMetrics != null) solrMetrics.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,17 +247,9 @@ public class FastLRUCache extends SolrCacheBase implements SolrCache return metricNames; } - - SolrMetrics solrMetrics; - @Override - public SolrMetrics getMetrics() { - return solrMetrics; - } - - @Override - public void initializeMetrics(SolrMetrics info) { - solrMetrics = info.getChildInfo(this); + public void initializeMetrics(SolrMetricManager manager, String registryName, String tag, String scope) { + registry = manager.registry(registryName); cacheMap = new MetricsMap((detailed, map) -> { if (cache != null) { ConcurrentLRUCache.Stats stats = cache.getStats(); @@ -311,8 +302,7 @@ public class FastLRUCache extends SolrCacheBase implements SolrCache } } }); - String metricName = SolrMetricManager.makeName(ImmutableList.of(getCategory().toString()), solrMetrics.scope); - solrMetrics.gauge(this, cacheMap,true, metricName); + manager.registerGauge(this, registryName, cacheMap, tag, true, scope, getCategory().toString()); } @@ -323,7 +313,7 @@ public class FastLRUCache extends SolrCacheBase implements SolrCache @Override public MetricRegistry getMetricRegistry() { - return solrMetrics == null ? null : solrMetrics.getRegistry(); + return registry; } @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 78e7cb8c0be..48bd92c0473 100644 --- a/solr/core/src/java/org/apache/solr/search/LFUCache.java +++ b/solr/core/src/java/org/apache/solr/search/LFUCache.java @@ -25,13 +25,11 @@ import java.util.concurrent.CopyOnWriteArrayList; import java.util.concurrent.TimeUnit; import com.codahale.metrics.MetricRegistry; -import com.google.common.collect.ImmutableList; 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; @@ -80,6 +78,7 @@ 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; @@ -227,7 +226,6 @@ 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 ////////////////////// @@ -255,17 +253,9 @@ public class LFUCache implements SolrCache, Accountable { return "0." + hundredths; } - - private SolrMetrics solrMetrics; - @Override - public SolrMetrics getMetrics() { - return solrMetrics; - } - - @Override - public void initializeMetrics(SolrMetrics info) { - solrMetrics = info.getChildInfo(this); + public void initializeMetrics(SolrMetricManager manager, String registryName, String tag, String scope) { + registry = manager.registry(registryName); cacheMap = new MetricsMap((detailed, map) -> { if (cache != null) { ConcurrentLFUCache.Stats stats = cache.getStats(); @@ -326,8 +316,7 @@ public class LFUCache implements SolrCache, Accountable { } }); - String metricName = SolrMetricManager.makeName(ImmutableList.of(getCategory().toString()), solrMetrics.scope); - solrMetrics.gauge(this, cacheMap, true, metricName); + manager.registerGauge(this, registryName, cacheMap, tag, true, scope, getCategory().toString()); } // for unit tests only @@ -342,8 +331,7 @@ public class LFUCache implements SolrCache, Accountable { @Override public MetricRegistry getMetricRegistry() { - return solrMetrics == null ? null : solrMetrics.getRegistry(); - + return registry; } @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 e9e378987e7..a76fefa89ed 100644 --- a/solr/core/src/java/org/apache/solr/search/LRUCache.java +++ b/solr/core/src/java/org/apache/solr/search/LRUCache.java @@ -27,14 +27,12 @@ import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.LongAdder; import com.codahale.metrics.MetricRegistry; -import com.google.common.collect.ImmutableList; import org.apache.lucene.util.Accountable; 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; @@ -76,6 +74,7 @@ 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; @@ -282,7 +281,7 @@ public class LRUCache extends SolrCacheBase implements SolrCache, Acco @Override public void close() { - if(solrMetrics != null) solrMetrics.unregister(); + } //////////////////////// SolrInfoMBeans methods ////////////////////// @@ -303,16 +302,9 @@ public class LRUCache extends SolrCacheBase implements SolrCache, Acco return metricNames; } - SolrMetrics solrMetrics; - @Override - public SolrMetrics getMetrics() { - return solrMetrics; - } - - @Override - public void initializeMetrics(SolrMetrics m) { - solrMetrics = m.getChildInfo(this); + public void initializeMetrics(SolrMetricManager manager, String registryName, String tag, String scope) { + registry = manager.registry(registryName); cacheMap = new MetricsMap((detailed, res) -> { synchronized (map) { res.put(LOOKUPS_PARAM, lookups); @@ -337,8 +329,7 @@ public class LRUCache extends SolrCacheBase implements SolrCache, Acco res.put("cumulative_evictions", stats.evictions.longValue()); res.put("cumulative_evictionsRamUsage", stats.evictionsRamUsage.longValue()); }); - String metricName = SolrMetricManager.makeName(ImmutableList.of(getCategory().toString()), solrMetrics.scope); - solrMetrics.gauge(this, cacheMap, true, metricName); + manager.registerGauge(this, registryName, cacheMap, tag, true, scope, getCategory().toString()); } // for unit tests only @@ -348,7 +339,7 @@ public class LRUCache extends SolrCacheBase implements SolrCache, Acco @Override public MetricRegistry getMetricRegistry() { - return solrMetrics ==null ?null: solrMetrics.getRegistry(); + return registry; } @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 9a83a984eb9..3b64e9dd9e6 100644 --- a/solr/core/src/java/org/apache/solr/search/SolrCacheHolder.java +++ b/solr/core/src/java/org/apache/solr/search/SolrCacheHolder.java @@ -26,7 +26,8 @@ import org.apache.solr.common.MapWriter; import org.apache.solr.core.PluginInfo; import org.apache.solr.core.RuntimeLib; import org.apache.solr.core.SolrCore; -import org.apache.solr.metrics.SolrMetrics; +import org.apache.solr.metrics.SolrMetricManager; +import org.apache.solr.metrics.SolrMetricProducer; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -75,7 +76,10 @@ public class SolrCacheHolder implements SolrCache { info = new CacheConfig.CacheInfo(info.cfg, info.core); delegate.close(); delegate = info.cache; - delegate.initializeMetrics(metrics); + if(metricsInfo != null){ + metricsInfo.init(delegate); + + } } } @@ -182,11 +186,31 @@ public class SolrCacheHolder implements SolrCache { return delegate.getCategory(); } - private SolrMetrics metrics; - @Override - public void initializeMetrics(SolrMetrics info) { - this.metrics = info; - delegate.initializeMetrics(info); + + private MetricsInfo metricsInfo; + + public static class MetricsInfo { + final SolrMetricManager manager; + final String registry; + final String tag; + final String scope; + + MetricsInfo(SolrMetricManager manager, String registry, String tag, String scope) { + this.manager = manager; + this.registry = registry; + this.tag = tag; + this.scope = scope; + } + + public void init(SolrMetricProducer metricProducer) { + metricProducer.initializeMetrics(manager,registry,tag,scope); + } } + @Override + public void initializeMetrics(SolrMetricManager manager, String registry, String tag, String scope) { + this.metricsInfo = new MetricsInfo(manager, registry, tag, scope); + delegate.initializeMetrics(manager, registry, tag, scope); + + } } 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 a0ec89ab554..31f5a74a731 100644 --- a/solr/core/src/java/org/apache/solr/security/AuthenticationPlugin.java +++ b/solr/core/src/java/org/apache/solr/security/AuthenticationPlugin.java @@ -19,6 +19,8 @@ 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; import java.util.concurrent.ConcurrentHashMap; @@ -30,22 +32,25 @@ import com.codahale.metrics.Timer; import org.apache.http.HttpRequest; import org.apache.http.protocol.HttpContext; import org.apache.solr.core.SolrInfoBean; +import org.apache.solr.metrics.SolrMetricManager; 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 SolrInfoBean, SolrMetricProducer { +public abstract class AuthenticationPlugin implements Closeable, 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(); @@ -137,25 +142,23 @@ public abstract class AuthenticationPlugin implements SolrInfoBean, SolrMetricPr */ public void closeRequest() { } - protected SolrMetrics metrics; @Override - public SolrMetrics getMetrics() { - return metrics; - } - - @Override - public void initializeMetrics(SolrMetrics metrics) { - this.metrics = metrics.getChildInfo(this); + public void initializeMetrics(SolrMetricManager manager, String registryName, String tag, final String scope) { + this.metricManager = manager; + this.registryName = registryName; // Metrics - 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()); + 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); + metricNames.addAll(Arrays.asList("errors", "requests", "authenticated", "passThrough", + "failWrongCredentials", "failMissingCredentials", "requestTimes", "totalTime")); } @Override @@ -180,7 +183,7 @@ public abstract class AuthenticationPlugin implements SolrInfoBean, SolrMetricPr @Override public MetricRegistry getMetricRegistry() { - return metrics == null ? null : metrics.getRegistry(); + return registry; } } 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 3c16ce65a5f..356e865427d 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,7 +17,6 @@ package org.apache.solr.handler.admin; -import java.util.Arrays; import java.util.Map; import com.codahale.metrics.Counter; @@ -25,15 +24,6 @@ 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; @@ -339,118 +329,4 @@ public class MetricsHandlerTest extends SolrTestCaseJ4 { 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; - } - } }