You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by ds...@apache.org on 2018/09/27 19:40:07 UTC

lucene-solr:master: SOLR-12652: Remove SolrMetricManager.overridableRegistryName()

Repository: lucene-solr
Updated Branches:
  refs/heads/master 2369c8963 -> 044bc2a48


SOLR-12652: Remove SolrMetricManager.overridableRegistryName()


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

Branch: refs/heads/master
Commit: 044bc2a48522cb9d1e112aa3be4f2d7e6c2ed498
Parents: 2369c89
Author: Peter Somogyi <ps...@apache.org>
Authored: Thu Sep 27 15:39:55 2018 -0400
Committer: David Smiley <ds...@apache.org>
Committed: Thu Sep 27 15:39:55 2018 -0400

----------------------------------------------------------------------
 solr/CHANGES.txt                                |  2 +
 .../solr/handler/admin/MetricsHandler.java      |  4 +-
 .../handler/admin/MetricsHistoryHandler.java    |  4 +-
 .../apache/solr/metrics/SolrMetricManager.java  | 65 ++++++--------------
 .../metrics/reporters/SolrSlf4jReporter.java    |  4 +-
 .../reporters/solr/SolrClusterReporter.java     |  8 +--
 .../solr/metrics/SolrMetricManagerTest.java     | 14 -----
 7 files changed, 32 insertions(+), 69 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/044bc2a4/solr/CHANGES.txt
----------------------------------------------------------------------
diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index 914fa7c..9ef4145 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -73,6 +73,8 @@ Other Changes
 
 * SOLR-12805: Store previous term (generation) of replica when start recovery process (Cao Manh Dat)
 
+* SOLR-12652: Remove SolrMetricManager.overridableRegistryName method (Peter Somogyi via David Smiley)
+
 ==================  7.6.0 ==================
 
 Consult the LUCENE_CHANGES.txt file for additional, low level, changes in this release.

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/044bc2a4/solr/core/src/java/org/apache/solr/handler/admin/MetricsHandler.java
----------------------------------------------------------------------
diff --git a/solr/core/src/java/org/apache/solr/handler/admin/MetricsHandler.java b/solr/core/src/java/org/apache/solr/handler/admin/MetricsHandler.java
index 1f1a820..752e021 100644
--- a/solr/core/src/java/org/apache/solr/handler/admin/MetricsHandler.java
+++ b/solr/core/src/java/org/apache/solr/handler/admin/MetricsHandler.java
@@ -257,7 +257,7 @@ public class MetricsHandler extends RequestHandlerBase implements PermissionName
             allRegistries = true;
             break;
           }
-          initialPrefixes.add(SolrMetricManager.overridableRegistryName(s.trim()));
+          initialPrefixes.add(SolrMetricManager.enforcePrefix(s.trim()));
         }
         if (allRegistries) {
           return metricManager.registryNames();
@@ -276,7 +276,7 @@ public class MetricsHandler extends RequestHandlerBase implements PermissionName
             allRegistries = true;
             break;
           }
-          initialPrefixes.add(SolrMetricManager.overridableRegistryName(s.trim()));
+          initialPrefixes.add(SolrMetricManager.enforcePrefix(s.trim()));
         }
         if (allRegistries) {
           return metricManager.registryNames();

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/044bc2a4/solr/core/src/java/org/apache/solr/handler/admin/MetricsHistoryHandler.java
----------------------------------------------------------------------
diff --git a/solr/core/src/java/org/apache/solr/handler/admin/MetricsHistoryHandler.java b/solr/core/src/java/org/apache/solr/handler/admin/MetricsHistoryHandler.java
index 1c74dba..b569fe8 100644
--- a/solr/core/src/java/org/apache/solr/handler/admin/MetricsHistoryHandler.java
+++ b/solr/core/src/java/org/apache/solr/handler/admin/MetricsHistoryHandler.java
@@ -297,7 +297,7 @@ public class MetricsHistoryHandler extends RequestHandlerBase implements Permiss
   }
 
   public void removeHistory(String registry) throws IOException {
-    registry = SolrMetricManager.overridableRegistryName(registry);
+    registry = SolrMetricManager.enforcePrefix(registry);
     knownDbs.remove(registry);
     factory.remove(registry);
   }
