Skip to content

Commit

Permalink
Merge pull request #613 from newrelic/#355-solr-filtercache-leak
Browse files Browse the repository at this point in the history 
#355 - Solr FilterCache Memory Leak - Replace SolrInfoBean in NRMetric with String
  • Loading branch information
@meiao
meiao authored Dec 21, 2021
2 parents 0cf1690 + c430d85 commit 7a72ab9
Show file tree
Hide file tree
Showing 14 changed files with 392 additions and 28 deletions.
4 changes: 2 additions & 2 deletions instrumentation/solr-jmx-7.0.0/src/main/java/com/agent/instrumentation/solr/CacheMetric.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ public class CacheMetric extends NRMetric {
MetricsMap metric = null;
String metricType = null;

public CacheMetric(String mt, String r, Metric m, SolrInfoBean b) {
public CacheMetric(String mt, String r, Metric m, String b) {
super(r, b);
metricType = mt;
if (MetricsMap.class.isInstance(m)) {
Expand Down Expand Up @@ -49,7 +49,7 @@ public int reportMetrics() {

@Override
public String getMetricBase() {
return prefix + registry + "/" + metricType + "/" + info.getName();
return prefix + registry + "/" + metricType + "/" + name;
}

}
4 changes: 2 additions & 2 deletions instrumentation/solr-jmx-7.0.0/src/main/java/com/agent/instrumentation/solr/GaugeMetric.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ public class GaugeMetric extends NRMetric {
String metricName;

@SuppressWarnings("rawtypes")
public GaugeMetric(String mn, String mt, String r, Gauge m, SolrInfoBean b) {
public GaugeMetric(String mn, String mt, String r, Gauge m, String b) {
super(r, b);
metric = m;
metricType = mt;
Expand All @@ -33,7 +33,7 @@ public String getMetricName(String name) {

@Override
public String getMetricBase() {
return prefix + registry + "/" + metricType + "/" + info.getName();
return prefix + registry + "/" + metricType + "/" + name;
}

@Override
Expand Down
4 changes: 2 additions & 2 deletions ...umentation/solr-jmx-7.0.0/src/main/java/com/agent/instrumentation/solr/MeteredMetric.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ public class MeteredMetric extends NRMetric {
String metricType;
String metricName;

public MeteredMetric(String mn, String mt, String r, SolrInfoBean b, Metered m) {
public MeteredMetric(String mn, String mt, String r, String b, Metered m) {
super(r, b);
metered = m;
metricType = mt;
Expand All @@ -31,7 +31,7 @@ public String getMetricName(String name) {

@Override
public String getMetricBase() {
return prefix + registry + "/" + metricType + "/" + info.getName();
return prefix + registry + "/" + metricType + "/" + name;
}

@Override
Expand Down
28 changes: 28 additions & 0 deletions instrumentation/solr-jmx-7.0.0/src/main/java/com/agent/instrumentation/solr/MetricUtil.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
package com.agent.instrumentation.solr;

import java.util.ArrayList;
import java.util.Arrays;
import java.util.HashMap;
import java.util.List;
import java.util.StringTokenizer;
Expand Down Expand Up @@ -48,6 +49,33 @@ public static void addMetric(NRMetric metric) {
metrics.put(metricBase, metric);
}

public static void removeMetric(String registry, String... metricPath) {
metrics.entrySet()
.stream()
.filter(entry -> entry.getValue().registry.equals(registry) && Arrays.stream(metricPath).anyMatch(path -> path.startsWith(entry.getValue().name)))
.forEach(x -> metrics.remove(x.getKey()));
}

public static void swapRegistries(String sourceRegistry, String targetRegistry) {
metrics.entrySet()
.stream()
.filter(entry -> entry.getValue().registry.equals(getRegistry(sourceRegistry)))
.forEach(x -> {
String currentKey = x.getKey();
NRMetric metric = x.getValue();
metric.setRegistry(getRegistry(targetRegistry));
addMetric(metric);
metrics.remove(currentKey);
});
}

public static void clearRegistry(String registry) {
metrics.entrySet()
.stream()
.filter(entry -> entry.getValue().registry.equals(registry))
.forEach(x -> metrics.remove(x.getKey()));
}

public static String getRegistry(String r) {
if (r.startsWith(REGISTRY_PREFIX)) {
return r.substring(REGISTRY_PREFIX.length() + 1);
Expand Down
12 changes: 7 additions & 5 deletions instrumentation/solr-jmx-7.0.0/src/main/java/com/agent/instrumentation/solr/NRMetric.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,25 +7,27 @@

package com.agent.instrumentation.solr;

import org.apache.solr.core.SolrInfoBean;

public abstract class NRMetric {

protected static final String prefix = "JMX/solr/";

public NRMetric(String r, SolrInfoBean b) {
public NRMetric(String r, String b) {
registry = r;
info = b;
name = b;
}

protected String registry;

protected SolrInfoBean info;
protected String name;

public String getRegistry() {
return registry;
}

public void setRegistry(String registry) {
this.registry = registry;
}

public abstract String getMetricName(String name);

public abstract int reportMetrics();
Expand Down
35 changes: 32 additions & 3 deletions ...lr-jmx-7.0.0/src/main/java/org/apache/solr/metrics/SolrMetricManager_Instrumentation.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import com.newrelic.api.agent.weaver.Weaver;
import org.apache.solr.core.SolrInfoBean;

import java.util.Set;
import java.util.logging.Level;

@Weave(originalName = "org.apache.solr.metrics.SolrMetricManager", type = MatchType.ExactClass)
Expand All @@ -37,12 +38,12 @@ public void register(SolrInfoBean info, String registry, Metric metric, boolean

if (isCacheMetric) {
MetricsMap mMap = (MetricsMap) metric;
CacheMetric nrMetric = new CacheMetric(desired, MetricUtil.getRegistry(registry), mMap, info);
CacheMetric nrMetric = new CacheMetric(desired, MetricUtil.getRegistry(registry), mMap, info.getName());
NewRelic.getAgent().getLogger().log(Level.FINEST, "Created CacheMetric of name {0}", metricName);
MetricUtil.addMetric(nrMetric);
} else if (isGaugeMetric) {
Gauge gauge = (Gauge) metric;
GaugeMetric gMetric = new GaugeMetric(metricName, desired, MetricUtil.getRegistry(registry), gauge, info);
GaugeMetric gMetric = new GaugeMetric(metricName, desired, MetricUtil.getRegistry(registry), gauge, info.getName());
NewRelic.getAgent().getLogger().log(Level.FINEST, "Created GaugeMetric of name {0}", metricName);
MetricUtil.addMetric(gMetric);
}
Expand All @@ -56,11 +57,39 @@ public Meter meter(SolrInfoBean info, String registry, String metricName, String
if (MetricUtil.isDesired(metricName, metricPath)) {
String mName = MetricUtil.getRemap(metricName);
String desired = MetricUtil.getDesired(metricName, metricPath);
MeteredMetric meteredMetric = new MeteredMetric(mName, desired, MetricUtil.getRegistry(registry), info, meter);
MeteredMetric meteredMetric = new MeteredMetric(mName, desired, MetricUtil.getRegistry(registry), info.getName(), meter);
MetricUtil.addMetric(meteredMetric);
NewRelic.getAgent().getLogger().log(Level.FINEST, "Added NRMetric from ({0}, {1}, {2})", info, registry, metricName);
}
return meter;
}

public void removeRegistry(String registry) {
Weaver.callOriginal();
MetricUtil.clearRegistry(registry);
NewRelic.getAgent().getLogger().log(Level.FINEST, "Removed {0} metric registry", registry);
}

public void clearRegistry(String registry) {
Weaver.callOriginal();
MetricUtil.clearRegistry(registry);
NewRelic.getAgent().getLogger().log(Level.FINEST, "Cleared {0} metric registry", registry);
}

public Set<String> clearMetrics(String registry, String... metricPath) {
Set<String> removedMetrics = Weaver.callOriginal();
if(removedMetrics != null) {
for (String removedMetric: removedMetrics) {
MetricUtil.removeMetric(registry, removedMetric);
}
NewRelic.getAgent().getLogger().log(Level.FINEST, "Cleared {0} metrics from {1} metric registry", removedMetrics.size(), registry);
}
return removedMetrics;
}

public void swapRegistries(String registry1, String registry2) {
Weaver.callOriginal();
MetricUtil.swapRegistries(registry1, registry2);
NewRelic.getAgent().getLogger().log(Level.FINEST, "Swapped {0} metric registry to {1} metric registry", registry1, registry2);
}
}
121 changes: 121 additions & 0 deletions ...0/src/test/java/com/agent/instrumentation/solr/SolrMetricManagerInstrumentationTests.java
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,121 @@
package com.agent.instrumentation.solr;

import com.newrelic.agent.bridge.AgentBridge;
import com.newrelic.agent.bridge.NoOpPrivateApi;
import com.newrelic.agent.introspec.InstrumentationTestConfig;
import com.newrelic.agent.introspec.InstrumentationTestRunner;

import org.apache.solr.core.SolrInfoBean;
import org.apache.solr.metrics.MetricsMap;
import org.apache.solr.metrics.SolrMetricManager;
import org.apache.solr.store.blockcache.Metrics;

import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;

import java.util.stream.Collectors;

import static org.junit.Assert.assertEquals;


@RunWith(InstrumentationTestRunner.class)
@InstrumentationTestConfig(includePrefixes = { "org.apache.solr.metrics" })
public class SolrMetricManagerInstrumentationTests {

@Before
public void before() {
MetricUtil.clearRegistry("exampleRegistry");
MetricUtil.clearRegistry("targetRegistry");
}

@Test
public void register() {
//Given
AgentBridge.privateApi = new NoOpPrivateApi();
SolrMetricManager solrMetricManager = new SolrMetricManager();
SolrInfoBean solrInfoBean = new Metrics();
MetricsMap metricsMap = new MetricsMap((detailed, map) -> map.put("exampleMetric", 100));

//When
solrMetricManager.register(solrInfoBean, "exampleRegistry", metricsMap, false,"filterCache", "hdfsBlockCache");

//Then
int metricsReported = MetricUtil.getMetrics().values().stream().map(NRMetric::reportMetrics).mapToInt(Integer::intValue).sum();
assertEquals(1, metricsReported);

String registryName = MetricUtil.getMetrics().values().stream().map(x -> x.registry).collect(Collectors.joining());
assertEquals("exampleRegistry", registryName);
}

@Test
public void removeRegistry() {
//Given
AgentBridge.privateApi = new NoOpPrivateApi();
SolrMetricManager solrMetricManager = new SolrMetricManager();
SolrInfoBean solrInfoBean = new Metrics();
MetricsMap metricsMap = new MetricsMap((detailed, map) -> map.put("exampleMetric", 100));

//When
solrMetricManager.register(solrInfoBean, "exampleRegistry", metricsMap, false,"filterCache", "hdfsBlockCache");
solrMetricManager.removeRegistry("exampleRegistry");

//Then
int metricsReported = MetricUtil.getMetrics().values().stream().map(NRMetric::reportMetrics).mapToInt(Integer::intValue).sum();
assertEquals(0, metricsReported);
}

@Test
public void clearRegistry() {
//Given
AgentBridge.privateApi = new NoOpPrivateApi();
SolrMetricManager solrMetricManager = new SolrMetricManager();
SolrInfoBean solrInfoBean = new Metrics();
MetricsMap metricsMap = new MetricsMap((detailed, map) -> map.put("exampleMetric", 100));

//When
solrMetricManager.register(solrInfoBean, "exampleRegistry", metricsMap, false,"filterCache", "hdfsBlockCache");
solrMetricManager.clearRegistry("exampleRegistry");

//Then
int metricsReported = MetricUtil.getMetrics().values().stream().map(NRMetric::reportMetrics).mapToInt(Integer::intValue).sum();
assertEquals(0, metricsReported);
}

@Test
public void clearMetrics() {
//Given
AgentBridge.privateApi = new NoOpPrivateApi();
SolrMetricManager solrMetricManager = new SolrMetricManager();
SolrInfoBean solrInfoBean = new Metrics();
MetricsMap metricsMap = new MetricsMap((detailed, map) -> map.put("exampleMetric", 100));

//When
solrMetricManager.register(solrInfoBean, "exampleRegistry", metricsMap, false,"filterCache", "hdfsBlockCache");
solrMetricManager.clearMetrics("exampleRegistry", "hdfsBlockCache");

//Then
int metricsReported = MetricUtil.getMetrics().values().stream().map(NRMetric::reportMetrics).mapToInt(Integer::intValue).sum();
assertEquals(0, metricsReported);
}

@Test
public void swapRegistries() {
//Given
AgentBridge.privateApi = new NoOpPrivateApi();
SolrMetricManager solrMetricManager = new SolrMetricManager();
SolrInfoBean solrInfoBean = new Metrics();
MetricsMap metricsMap = new MetricsMap((detailed, map) -> map.put("exampleMetric", 100));

//When
solrMetricManager.register(solrInfoBean, "exampleRegistry", metricsMap, false,"filterCache", "hdfsBlockCache");
solrMetricManager.swapRegistries("exampleRegistry", "targetRegistry");

//Then
int metricsReported = MetricUtil.getMetrics().values().stream().map(NRMetric::reportMetrics).mapToInt(Integer::intValue).sum();
assertEquals(1, metricsReported);

String registryName = MetricUtil.getMetrics().values().stream().map(x -> x.registry).collect(Collectors.joining());
assertEquals("targetRegistry", registryName);
}
}
4 changes: 2 additions & 2 deletions instrumentation/solr-jmx-7.4.0/src/main/java/com/agent/instrumentation/solr/CacheMetric.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ public class CacheMetric extends NRMetric {
MetricsMap metric = null;
String metricType = null;

public CacheMetric(String mt, String r, Metric m, SolrInfoBean b) {
public CacheMetric(String mt, String r, Metric m, String b) {
super(r, b);
metricType = mt;
if (MetricsMap.class.isInstance(m)) {
Expand Down Expand Up @@ -49,7 +49,7 @@ public int reportMetrics() {

@Override
public String getMetricBase() {
return prefix + registry + "/" + metricType + "/" + info.getName();
return prefix + registry + "/" + metricType + "/" + name;
}

}
4 changes: 2 additions & 2 deletions instrumentation/solr-jmx-7.4.0/src/main/java/com/agent/instrumentation/solr/GaugeMetric.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ public class GaugeMetric extends NRMetric {
String metricName;

@SuppressWarnings("rawtypes")
public GaugeMetric(String mn, String mt, String r, Gauge m, SolrInfoBean b) {
public GaugeMetric(String mn, String mt, String r, Gauge m, String b) {
super(r, b);
metric = m;
metricType = mt;
Expand All @@ -33,7 +33,7 @@ public String getMetricName(String name) {

@Override
public String getMetricBase() {
return prefix + registry + "/" + metricType + "/" + info.getName();
return prefix + registry + "/" + metricType + "/" + name;
}

@Override
Expand Down
4 changes: 2 additions & 2 deletions ...umentation/solr-jmx-7.4.0/src/main/java/com/agent/instrumentation/solr/MeteredMetric.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ public class MeteredMetric extends NRMetric {
String metricType;
String metricName;

public MeteredMetric(String mn, String mt, String r, SolrInfoBean b, Metered m) {
public MeteredMetric(String mn, String mt, String r, String b, Metered m) {
super(r, b);
metered = m;
metricType = mt;
Expand All @@ -31,7 +31,7 @@ public String getMetricName(String name) {

@Override
public String getMetricBase() {
return prefix + registry + "/" + metricType + "/" + info.getName();
return prefix + registry + "/" + metricType + "/" + name;
}

@Override
Expand Down
Loading

0 comments on commit 7a72ab9

Please sign in to comment.