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 2016/12/13 19:14:18 UTC

[1/2] lucene-solr:feature/metrics: SOLR-4735 Add unit test for SolrMetricManager. Other changes: * add metrics to SolrCore related to newSearcher events. * build a dot-separated hierarchical registry name for cores that are a part of SolrCloud collecti

Repository: lucene-solr
Updated Branches:
  refs/heads/feature/metrics d1227af72 -> 3ba5d7db0


SOLR-4735 Add unit test for SolrMetricManager. Other changes:
* add metrics to SolrCore related to newSearcher events.
* build a dot-separated hierarchical registry name for cores that are a part of
  SolrCloud collection.
* don't split metrics by class in SolrJmxReporter - this only obscured the hierarchy.


Project: http://git-wip-us.apache.org/repos/asf/lucene-solr/repo
Commit: http://git-wip-us.apache.org/repos/asf/lucene-solr/commit/98a04b6c
Tree: http://git-wip-us.apache.org/repos/asf/lucene-solr/tree/98a04b6c
Diff: http://git-wip-us.apache.org/repos/asf/lucene-solr/diff/98a04b6c

Branch: refs/heads/feature/metrics
Commit: 98a04b6cca32d7b2a3da8b30291e1df1edd885cd
Parents: d1227af
Author: Andrzej Bialecki <ab...@apache.org>
Authored: Tue Dec 13 19:16:40 2016 +0100
Committer: Andrzej Bialecki <ab...@apache.org>
Committed: Tue Dec 13 19:16:40 2016 +0100

----------------------------------------------------------------------
 .../org/apache/solr/core/CoreContainer.java     |   2 +-
 .../src/java/org/apache/solr/core/SolrCore.java |  27 +-
 .../solr/metrics/SolrCoreMetricManager.java     |  50 +++-
 .../apache/solr/metrics/SolrMetricManager.java  |  30 ++-
 .../solr/metrics/reporters/SolrJmxReporter.java |  12 +-
 .../solr/metrics/SolrCoreMetricManagerTest.java |  18 ++
 .../solr/metrics/SolrMetricManagerTest.java     | 264 +++++++++++++++++++
 .../metrics/SolrMetricsIntegrationTest.java     |   2 +-
 .../metrics/reporters/SolrJmxReporterTest.java  |   1 -
 9 files changed, 380 insertions(+), 26 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/98a04b6c/solr/core/src/java/org/apache/solr/core/CoreContainer.java