@@ -586,7 +586,7 @@ public class MetricsHistoryHandler extends RequestHandlerBase implements Permiss
   }
 
   private RrdDef createDef(String registry, Group group) {
-    registry = SolrMetricManager.overridableRegistryName(registry);
+    registry = SolrMetricManager.enforcePrefix(registry);
 
     // base sampling period is collectPeriod - samples more frequent than
     // that will be dropped, samples less frequent will be interpolated

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/044bc2a4/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 e9cb111..6e01204 100644
--- a/solr/core/src/java/org/apache/solr/metrics/SolrMetricManager.java
+++ b/solr/core/src/java/org/apache/solr/metrics/SolrMetricManager.java
@@ -354,13 +354,13 @@ public class SolrMetricManager {
   }
 
   /**
-   * Check whether a registry with a given (overridable) name already exists.
+   * Check whether a registry with a given name already exists.
    * @param name registry name
    * @return true if this name points to a registry that already exists, false otherwise
    */
   public boolean hasRegistry(String name) {
     Set<String> names = registryNames();
-    name = overridableRegistryName(name);
+    name = enforcePrefix(name);
     return names.contains(name);
   }
 
@@ -399,18 +399,13 @@ public class SolrMetricManager {
 
   /**
    * Check for predefined shared registry names. This compares the input name
-   * with normalized and possibly overriden names of predefined shared registries -
+   * with normalized names of predefined shared registries -
    * {@link #JVM_REGISTRY} and {@link #JETTY_REGISTRY}.
-   * @param registry already normalized and possibly overriden name
+   * @param registry already normalized name
    * @return true if the name matches one of shared registries
    */
   private static boolean isSharedRegistry(String registry) {
-    if (overridableRegistryName(JETTY_REGISTRY).equals(registry) ||
-        overridableRegistryName(JVM_REGISTRY).equals(registry)) {
-      return true;
-    } else {
-      return false;
-    }
+    return JETTY_REGISTRY.equals(registry) || JVM_REGISTRY.equals(registry);
   }
 
   /**
@@ -419,7 +414,7 @@ public class SolrMetricManager {
    * @return existing or newly created registry
    */
   public MetricRegistry registry(String registry) {
-    registry = overridableRegistryName(registry);
+    registry = enforcePrefix(registry);
     if (isSharedRegistry(registry)) {
       return SharedMetricRegistries.getOrCreate(registry);
     } else {
@@ -454,8 +449,8 @@ public class SolrMetricManager {
   public void removeRegistry(String registry) {
     // close any reporters for this registry first
     closeReporters(registry, null);
-    // make sure we use a name with prefix, with overrides
-    registry = overridableRegistryName(registry);
+    // make sure we use a name with prefix
+    registry = enforcePrefix(registry);
     if (isSharedRegistry(registry)) {
       SharedMetricRegistries.remove(registry);
     } else {
@@ -478,8 +473,8 @@ public class SolrMetricManager {
    *                  an empty one under the previous name.
    */
   public void swapRegistries(String registry1, String registry2) {
-    registry1 = overridableRegistryName(registry1);
-    registry2 = overridableRegistryName(registry2);
+    registry1 = enforcePrefix(registry1);
+    registry2 = enforcePrefix(registry2);
     if (isSharedRegistry(registry1) || isSharedRegistry(registry2)) {
       throw new UnsupportedOperationException("Cannot swap shared registry: " + registry1 + ", " + registry2);
     }
@@ -748,26 +743,6 @@ public class SolrMetricManager {
   }
 
   /**
-   * Allows named registries to be renamed using System properties.
-   * 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:</p>
-   * <pre>
-   *   ... -Dsolr.core.collection1=solr.core.allCollections -Dsolr.core.collection2=solr.core.allCollections
-   * </pre>
-   * <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.SolrInfoBean.Group} of components that reported to that name is restarted.
-   * @param registry The name of the registry
-   * @return A potentially overridden (via System properties) registry name
-   */
-  public static String overridableRegistryName(String registry) {
-    String fqRegistry = enforcePrefix(registry);
-    return enforcePrefix(System.getProperty(fqRegistry,fqRegistry));
-  }
-
-  /**
    * Enforces the leading {@link #REGISTRY_NAME_PREFIX} in a name.
    * @param name input name, possibly without the prefix
    * @return original name if it contained the prefix, or the
@@ -805,7 +780,7 @@ public class SolrMetricManager {
     } else {
       fullName = MetricRegistry.name(group.toString(), names);
     }
-    return overridableRegistryName(fullName);
+    return enforcePrefix(fullName);
   }
 
   // reporter management
@@ -838,7 +813,7 @@ public class SolrMetricManager {
           String[] targets = target.split("[\\s,]+");
           boolean found = false;
           for (String t : targets) {
-            t = overridableRegistryName(t);
+            t = enforcePrefix(t);
             if (registryName.equals(t)) {
               found = true;
               break;
@@ -914,8 +889,8 @@ public class SolrMetricManager {
       throw new IllegalArgumentException("loadReporter called with missing arguments: " +
           "registry=" + registry + ", loader=" + loader + ", pluginInfo=" + pluginInfo);
     }
-    // make sure we use a name with prefix, with overrides
-    registry = overridableRegistryName(registry);
+    // make sure we use a name with prefix
+    registry = enforcePrefix(registry);
     SolrMetricReporter reporter = loader.newInstance(
         pluginInfo.className,
         SolrMetricReporter.class,
@@ -987,8 +962,8 @@ public class SolrMetricManager {
    * @return true if a named reporter existed and was closed.
    */
   public boolean closeReporter(String registry, String name, String tag) {
-    // make sure we use a name with prefix, with overrides
-    registry = overridableRegistryName(registry);
+    // make sure we use a name with prefix
+    registry = enforcePrefix(registry);
     try {
       if (!reportersLock.tryLock(10, TimeUnit.SECONDS)) {
         log.warn("Could not obtain lock to modify reporters registry: " + registry);
@@ -1038,8 +1013,8 @@ public class SolrMetricManager {
    * @return names of closed reporters
    */
   public Set<String> closeReporters(String registry, String tag) {
-    // make sure we use a name with prefix, with overrides
-    registry = overridableRegistryName(registry);
+    // make sure we use a name with prefix
+    registry = enforcePrefix(registry);
     try {
       if (!reportersLock.tryLock(10, TimeUnit.SECONDS)) {
         log.warn("Could not obtain lock to modify reporters registry: " + registry);
@@ -1085,8 +1060,8 @@ public class SolrMetricManager {
    * @return map of reporters and their names, may be empty but never null
    */
   public Map<String, SolrMetricReporter> getReporters(String registry) {
-    // make sure we use a name with prefix, with overrides
-    registry = overridableRegistryName(registry);
+    // make sure we use a name with prefix
+    registry = enforcePrefix(registry);
     try {
       if (!reportersLock.tryLock(10, TimeUnit.SECONDS)) {
         log.warn("Could not obtain lock to modify reporters registry: " + registry);

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/044bc2a4/solr/core/src/java/org/apache/solr/metrics/reporters/SolrSlf4jReporter.java
----------------------------------------------------------------------
diff --git a/solr/core/src/java/org/apache/solr/metrics/reporters/SolrSlf4jReporter.java b/solr/core/src/java/org/apache/solr/metrics/reporters/SolrSlf4jReporter.java
index ff00a00..e65d119 100644
--- a/solr/core/src/java/org/apache/solr/metrics/reporters/SolrSlf4jReporter.java
+++ b/solr/core/src/java/org/apache/solr/metrics/reporters/SolrSlf4jReporter.java
@@ -131,9 +131,9 @@ public class SolrSlf4jReporter extends FilteringSolrMetricReporter {
     if (logger == null || logger.isEmpty()) {
       // construct logger name from Group
       if (pluginInfo.attributes.containsKey("group")) {
-        logger = SolrMetricManager.overridableRegistryName(pluginInfo.attributes.get("group"));
+        logger = SolrMetricManager.enforcePrefix(pluginInfo.attributes.get("group"));
       } else if (pluginInfo.attributes.containsKey("registry")) {
-        String reg = SolrMetricManager.overridableRegistryName(pluginInfo.attributes.get("registry"));
+        String reg = SolrMetricManager.enforcePrefix(pluginInfo.attributes.get("registry"));
         String[] names = reg.split("\\.");
         if (names.length < 2) {
           logger = reg;

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/044bc2a4/solr/core/src/java/org/apache/solr/metrics/reporters/solr/SolrClusterReporter.java
----------------------------------------------------------------------
diff --git a/solr/core/src/java/org/apache/solr/metrics/reporters/solr/SolrClusterReporter.java b/solr/core/src/java/org/apache/solr/metrics/reporters/solr/SolrClusterReporter.java
index 17390e1..c1b424f 100644
--- a/solr/core/src/java/org/apache/solr/metrics/reporters/solr/SolrClusterReporter.java
+++ b/solr/core/src/java/org/apache/solr/metrics/reporters/solr/SolrClusterReporter.java
@@ -94,14 +94,14 @@ import static org.apache.solr.common.params.CommonParams.ID;
 public class SolrClusterReporter extends SolrCoreContainerReporter {
   private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
 
-  public static final String CLUSTER_GROUP = SolrMetricManager.overridableRegistryName(SolrInfoBean.Group.cluster.toString());
+  public static final String CLUSTER_GROUP = SolrMetricManager.enforcePrefix(SolrInfoBean.Group.cluster.toString());
 
   public static final List<SolrReporter.Report> DEFAULT_REPORTS = new ArrayList<SolrReporter.Report>() {{
     add(new SolrReporter.Report(CLUSTER_GROUP, "jetty",
-        SolrMetricManager.overridableRegistryName(SolrInfoBean.Group.jetty.toString()),
+        SolrMetricManager.enforcePrefix(SolrInfoBean.Group.jetty.toString()),
         Collections.emptySet())); // all metrics
     add(new SolrReporter.Report(CLUSTER_GROUP, "jvm",
-        SolrMetricManager.overridableRegistryName(SolrInfoBean.Group.jvm.toString()),
+        SolrMetricManager.enforcePrefix(SolrInfoBean.Group.jvm.toString()),
         new HashSet<String>() {{
           add("memory\\.total\\..*");
           add("memory\\.heap\\..*");
@@ -111,7 +111,7 @@ public class SolrClusterReporter extends SolrCoreContainerReporter {
           add("os\\.OpenFileDescriptorCount");
           add("threads\\.count");
         }}));
-    add(new SolrReporter.Report(CLUSTER_GROUP, "node", SolrMetricManager.overridableRegistryName(SolrInfoBean.Group.node.toString()),
+    add(new SolrReporter.Report(CLUSTER_GROUP, "node", SolrMetricManager.enforcePrefix(SolrInfoBean.Group.node.toString()),
         new HashSet<String>() {{
           add("CONTAINER\\.cores\\..*");
           add("CONTAINER\\.fs\\..*");

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/044bc2a4/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
index 9e15acf..e95b73e 100644
--- a/solr/core/src/test/org/apache/solr/metrics/SolrMetricManagerTest.java
+++ b/solr/core/src/test/org/apache/solr/metrics/SolrMetricManagerTest.java
@@ -37,20 +37,6 @@ 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 testSwapRegistries() throws Exception {
     Random r = random();