SOLR-13677: reverting the last commit (#863)

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) {
+  public void initializeMetrics(SolrMetricManager manager, String registryName, String tag, String scope) {
-    super.initializeMetrics(m);
+    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 //////////////////////

solr/core/src/java/org/apache/solr/core/PluginBag.java

@@ -197,7 +197,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();
   }
 

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<String> 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;

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) {
+  public void initializeMetrics(SolrMetricManager manager, String registry, String tag, String scope) {
-    super.initializeMetrics(m);
+    super.initializeMetrics(manager, registry, tag, scope);
-    solrMetrics.gauge(this,  () -> (core != null && !core.isClosed() ? NumberUtils.readableSize(core.getIndexSize()) : ""),
+
-        true, "indexSize", getCategory().toString());
+    manager.registerGauge(this, registry, () -> (core != null && !core.isClosed() ? NumberUtils.readableSize(core.getIndexSize()) : ""),
-    solrMetrics.gauge(this, () -> (core != null && !core.isClosed() ? getIndexVersion().toString() : ""),
+        tag, true, "indexSize", getCategory().toString(), scope);
-         true, "indexVersion", getCategory().toString());
+    manager.registerGauge(this, registry, () -> (core != null && !core.isClosed() ? getIndexVersion().toString() : ""),
-    solrMetrics.gauge(this, () -> (core != null && !core.isClosed() ? getIndexVersion().generation : 0),
+        tag, true, "indexVersion", getCategory().toString(), scope);
-        true, GENERATION, getCategory().toString());
+    manager.registerGauge(this, registry, () -> (core != null && !core.isClosed() ? getIndexVersion().generation : 0),
-    solrMetrics.gauge(this, () -> (core != null && !core.isClosed() ? core.getIndexDir() : ""),
+        tag, true, GENERATION, getCategory().toString(), scope);
-        true, "indexPath", getCategory().toString());
+    manager.registerGauge(this, registry, () -> (core != null && !core.isClosed() ? core.getIndexDir() : ""),
-    solrMetrics.gauge(this, () -> isMaster,
+        tag, true, "indexPath", getCategory().toString(), scope);
-         true, "isMaster", getCategory().toString());
+    manager.registerGauge(this, registry, () -> isMaster,
-    solrMetrics.gauge(this, () -> isSlave,
+        tag, true, "isMaster", getCategory().toString(), scope);
-         true, "isSlave", getCategory().toString());
+    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());
+    manager.registerGauge(this, registry, fetcherMap, tag, true, "fetcher", getCategory().toString(), scope);
-    solrMetrics.gauge(this, () -> isMaster && includeConfFiles != null ? includeConfFiles : "",
+    manager.registerGauge(this, registry, () -> isMaster && includeConfFiles != null ? includeConfFiles : "",
-         true, "confFilesToReplicate", getCategory().toString());
+        tag, true, "confFilesToReplicate", getCategory().toString(), scope);
-    solrMetrics.gauge(this, () -> isMaster ? getReplicateAfterStrings() : Collections.<String>emptyList(),
+    manager.registerGauge(this, registry, () -> isMaster ? getReplicateAfterStrings() : Collections.<String>emptyList(),
-        true, REPLICATE_AFTER, getCategory().toString());
+        tag, true, REPLICATE_AFTER, getCategory().toString(), scope);
-    solrMetrics.gauge(this,  () -> isMaster && replicationEnabled.get(),
+    manager.registerGauge(this, registry, () -> isMaster && replicationEnabled.get(),
-        true, "replicationEnabled", getCategory().toString());
+        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?

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<String> 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() {
+  public void initializeMetrics(SolrMetricManager manager, String registryName, String tag, final String scope) {
-    return solrMetrics;
+    this.metricManager = manager;
-  }
+    this.registryName = registryName;
-
+    this.registry = manager.registry(registryName);
-  @Override
+    numErrors = manager.meter(this, registryName, "errors", getCategory().toString(), scope);
-  public void initializeMetrics(SolrMetrics m) {
+    numServerErrors = manager.meter(this, registryName, "serverErrors", getCategory().toString(), scope);
-    this.solrMetrics = m.getChildInfo(this);
+    numClientErrors = manager.meter(this, registryName, "clientErrors", getCategory().toString(), scope);
-    numErrors = solrMetrics.meter(this, "errors", getCategory().toString());
+    numTimeouts = manager.meter(this, registryName, "timeouts", getCategory().toString(), scope);
-    numServerErrors = solrMetrics.meter(this, "serverErrors", getCategory().toString());
+    requests = manager.counter(this, registryName, "requests", getCategory().toString(), scope);
-    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())));
-    solrMetrics.gauge(this, metricsMap, true, "shardRequests", getCategory().toString());
+    manager.registerGauge(this, registryName, metricsMap, tag, true, "shardRequests", getCategory().toString(), scope);
-    requestTimes = solrMetrics.timer(this,"requestTimes", getCategory().toString());
+    requestTimes = manager.timer(this, registryName, "requestTimes", getCategory().toString(), scope);
-    totalTime = solrMetrics.counter(this, "totalTime", getCategory().toString());
+    totalTime = manager.counter(this, registryName, "totalTime", getCategory().toString(), scope);
-    solrMetrics.gauge(this, () -> handlerStart, true, "handlerStart", getCategory().toString());
+    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

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) {
+  public void initializeMetrics(SolrMetricManager manager, String registryName, String tag, String scope) {
-    super.initializeMetrics(m);
+    super.initializeMetrics(manager, registryName, tag, scope);
-    parallelExecutor = MetricUtils.instrumentedExecutorService(parallelExecutor, this, solrMetrics.getRegistry(),
+    parallelExecutor = MetricUtils.instrumentedExecutorService(parallelExecutor, this, manager.registry(registryName),
-        SolrMetricManager.mkName("parallelCoreAdminExecutor", getCategory().name(), solrMetrics.scope, "threadPool"));
+        SolrMetricManager.mkName("parallelCoreAdminExecutor", getCategory().name(),scope, "threadPool"));
   }
   @Override
   public Boolean registerV2() {

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() {
+  public void initializeMetrics(SolrMetricManager manager, String registryName, String tag, String scope) {
-    return metricsInfo;
+    this.registryName = registryName;
-  }
+    this.metricManager = manager;
-
+    registry = manager.registry(registryName);
-  @Override
+    manager.registerGauge(this, registryName, () -> ramBytesUsed(), tag, true, "totalSizeInBytes", getCategory().toString(), scope);
-  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<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);
+    manager.registerGauge(this, registryName, suggestersMap, tag, true, "suggesters", getCategory().toString(), scope);
   }
 
   @Override

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) {
+  public int unregisterGauges(String registryName, String tag) {
-    if (tagSegment == null) {
+    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) {
+      if (metric instanceof GaugeWrapper &&
-        GaugeWrapper wrapper = (GaugeWrapper) metric;
+          tag.equals(((GaugeWrapper) metric).getTag())) {
-        boolean toRemove = tagSegment.equals(wrapper.getTag()) || wrapper.getTag().contains(tagSegment);
+        removed.incrementAndGet();
-        if (toRemove) removed.incrementAndGet();
+        return true;
-        return toRemove;
+      } 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<String> 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();

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 {
+public interface SolrMetricProducer {
-
-  /**
-   * 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 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) {
+  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();
-  }
-
 }

solr/core/src/java/org/apache/solr/metrics/SolrMetrics.java

@@ -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<String> 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);
-  }
-}

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;
  * <p>
  * Also see <a href="http://wiki.apache.org/solr/SolrCaching">SolrCaching</a>
  *
+ *
  * @see org.apache.solr.util.ConcurrentLRUCache
  * @see org.apache.solr.search.SolrCache
  * @since solr 1.4
@@ -78,6 +77,7 @@ public class FastLRUCache<K, V> extends SolrCacheBase implements SolrCache<K, V>
 
   private MetricsMap cacheMap;
   private Set<String> metricNames = ConcurrentHashMap.newKeySet();
+  private MetricRegistry registry;
 
   @Override
   public Object init(Map args, Object persistence, CacheRegenerator regenerator) {
@@ -225,7 +225,6 @@ public class FastLRUCache<K, V> extends SolrCacheBase implements SolrCache<K, V>
 
   @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<K, V> extends SolrCacheBase implements SolrCache<K, V>
     return metricNames;
   }
 
-
-  SolrMetrics solrMetrics;
-
   @Override
-  public SolrMetrics getMetrics() {
+  public void initializeMetrics(SolrMetricManager manager, String registryName, String tag, String scope) {
-    return solrMetrics;
+    registry = manager.registry(registryName);
-  }
-
-  @Override
-  public void initializeMetrics(SolrMetrics info) {
-    solrMetrics = info.getChildInfo(this);
     cacheMap = new MetricsMap((detailed, map) -> {
       if (cache != null) {
         ConcurrentLRUCache.Stats stats = cache.getStats();
@@ -311,8 +302,7 @@ public class FastLRUCache<K, V> extends SolrCacheBase implements SolrCache<K, V>
         }
       }
     });
-    String metricName = SolrMetricManager.makeName(ImmutableList.of(getCategory().toString()), solrMetrics.scope);
+    manager.registerGauge(this, registryName, cacheMap, tag, true, scope, getCategory().toString());
-    solrMetrics.gauge(this, cacheMap,true, metricName);
   }
 
 
@@ -323,7 +313,7 @@ public class FastLRUCache<K, V> extends SolrCacheBase implements SolrCache<K, V>
 
   @Override
   public MetricRegistry getMetricRegistry() {
-    return solrMetrics == null ? null : solrMetrics.getRegistry();
+    return registry;
   }
 
   @Override

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<K, V> implements SolrCache<K, V>, Accountable {
   private Boolean timeDecay = true;
   private MetricsMap cacheMap;
   private Set<String> metricNames = ConcurrentHashMap.newKeySet();
+  private MetricRegistry registry;
 
   private int maxSize;
   private int minSizeLimit;
@@ -227,7 +226,6 @@ public class LFUCache<K, V> implements SolrCache<K, V>, 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<K, V> implements SolrCache<K, V>, Accountable {
     return "0." + hundredths;
   }
 
-
-  private SolrMetrics solrMetrics;
-
   @Override
-  public SolrMetrics getMetrics() {
+  public void initializeMetrics(SolrMetricManager manager, String registryName, String tag, String scope) {
-    return solrMetrics;
+    registry = manager.registry(registryName);
-  }
-
-  @Override
-  public void initializeMetrics(SolrMetrics info) {
-    solrMetrics = info.getChildInfo(this);
     cacheMap = new MetricsMap((detailed, map) -> {
       if (cache != null) {
         ConcurrentLFUCache.Stats stats = cache.getStats();
@@ -326,8 +316,7 @@ public class LFUCache<K, V> implements SolrCache<K, V>, Accountable {
 
       }
     });
-    String metricName = SolrMetricManager.makeName(ImmutableList.of(getCategory().toString()), solrMetrics.scope);
+    manager.registerGauge(this, registryName, cacheMap, tag, true, scope, getCategory().toString());
-    solrMetrics.gauge(this, cacheMap, true, metricName);
   }
 
   // for unit tests only
@@ -342,8 +331,7 @@ public class LFUCache<K, V> implements SolrCache<K, V>, Accountable {
 
   @Override
   public MetricRegistry getMetricRegistry() {
-    return solrMetrics == null ? null : solrMetrics.getRegistry();
+    return registry;
-
   }
 
   @Override

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<K,V> extends SolrCacheBase implements SolrCache<K,V>, Acco
   private String description="LRU Cache";
   private MetricsMap cacheMap;
   private Set<String> metricNames = ConcurrentHashMap.newKeySet();
+  private MetricRegistry registry;
   private int maxSize;
   private int initialSize;
 
@@ -282,7 +281,7 @@ public class LRUCache<K,V> extends SolrCacheBase implements SolrCache<K,V>, Acco
 
   @Override
   public void close() {
-    if(solrMetrics != null) solrMetrics.unregister();
+
   }
 
   //////////////////////// SolrInfoMBeans methods //////////////////////
@@ -303,16 +302,9 @@ public class LRUCache<K,V> extends SolrCacheBase implements SolrCache<K,V>, Acco
     return metricNames;
   }
 
-  SolrMetrics solrMetrics;
-
   @Override
-  public SolrMetrics getMetrics() {
+  public void initializeMetrics(SolrMetricManager manager, String registryName, String tag, String scope) {
-    return solrMetrics;
+    registry = manager.registry(registryName);
-  }
-
-  @Override
-  public void initializeMetrics(SolrMetrics m) {
-    solrMetrics = m.getChildInfo(this);
     cacheMap = new MetricsMap((detailed, res) -> {
       synchronized (map) {
         res.put(LOOKUPS_PARAM, lookups);
@@ -337,8 +329,7 @@ public class LRUCache<K,V> extends SolrCacheBase implements SolrCache<K,V>, 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);
+    manager.registerGauge(this, registryName, cacheMap, tag, true, scope, getCategory().toString());
-    solrMetrics.gauge(this, cacheMap, true, metricName);
   }
 
   // for unit tests only
@@ -348,7 +339,7 @@ public class LRUCache<K,V> extends SolrCacheBase implements SolrCache<K,V>, Acco
 
   @Override
   public MetricRegistry getMetricRegistry() {
-    return solrMetrics ==null ?null: solrMetrics.getRegistry();
+    return registry;
   }
 
   @Override

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<K, V> implements SolrCache<K,V> {
       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<K, V> implements SolrCache<K,V> {
     return delegate.getCategory();
   }
 
-  private SolrMetrics metrics;
+
-  @Override
+  private MetricsInfo metricsInfo;
-  public void initializeMetrics(SolrMetrics info) {
+
-    this.metrics = info;
+  public static class MetricsInfo {
-    delegate.initializeMetrics(info);
+    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);
+
+  }
 }

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<String> 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() {
+  public void initializeMetrics(SolrMetricManager manager, String registryName, String tag, final String scope) {
-    return metrics;
+    this.metricManager = manager;
-  }
+    this.registryName = registryName;
-
-  @Override
-  public void initializeMetrics(SolrMetrics metrics) {
-    this.metrics = metrics.getChildInfo(this);
     // Metrics
-    numErrors = this.metrics.meter(this, "errors", getCategory().toString());
+    registry = manager.registry(registryName);
-    requests = this.metrics.counter(this, "requests", getCategory().toString());
+    numErrors = manager.meter(this, registryName, "errors", getCategory().toString(), scope);
-    numAuthenticated = this.metrics.counter(this, "authenticated",getCategory().toString());
+    requests = manager.counter(this, registryName, "requests", getCategory().toString(), scope);
-    numPassThrough = this.metrics.counter(this, "passThrough",  getCategory().toString());
+    numAuthenticated = manager.counter(this, registryName, "authenticated", getCategory().toString(), scope);
-    numWrongCredentials = this.metrics.counter(this, "failWrongCredentials",getCategory().toString());
+    numPassThrough = manager.counter(this, registryName, "passThrough", getCategory().toString(), scope);
-    numMissingCredentials = this.metrics.counter(this,  "failMissingCredentials",getCategory().toString());
+    numWrongCredentials = manager.counter(this, registryName, "failWrongCredentials", getCategory().toString(), scope);
-    requestTimes = this.metrics.timer(this,"requestTimes", getCategory().toString());
+    numMissingCredentials = manager.counter(this, registryName, "failMissingCredentials", getCategory().toString(), scope);
-    totalTime = this.metrics.counter(this,"totalTime", getCategory().toString());
+    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;
   }
 
 }

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<SolrRequestHandler> {
-
-    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<String, Object> 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;
-    }
-  }
 }