----------------------------------------------------------------------
diff --git a/solr/core/src/java/org/apache/solr/core/CoreContainer.java b/solr/core/src/java/org/apache/solr/core/CoreContainer.java
index 030a15f..3348d71 100644
--- a/solr/core/src/java/org/apache/solr/core/CoreContainer.java
+++ b/solr/core/src/java/org/apache/solr/core/CoreContainer.java
@@ -482,7 +482,7 @@ public class CoreContainer {
     if(pkiAuthenticationPlugin != null)
       containerHandlers.put(PKIAuthenticationPlugin.PATH, pkiAuthenticationPlugin.getRequestHandler());
 
-    SolrMetricManager.loadReporters(cfg.getMetricReporterPlugins(), loader, SolrInfoMBean.Group.node, null);
+    SolrMetricManager.loadReporters(cfg.getMetricReporterPlugins(), loader, SolrInfoMBean.Group.node);
 
     coreConfigService = ConfigSetService.createConfigSetService(cfg, loader, zkSys.zkController);
 

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/98a04b6c/solr/core/src/java/org/apache/solr/core/SolrCore.java
----------------------------------------------------------------------
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 b430c6a..6f50559 100644
--- a/solr/core/src/java/org/apache/solr/core/SolrCore.java
+++ b/solr/core/src/java/org/apache/solr/core/SolrCore.java
@@ -53,6 +53,8 @@ import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicInteger;
 import java.util.concurrent.locks.ReentrantLock;
 
+import com.codahale.metrics.Counter;
+import com.codahale.metrics.Timer;
 import com.google.common.collect.MapMaker;
 import org.apache.commons.io.FileUtils;
 import org.apache.lucene.analysis.util.ResourceLoader;
@@ -95,6 +97,7 @@ import org.apache.solr.handler.component.HighlightComponent;
 import org.apache.solr.handler.component.SearchComponent;
 import org.apache.solr.logging.MDCLoggingContext;
 import org.apache.solr.metrics.SolrCoreMetricManager;
+import org.apache.solr.metrics.SolrMetricManager;
 import org.apache.solr.metrics.SolrMetricProducer;
 import org.apache.solr.request.SolrQueryRequest;
 import org.apache.solr.request.SolrRequestHandler;
@@ -204,6 +207,12 @@ public final class SolrCore implements SolrInfoMBean, Closeable {
   private final ReentrantLock ruleExpiryLock;
   private final ReentrantLock snapshotDelLock; // A lock instance to guard against concurrent deletions.
 
+  private final Timer newSearcherTimer;
+  private final Timer newSearcherWarmupTimer;
+  private final Counter newSearcherCounter;
+  private final Counter newSearcherMaxErrorsCounter;
+  private final Counter newSearcherOtherErrorsCounter;
+
   public Date getStartTimeStamp() { return startTime; }
 
   private final Map<Object, IndexFingerprint> perSegmentFingerprintCache = new MapMaker().weakKeys().makeMap();
@@ -395,7 +404,7 @@ public final class SolrCore implements SolrInfoMBean, Closeable {
     this.logid = (v==null)?"":("["+v+"] ");
     this.coreDescriptor = new CoreDescriptor(v, this.coreDescriptor);
     if (metricManager != null) {
-      metricManager.afterCoreSetName(oldName, this.name);
+      metricManager.afterCoreSetName();
     }
   }
 
@@ -858,6 +867,13 @@ public final class SolrCore implements SolrInfoMBean, Closeable {
     // Initialize the metrics manager
     this.metricManager = initMetricManager(config);
 
+    // initialize searcher-related metrics
+    newSearcherCounter = SolrMetricManager.counter(metricManager.getRegistryName(), "newSearcher");
+    newSearcherTimer = SolrMetricManager.timer(metricManager.getRegistryName(), "newSearcherTime");
+    newSearcherWarmupTimer = SolrMetricManager.timer(metricManager.getRegistryName(), "newSearcherWarmup");
+    newSearcherMaxErrorsCounter = SolrMetricManager.counter(metricManager.getRegistryName(), "newSearcherMaxErrors");
+    newSearcherOtherErrorsCounter = SolrMetricManager.counter(metricManager.getRegistryName(), "newSearcherErrors");
+
     // Initialize JMX
     this.infoRegistry = initInfoRegistry(name, config);
     infoRegistry.put("fieldCache", new SolrFieldCacheMBean());
@@ -1961,6 +1977,7 @@ public final class SolrCore implements SolrInfoMBean, Closeable {
       // first: increment count to signal other threads that we are
       //        opening a new searcher.
       onDeckSearchers++;
+      newSearcherCounter.inc();
       if (onDeckSearchers < 1) {
         // should never happen... just a sanity check
         log.error(logid+"ERROR!!! onDeckSearchers is " + onDeckSearchers);
@@ -1970,6 +1987,7 @@ public final class SolrCore implements SolrInfoMBean, Closeable {
         String msg="Error opening new searcher. exceeded limit of maxWarmingSearchers="+maxWarmingSearchers + ", try again later.";
         log.warn(logid+""+ msg);
         // HTTP 503==service unavailable, or 409==Conflict
+        newSearcherMaxErrorsCounter.inc();
         throw new SolrException(SolrException.ErrorCode.SERVICE_UNAVAILABLE,msg);
       } else if (onDeckSearchers > 1) {
         log.warn(logid+"PERFORMANCE WARNING: Overlapping onDeckSearchers=" + onDeckSearchers);
@@ -1983,6 +2001,7 @@ public final class SolrCore implements SolrInfoMBean, Closeable {
     boolean success = false;
 
     openSearcherLock.lock();
+    Timer.Context timerContext = newSearcherTimer.time();
     try {
       searchHolder = openNewSearcher(updateHandlerReopens, false);
        // the searchHolder will be incremented once already (and it will eventually be assigned to _searcher when registered)
@@ -2025,6 +2044,7 @@ public final class SolrCore implements SolrInfoMBean, Closeable {
         // should this go before the other event handlers or after?
         if (currSearcher != null) {
           future = searcherExecutor.submit(() -> {
+            Timer.Context warmupContext = newSearcherWarmupTimer.time();
             try {
               newSearcher.warm(currSearcher);
             } catch (Throwable e) {
@@ -2032,6 +2052,8 @@ public final class SolrCore implements SolrInfoMBean, Closeable {
               if (e instanceof Error) {
                 throw (Error) e;
               }
+            } finally {
+              warmupContext.close();
             }
             return null;
           });
@@ -2112,7 +2134,10 @@ public final class SolrCore implements SolrInfoMBean, Closeable {
       throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, e);
     } finally {
 
+      timerContext.close();
+
       if (!success) {
+        newSearcherOtherErrorsCounter.inc();;
         synchronized (searcherLock) {
           onDeckSearchers--;
 

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/98a04b6c/solr/core/src/java/org/apache/solr/metrics/SolrCoreMetricManager.java
----------------------------------------------------------------------
diff --git a/solr/core/src/java/org/apache/solr/metrics/SolrCoreMetricManager.java b/solr/core/src/java/org/apache/solr/metrics/SolrCoreMetricManager.java
index 22faa1a..bf3ee1b 100644
--- a/solr/core/src/java/org/apache/solr/metrics/SolrCoreMetricManager.java
+++ b/solr/core/src/java/org/apache/solr/metrics/SolrCoreMetricManager.java
@@ -43,6 +43,7 @@ public class SolrCoreMetricManager implements Closeable {
   private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
 
   private final SolrCore core;
+  private String registryName;
 
   /**
    * Constructs a metric manager.
@@ -51,6 +52,7 @@ public class SolrCoreMetricManager implements Closeable {
    */
   public SolrCoreMetricManager(SolrCore core) {
     this.core = core;
+    registryName = createRegistryName(core.getCoreDescriptor().getCollectionName(), core.getName());
   }
 
   /**
@@ -60,25 +62,23 @@ public class SolrCoreMetricManager implements Closeable {
   public void loadReporters() {
     NodeConfig nodeConfig = core.getCoreDescriptor().getCoreContainer().getConfig();
     PluginInfo[] pluginInfos = nodeConfig.getMetricReporterPlugins();
-    SolrMetricManager.loadReporters(pluginInfos, core.getResourceLoader(), SolrInfoMBean.Group.core, core.getName());
+    SolrMetricManager.loadReporters(pluginInfos, core.getResourceLoader(), SolrInfoMBean.Group.core, registryName);
   }
 
   /**
    * Make sure that metrics already collected that correspond to the old core name
    * are carried over and will be used under the new core name.
    * This method also reloads reporters so that they use the new core name.
-   * @param oldCoreName core name before renaming, may be null
-   * @param newCoreName core name after renaming, not null
    */
-  public void afterCoreSetName(String oldCoreName, String newCoreName) {
-    if (newCoreName.equals(oldCoreName)) {
+  public void afterCoreSetName() {
+    String oldRegistryName = registryName;
+    registryName = createRegistryName(core.getCoreDescriptor().getCollectionName(), core.getName());
+    if (oldRegistryName.equals(registryName)) {
       return;
     }
-    String oldRegistryName = SolrMetricManager.getRegistryName(SolrInfoMBean.Group.core, oldCoreName);
-    String newRegistryName = SolrMetricManager.getRegistryName(SolrInfoMBean.Group.core, newCoreName);
     // close old reporters
     SolrMetricManager.closeReporters(oldRegistryName);
-    SolrMetricManager.moveMetrics(oldRegistryName, newRegistryName, null);
+    SolrMetricManager.moveMetrics(oldRegistryName, registryName, null);
     // old registry is no longer used - we have moved the metrics
     SolrMetricManager.removeRegistry(oldRegistryName);
     // load reporters again, using the new core name
@@ -118,9 +118,41 @@ public class SolrCoreMetricManager implements Closeable {
   /**
    * Retrieves the metric registry name of the manager.
    *
+   * In order to make it easier for reporting tools to aggregate metrics from
+   * different cores that logically belong to a single collection we convert the
+   * core name into a dot-separated hierarchy of: collection name, shard name (with optional split)
+   * and replica name.
+   *
+   * <p>For example, when the core name looks like this but it's NOT a SolrCloud collection:
+   * <code>my_collection_shard1_1_replica1</code> then this will be used as the registry name (plus
+   * the required <code>solr.core</code> prefix). However,
+   * if this is a SolrCloud collection <code>my_collection</code> then the registry name will become
+   * <code>solr.core.my_collection.shard1_1.replica1</code>.</p>
+   *
+   *
    * @return the metric registry name of the manager.
    */
   public String getRegistryName() {
-    return SolrMetricManager.getRegistryName(SolrInfoMBean.Group.core, core.getName());
+    return registryName;
+  }
+
+  /* package visibility for tests. */
+  String createRegistryName(String collectionName, String coreName) {
+    if (collectionName == null || (collectionName != null && !coreName.startsWith(collectionName + "_"))) {
+      // single core, or unknown naming scheme
+      return SolrMetricManager.getRegistryName(SolrInfoMBean.Group.core, coreName);
+    }
+    // split "collection1_shard1_1_replica1" into parts
+    String str = coreName.substring(collectionName.length() + 1);
+    String shard;
+    String replica = null;
+    int pos = str.lastIndexOf("_replica");
+    if (pos == -1) { // ?? no _replicaN part ??
+      shard = str;
+    } else {
+      shard = str.substring(0, pos);
+      replica = str.substring(pos + 1);
+    }
+    return SolrMetricManager.getRegistryName(SolrInfoMBean.Group.core, collectionName, shard, replica);
   }
 }

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/98a04b6c/solr/core/src/java/org/apache/solr/metrics/SolrMetricManager.java
----------------------------------------------------------------------
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 5ce4bb2..24cf36d 100644
--- a/solr/core/src/java/org/apache/solr/metrics/SolrMetricManager.java
+++ b/solr/core/src/java/org/apache/solr/metrics/SolrMetricManager.java
@@ -369,11 +369,27 @@ public class SolrMetricManager {
   /**
    * Helper method to construct a properly prefixed registry name based on the group.
    * @param group reporting group
-   * @param names optional child elements of the registry name
-   * @return fully-qualified and prefixed registry name
+   * @param names optional child elements of the registry name. If exactly one element is provided
+   *              and it already contains the required prefix and group name then this value will be used,
+   *              and the {@param group} parameter will be ignored.
+   * @return fully-qualified and prefixed registry name, with overrides applied.
    */
   public static String getRegistryName(SolrInfoMBean.Group group, String... names) {
-    String fullName = MetricRegistry.name(group.toString(), names);
+    String fullName;
+    String prefix = REGISTRY_NAME_PREFIX + group.toString() + ".";
+    // check for existing prefix and group
+    if (names != null && names.length > 0 && names[0] != null && names[0].startsWith(prefix)) {
+      // assume the first segment already was expanded
+      if (names.length > 1) {
+        String[] newNames = new String[names.length - 1];
+        System.arraycopy(names, 1, newNames, 0, newNames.length);
+        fullName = MetricRegistry.name(names[0], newNames);
+      } else {
+        fullName = MetricRegistry.name(names[0]);
+      }
+    } else {
+      fullName = MetricRegistry.name(group.toString(), names);
+    }
     return overridableRegistryName(fullName);
   }
 
@@ -388,13 +404,13 @@ public class SolrMetricManager {
    * @param pluginInfos plugin configurations
    * @param loader resource loader
    * @param group selected group, not null
-   * @param registry optional fully-qualified registry name
+   * @param registryNames optional child registry name elements
    */
-  public static void loadReporters(PluginInfo[] pluginInfos, SolrResourceLoader loader, SolrInfoMBean.Group group, String registry) {
+  public static void loadReporters(PluginInfo[] pluginInfos, SolrResourceLoader loader, SolrInfoMBean.Group group, String... registryNames) {
     if (pluginInfos == null || pluginInfos.length == 0) {
       return;
     }
-    String registryName = getRegistryName(group, registry);
+    String registryName = getRegistryName(group, registryNames);
     for (PluginInfo info : pluginInfos) {
       String target = info.attributes.get("group");
       if (target == null) { // no "group"
@@ -461,7 +477,7 @@ public class SolrMetricManager {
     try {
       reporter.init(pluginInfo);
     } catch (IllegalStateException e) {
-      throw new IllegalArgumentException("loadReporter called with invalid plugin info = " + pluginInfo);
+      throw new IllegalArgumentException("reporter init failed: " + pluginInfo, e);
     }
     try {
       if (!reportersLock.tryLock(10, TimeUnit.SECONDS)) {

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/98a04b6c/solr/core/src/java/org/apache/solr/metrics/reporters/SolrJmxReporter.java
----------------------------------------------------------------------
diff --git a/solr/core/src/java/org/apache/solr/metrics/reporters/SolrJmxReporter.java b/solr/core/src/java/org/apache/solr/metrics/reporters/SolrJmxReporter.java
index 4f5b5aa..380bbaa 100644
--- a/solr/core/src/java/org/apache/solr/metrics/reporters/SolrJmxReporter.java
+++ b/solr/core/src/java/org/apache/solr/metrics/reporters/SolrJmxReporter.java
@@ -243,9 +243,9 @@ public class SolrJmxReporter extends SolrMetricReporter {
         sb.append(metricInfo.category.toString());
         sb.append(",scope=");
         sb.append(metricInfo.scope);
-        // split by type, but don't call it 'type' :)
-        sb.append(",class=");
-        sb.append(type);
+        // we could also split by type, but don't call it 'type' :)
+        // sb.append(",class=");
+        //sb.append(type);
         sb.append(",name=");
         sb.append(metricInfo.name);
       } else {
@@ -263,9 +263,9 @@ public class SolrJmxReporter extends SolrMetricReporter {
           sb.append(',');
         }
         // split by type
-        sb.append("class=");
-        sb.append(type);
-        sb.append(",name=");
+        // sb.append("class=");
+        // sb.append(type);
+        sb.append("name=");
         sb.append(path[path.length - 1]);
       }
 

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/98a04b6c/solr/core/src/test/org/apache/solr/metrics/SolrCoreMetricManagerTest.java
----------------------------------------------------------------------
diff --git a/solr/core/src/test/org/apache/solr/metrics/SolrCoreMetricManagerTest.java b/solr/core/src/test/org/apache/solr/metrics/SolrCoreMetricManagerTest.java
index 095eef9..f532d54 100644
--- a/solr/core/src/test/org/apache/solr/metrics/SolrCoreMetricManagerTest.java
+++ b/solr/core/src/test/org/apache/solr/metrics/SolrCoreMetricManagerTest.java
@@ -149,4 +149,22 @@ public class SolrCoreMetricManagerTest extends SolrTestCaseJ4 {
       assertEquals(expectedMetric, actualMetric);
     }
   }
+
+  @Test
+  public void testRegistryName() throws Exception {
+    String collectionName = "my_collection_";
+    String cloudCoreName = "my_collection__shard1_0_replica0";
+    String simpleCoreName = "collection_1_replica0";
+    String simpleRegistryName = "solr.core." + simpleCoreName;
+    String cloudRegistryName = "solr.core." + cloudCoreName;
+    String nestedRegistryName = "solr.core.my_collection_.shard1_0.replica0";
+    // pass through
+    assertEquals(cloudRegistryName, metricManager.createRegistryName(null, cloudCoreName));
+    assertEquals(simpleRegistryName, metricManager.createRegistryName(null, simpleCoreName));
+    // unknown naming scheme -> pass through
+    assertEquals(simpleRegistryName, metricManager.createRegistryName(collectionName, simpleCoreName));
+    // cloud collection
+    assertEquals(nestedRegistryName, metricManager.createRegistryName(collectionName, cloudCoreName));
+
+  }
 }

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/98a04b6c/solr/core/src/test/org/apache/solr/metrics/SolrMetricManagerTest.java
----------------------------------------------------------------------
diff --git a/solr/core/src/test/org/apache/solr/metrics/SolrMetricManagerTest.java b/solr/core/src/test/org/apache/solr/metrics/SolrMetricManagerTest.java
new file mode 100644
index 0000000..2886016
--- /dev/null
+++ b/solr/core/src/test/org/apache/solr/metrics/SolrMetricManagerTest.java
@@ -0,0 +1,264 @@
+/*
+ * 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.HashMap;
+import java.util.Map;
+import java.util.Random;
+import java.util.Set;
+
+import com.codahale.metrics.Counter;
+import com.codahale.metrics.Metric;
+import com.codahale.metrics.MetricRegistry;
+import org.apache.lucene.util.TestUtil;
+import org.apache.solr.SolrTestCaseJ4;
+import org.apache.solr.common.util.NamedList;
+import org.apache.solr.core.PluginInfo;
+import org.apache.solr.core.SolrInfoMBean;
+import org.apache.solr.core.SolrResourceLoader;
+import org.apache.solr.metrics.reporters.MockMetricReporter;
+import org.junit.Test;
+
+public class SolrMetricManagerTest extends SolrTestCaseJ4 {
+
+  @Test
+  public void testOverridableRegistryName() throws Exception {
+    Random r = random();
+    String originalName = TestUtil.randomSimpleString(r, 1, 10);
+    String targetName = TestUtil.randomSimpleString(r, 1, 10);
+    // no override
+    String result = SolrMetricManager.overridableRegistryName(originalName);
+    assertEquals(SolrMetricManager.REGISTRY_NAME_PREFIX + originalName, result);
+    // with override
+    System.setProperty(SolrMetricManager.REGISTRY_NAME_PREFIX + originalName, targetName);
+    result = SolrMetricManager.overridableRegistryName(originalName);
+    assertEquals(SolrMetricManager.REGISTRY_NAME_PREFIX + targetName, result);
+  }
+
+  @Test
+  public void testMoveMetrics() throws Exception {
+    Random r = random();
+
+    Map<String, Counter> metrics1 = SolrMetricTestUtils.getRandomMetrics(r, true);
+    Map<String, Counter> metrics2 = SolrMetricTestUtils.getRandomMetrics(r, true);
+    String fromName = TestUtil.randomSimpleString(r, 1, 10);
+    String toName = TestUtil.randomSimpleString(r, 1, 10);
+    // register test metrics
+    for (Map.Entry<String, Counter> entry : metrics1.entrySet()) {
+      SolrMetricManager.register(fromName, entry.getValue(), false, entry.getKey(), "metrics1");
+    }
+    for (Map.Entry<String, Counter> entry : metrics2.entrySet()) {
+      SolrMetricManager.register(fromName, entry.getValue(), false, entry.getKey(), "metrics2");
+    }
+    assertEquals(metrics1.size() + metrics2.size(), SolrMetricManager.registry(fromName).getMetrics().size());
+
+    // move metrics1
+    SolrMetricManager.moveMetrics(fromName, toName, new SolrMetricManager.PrefixFilter("metrics1"));
+    // check the remaining metrics
+    Map<String, Metric> fromMetrics = SolrMetricManager.registry(fromName).getMetrics();
+    assertEquals(metrics2.size(), fromMetrics.size());
+    for (Map.Entry<String, Counter> entry : metrics2.entrySet()) {
+      Object value = fromMetrics.get(SolrMetricManager.mkName(entry.getKey(), "metrics2"));
+      assertNotNull(value);
+      assertEquals(entry.getValue(), value);
+    }
+    // check the moved metrics
+    Map<String, Metric> toMetrics = SolrMetricManager.registry(toName).getMetrics();
+    assertEquals(metrics1.size(), toMetrics.size());
+    for (Map.Entry<String, Counter> entry : metrics1.entrySet()) {
+      Object value = toMetrics.get(SolrMetricManager.mkName(entry.getKey(), "metrics1"));
+      assertNotNull(value);
+      assertEquals(entry.getValue(), value);
+    }
+
+    // move all remaining metrics
+    SolrMetricManager.moveMetrics(fromName, toName, null);
+    fromMetrics = SolrMetricManager.registry(fromName).getMetrics();
+    assertEquals(0, fromMetrics.size());
+    toMetrics = SolrMetricManager.registry(toName).getMetrics();
+    assertEquals(metrics1.size() + metrics2.size(), toMetrics.size());
+  }
+
+  @Test
+  public void testRegisterAll() throws Exception {
+    Random r = random();
+
+    Map<String, Counter> metrics = SolrMetricTestUtils.getRandomMetrics(r, true);
+    MetricRegistry mr = new MetricRegistry();
+    for (Map.Entry<String, Counter> entry : metrics.entrySet()) {
+      mr.register(entry.getKey(), entry.getValue());
+    }
+
+    String registryName = TestUtil.randomSimpleString(r, 1, 10);
+    assertEquals(0, SolrMetricManager.registry(registryName).getMetrics().size());
+    SolrMetricManager.registerAll(registryName, mr, false);
+    // this should simply skip existing names
+    SolrMetricManager.registerAll(registryName, mr, true);
+    // this should produce error
+    try {
+      SolrMetricManager.registerAll(registryName, mr, false);
+      fail("registerAll with duplicate metric names should fail");
+    } catch (IllegalArgumentException e) {
+      // expected
+    }
+  }
+
+  @Test
+  public void testClearMetrics() throws Exception {
+    Random r = random();
+
+    Map<String, Counter> metrics = SolrMetricTestUtils.getRandomMetrics(r, true);
+    String registryName = TestUtil.randomSimpleString(r, 1, 10);
+
+    for (Map.Entry<String, Counter> entry : metrics.entrySet()) {
+      SolrMetricManager.register(registryName, entry.getValue(), false, entry.getKey(), "foo", "bar");
+    }
+    for (Map.Entry<String, Counter> entry : metrics.entrySet()) {
+      SolrMetricManager.register(registryName, entry.getValue(), false, entry.getKey(), "foo", "baz");
+    }
+    for (Map.Entry<String, Counter> entry : metrics.entrySet()) {
+      SolrMetricManager.register(registryName, entry.getValue(), false, entry.getKey(), "foo");
+    }
+
+    assertEquals(metrics.size() * 3, SolrMetricManager.registry(registryName).getMetrics().size());
+
+    // clear "foo.bar"
+    Set<String> removed = SolrMetricManager.clearMetrics(registryName, "foo", "bar");
+    assertEquals(metrics.size(), removed.size());
+    for (String s : removed) {
+      assertTrue(s.startsWith("foo.bar."));
+    }
+    removed = SolrMetricManager.clearMetrics(registryName, "foo", "baz");
+    assertEquals(metrics.size(), removed.size());
+    for (String s : removed) {
+      assertTrue(s.startsWith("foo.baz."));
+    }
+    // perhaps surprisingly, this works too - see PrefixFilter docs
+    removed = SolrMetricManager.clearMetrics(registryName, "fo");
+    assertEquals(metrics.size(), removed.size());
+    for (String s : removed) {
+      assertTrue(s.startsWith("foo."));
+    }
+  }
+
+  @Test
+  public void testSimpleMetrics() throws Exception {
+    Random r = random();
+
+    String registryName = TestUtil.randomSimpleString(r, 1, 10);
+
+    SolrMetricManager.counter(registryName, "simple_counter", "foo", "bar");
+    SolrMetricManager.timer(registryName, "simple_timer", "foo", "bar");
+    SolrMetricManager.meter(registryName, "simple_meter", "foo", "bar");
+    SolrMetricManager.histogram(registryName, "simple_histogram", "foo", "bar");
+    Map<String, Metric> metrics = SolrMetricManager.registry(registryName).getMetrics();
+    assertEquals(4, metrics.size());
+    for (Map.Entry<String, Metric> entry : metrics.entrySet()) {
+      assertTrue(entry.getKey().startsWith("foo.bar.simple_"));
+    }
+  }
+
+  @Test
+  public void testRegistryName() throws Exception {
+    Random r = random();
+
+    String name = TestUtil.randomSimpleString(r, 1, 10);
+
+    String result = SolrMetricManager.getRegistryName(SolrInfoMBean.Group.core, name, "collection1");
+    assertEquals("solr.core." + name + ".collection1", result);
+    // try it with already prefixed name - group will be ignored
+    result = SolrMetricManager.getRegistryName(SolrInfoMBean.Group.core, result);
+    assertEquals("solr.core." + name + ".collection1", result);
+    // try it with already prefixed name but with additional segments
+    result = SolrMetricManager.getRegistryName(SolrInfoMBean.Group.core, result, "shard1", "replica1");
+    assertEquals("solr.core." + name + ".collection1.shard1.replica1", result);
+  }
+
+  @Test
+  public void testReporters() throws Exception {
+    Random r = random();
+
+    SolrResourceLoader loader = new SolrResourceLoader();
+
+    PluginInfo[] plugins = new PluginInfo[] {
+        createPluginInfo("universal_foo", null, null),
+        createPluginInfo("multigroup_foo", "jvm, node, core", null),
+        createPluginInfo("multiregistry_foo", null, "solr.node, solr.core.collection1"),
+        createPluginInfo("specific_foo", null, "solr.core.collection1"),
+        createPluginInfo("node_foo", "node", null),
+        createPluginInfo("core_foo", "core", null)
+    };
+
+    SolrMetricManager.loadReporters(plugins, loader, SolrInfoMBean.Group.node);
+    Map<String, SolrMetricReporter> reporters = SolrMetricManager.getReporters(
+        SolrMetricManager.getRegistryName(SolrInfoMBean.Group.node));
+    assertEquals(4, reporters.size());
+    assertTrue(reporters.containsKey("universal_foo"));
+    assertTrue(reporters.containsKey("multigroup_foo"));
+    assertTrue(reporters.containsKey("node_foo"));
+    assertTrue(reporters.containsKey("multiregistry_foo"));
+
+    SolrMetricManager.loadReporters(plugins, loader, SolrInfoMBean.Group.core, "collection1");
+    reporters = SolrMetricManager.getReporters(
+        SolrMetricManager.getRegistryName(SolrInfoMBean.Group.core, "collection1"));
+    assertEquals(5, reporters.size());
+    assertTrue(reporters.containsKey("universal_foo"));
+    assertTrue(reporters.containsKey("multigroup_foo"));
+    assertTrue(reporters.containsKey("specific_foo"));
+    assertTrue(reporters.containsKey("core_foo"));
+    assertTrue(reporters.containsKey("multiregistry_foo"));
+
+    SolrMetricManager.loadReporters(plugins, loader, SolrInfoMBean.Group.jvm);
+    reporters = SolrMetricManager.getReporters(
+        SolrMetricManager.getRegistryName(SolrInfoMBean.Group.jvm));
+    assertEquals(2, reporters.size());
+    assertTrue(reporters.containsKey("universal_foo"));
+    assertTrue(reporters.containsKey("multigroup_foo"));
+
+    SolrMetricManager.removeRegistry("solr.jvm");
+    reporters = SolrMetricManager.getReporters(
+        SolrMetricManager.getRegistryName(SolrInfoMBean.Group.jvm));
+    assertEquals(0, reporters.size());
+
+    SolrMetricManager.removeRegistry("solr.node");
+    reporters = SolrMetricManager.getReporters(
+        SolrMetricManager.getRegistryName(SolrInfoMBean.Group.node));
+    assertEquals(0, reporters.size());
+
+    SolrMetricManager.removeRegistry("solr.core.collection1");
+    reporters = SolrMetricManager.getReporters(
+        SolrMetricManager.getRegistryName(SolrInfoMBean.Group.core, "collection1"));
+    assertEquals(0, reporters.size());
+
+  }
+
+  private PluginInfo createPluginInfo(String name, String group, String registry) {
+    Map<String,String> attrs = new HashMap<>();
+    attrs.put("name", name);
+    attrs.put("class", MockMetricReporter.class.getName());
+    if (group != null) {
+      attrs.put("group", group);
+    }
+    if (registry != null) {
+      attrs.put("registry", registry);
+    }
+    NamedList initArgs = new NamedList();
+    initArgs.add("configurable", "true");
+    return new PluginInfo("SolrMetricReporter", attrs, initArgs, null);
+  }
+}

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/98a04b6c/solr/core/src/test/org/apache/solr/metrics/SolrMetricsIntegrationTest.java
----------------------------------------------------------------------
diff --git a/solr/core/src/test/org/apache/solr/metrics/SolrMetricsIntegrationTest.java b/solr/core/src/test/org/apache/solr/metrics/SolrMetricsIntegrationTest.java
index 6b5abf8..67a073b 100644
--- a/solr/core/src/test/org/apache/solr/metrics/SolrMetricsIntegrationTest.java
+++ b/solr/core/src/test/org/apache/solr/metrics/SolrMetricsIntegrationTest.java
@@ -41,7 +41,7 @@ import org.junit.Test;
 
 public class SolrMetricsIntegrationTest extends SolrTestCaseJ4 {
   private static final int MAX_ITERATIONS = 20;
-  private static final String CORE_NAME = "metricsIntegration";
+  private static final String CORE_NAME = "metrics_integration";
   private static final String METRIC_NAME = "requestTimes";
   private static final String HANDLER_NAME = "standard";
   private static final String[] REPORTER_NAMES = {"reporter1", "reporter2"};

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/98a04b6c/solr/core/src/test/org/apache/solr/metrics/reporters/SolrJmxReporterTest.java
----------------------------------------------------------------------
diff --git a/solr/core/src/test/org/apache/solr/metrics/reporters/SolrJmxReporterTest.java b/solr/core/src/test/org/apache/solr/metrics/reporters/SolrJmxReporterTest.java
index 6f9f7c0..607296e 100644
--- a/solr/core/src/test/org/apache/solr/metrics/reporters/SolrJmxReporterTest.java
+++ b/solr/core/src/test/org/apache/solr/metrics/reporters/SolrJmxReporterTest.java
@@ -135,7 +135,6 @@ public class SolrJmxReporterTest extends SolrTestCaseJ4 {
     Map<String, Counter> metrics = SolrMetricTestUtils.getRandomMetrics(random, true);
     SolrMetricProducer producer = SolrMetricTestUtils.getProducerOf(category, scope, metrics);
     metricManager.registerMetricProducer(scope, producer);
-
     Set<ObjectInstance> objects = mBeanServer.queryMBeans(null, null);
     assertEquals(metrics.size(), objects.stream().
         filter(o -> scope.equals(o.getObjectName().getKeyProperty("scope")) &&


[2/2] lucene-solr:feature/metrics: SOLR-4735 Fix javadoc errors.

Posted by ab...@apache.org.
SOLR-4735 Fix javadoc errors.


Project: http://git-wip-us.apache.org/repos/asf/lucene-solr/repo
Commit: http://git-wip-us.apache.org/repos/asf/lucene-solr/commit/3ba5d7db
Tree: http://git-wip-us.apache.org/repos/asf/lucene-solr/tree/3ba5d7db
Diff: http://git-wip-us.apache.org/repos/asf/lucene-solr/diff/3ba5d7db

Branch: refs/heads/feature/metrics
Commit: 3ba5d7db0971eb638a5e1dae17fb3c9ea36440d4
Parents: 98a04b6
Author: Andrzej Bialecki <ab...@apache.org>
Authored: Tue Dec 13 20:13:58 2016 +0100
Committer: Andrzej Bialecki <ab...@apache.org>
Committed: Tue Dec 13 20:13:58 2016 +0100

----------------------------------------------------------------------
 .../src/java/org/apache/solr/metrics/SolrMetricManager.java     | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/3ba5d7db/solr/core/src/java/org/apache/solr/metrics/SolrMetricManager.java
----------------------------------------------------------------------
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 24cf36d..abb79fb 100644
--- a/solr/core/src/java/org/apache/solr/metrics/SolrMetricManager.java
+++ b/solr/core/src/java/org/apache/solr/metrics/SolrMetricManager.java
@@ -336,11 +336,10 @@ public class SolrMetricManager {
    * This would be mostly be useful if you want to combine the metrics from a few registries for a single
    * reporter.
    * <p>For example, in order to collect metrics from related cores in a single registry you could specify
-   * the following system properties:
+   * the following system properties:</p>
    * <pre>
    *   ... -Dsolr.core.collection1=solr.core.allCollections -Dsolr.core.collection2=solr.core.allCollections
    * </pre>
-   * </p>
    * <b>NOTE:</b> Once a registry is renamed in a way that its metrics are combined with another repository
    * it is no longer possible to retrieve the original metrics until this renaming is removed and the Solr
    * {@link org.apache.solr.core.SolrInfoMBean.Group} of components that reported to that name is restarted.
@@ -371,7 +370,7 @@ public class SolrMetricManager {
    * @param group reporting group
    * @param names optional child elements of the registry name. If exactly one element is provided
    *              and it already contains the required prefix and group name then this value will be used,
-   *              and the {@param group} parameter will be ignored.
+   *              and the group parameter will be ignored.
    * @return fully-qualified and prefixed registry name, with overrides applied.
    */
   public static String getRegistryName(SolrInfoMBean.Group group, String... names) {