You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by ma...@apache.org on 2020/08/20 15:29:47 UTC

[lucene-solr] branch reference_impl_dev updated: @580 Whoops, didn't actually finish addressing this.

This is an automated email from the ASF dual-hosted git repository.

markrmiller pushed a commit to branch reference_impl_dev
in repository https://gitbox.apache.org/repos/asf/lucene-solr.git


The following commit(s) were added to refs/heads/reference_impl_dev by this push:
     new 7b14abc  @580 Whoops, didn't actually finish addressing this.
7b14abc is described below

commit 7b14abc3b98f0519ec7b271052ddec1dab0b863e
Author: markrmiller@gmail.com <ma...@gmail.com>
AuthorDate: Thu Aug 20 10:25:06 2020 -0500

    @580 Whoops, didn't actually finish addressing this.
---
 .../org/apache/solr/metrics/SolrMetricManager.java | 39 ++++++++++------------
 .../apache/solr/metrics/SolrMetricsContext.java    |  4 +--
 .../solr/handler/admin/StatsReloadRaceTest.java    |  3 +-
 3 files changed, 21 insertions(+), 25 deletions(-)

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 231f472..c05b1c6 100644
--- a/solr/core/src/java/org/apache/solr/metrics/SolrMetricManager.java
+++ b/solr/core/src/java/org/apache/solr/metrics/SolrMetricManager.java
@@ -577,11 +577,15 @@ public class SolrMetricManager {
       for (Map.Entry<String,Metric> entry : metrics.getMetrics().entrySet()) {
         work.collect("registerMetric-" + entry.getKey(), () ->{
           String fullName = mkName(entry.getKey(), metricPath);
-          metricRegistry.remove(fullName);
           try {
             metricRegistry.register(fullName, entry.getValue());
           } catch (IllegalArgumentException e) {
-            log.warn("Metric already registered: " + fullName);
+            metricRegistry.remove(fullName);
+            try {
+              metricRegistry.register(fullName, entry.getValue());
+            } catch (IllegalArgumentException e2) {
+              log.warn("Metric already registered: " + fullName);
+            }
           }
         });
       }
@@ -779,27 +783,20 @@ public class SolrMetricManager {
     MetricRegistry registry = registry(registryName);
     if (registry == null) return 0;
     AtomicInteger removed = new AtomicInteger();
-    registry.removeMatching((name, metric) -> {
-      if (metric instanceof GaugeWrapper) {
-        GaugeWrapper wrapper = (GaugeWrapper) metric;
-        boolean toRemove = wrapper.getTag().contains(tagSegment);
-        if (toRemove) {
-          removed.incrementAndGet();
-        }
-        return toRemove;
-      }
-      return false;
-
-    });
-    return removed.get();
-  }
 
-  public int unregisterGauges(Set<String> names, String registryName) {
-    MetricRegistry registry = registry(registryName);
-    for (String name : names) {
-      registry.remove(name);
+    Set<Map.Entry<String,Metric>> entries = registry.getMetrics().entrySet();
+    for (Map.Entry<String,Metric> entry : entries) {
+      Metric metric = entry.getValue();
+        if (metric instanceof GaugeWrapper) {
+          GaugeWrapper wrapper = (GaugeWrapper) metric;
+          boolean toRemove = wrapper.getTag().contains(tagSegment);
+          if (toRemove) {
+            removed.incrementAndGet();
+            registry.remove(entry.getKey());
+          }
+        }
     }
-    return names.size();
+    return removed.get();
   }
 
   /**
diff --git a/solr/core/src/java/org/apache/solr/metrics/SolrMetricsContext.java b/solr/core/src/java/org/apache/solr/metrics/SolrMetricsContext.java
index a927d68..0f8c4f0 100644
--- a/solr/core/src/java/org/apache/solr/metrics/SolrMetricsContext.java
+++ b/solr/core/src/java/org/apache/solr/metrics/SolrMetricsContext.java
@@ -85,9 +85,7 @@ public class SolrMetricsContext {
    * do so may result in hard-to-debug memory leaks.</b></p>
    */
   public void unregister() {
-    for (String gauge : gaugeNames) {
-      metricManager.unregisterGauges(registryName, tag);
-    }
+    metricManager.unregisterGauges(registryName, tag);
   }
 
   /**
diff --git a/solr/core/src/test/org/apache/solr/handler/admin/StatsReloadRaceTest.java b/solr/core/src/test/org/apache/solr/handler/admin/StatsReloadRaceTest.java
index 32f00d7..b4ae67b 100644
--- a/solr/core/src/test/org/apache/solr/handler/admin/StatsReloadRaceTest.java
+++ b/solr/core/src/test/org/apache/solr/handler/admin/StatsReloadRaceTest.java
@@ -27,6 +27,7 @@ import org.apache.solr.common.params.CoreAdminParams.CoreAdminAction;
 import org.apache.solr.common.util.NamedList;
 import org.apache.solr.response.SolrQueryResponse;
 import org.junit.BeforeClass;
+import org.junit.Ignore;
 import org.junit.Test;
 
 public class StatsReloadRaceTest extends SolrTestCaseJ4 {
@@ -136,7 +137,7 @@ public class StatsReloadRaceTest extends SolrTestCaseJ4 {
         assertTrue(metrics.get(key) instanceof Long);
         break;
       } else {
-        Thread.sleep(50);
+        Thread.sleep(10);
       }
     }
     if (softFail && !found) {