This is an automated email from the ASF dual-hosted git repository.
psalagnac pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/solr.git
The following commit(s) were added to refs/heads/main by this push:
new 28a2ee239a3 SOLR-17777: Fix leaked metric registry after core unload
(#3382)
28a2ee239a3 is described below
commit 28a2ee239a32a58dad36aa14fa5f6a7187740131
Author: Pierre Salagnac <[email protected]>
AuthorDate: Mon Jul 7 11:11:27 2025 +0200
SOLR-17777: Fix leaked metric registry after core unload (#3382)
This fixes a minor memory leak when deleting a core. I don't think it can
have a significant impact on heap, but the fix is trivial and not risky.
---
solr/CHANGES.txt | 2 ++
.../org/apache/solr/metrics/SolrMetricManager.java | 22 +++++++++++--
.../org/apache/solr/cloud/MigrateReplicasTest.java | 3 +-
.../org/apache/solr/cloud/ReplaceNodeTest.java | 3 +-
.../solr/metrics/SolrCoreMetricManagerTest.java | 37 ++++++++++++++++++++++
5 files changed, 62 insertions(+), 5 deletions(-)
diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index 7f67abc2108..f2268dd6934 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -339,6 +339,8 @@ Bug Fixes
* SOLR-17711: Remove total request timeout during recovery that was
inadvertently added. (Luke Kot-Zaniewski)
+* SOLR-17777: Fix a very minor memory leak in metrics reporting code when a
core is deleted. (Pierre Salagnac)
+
Dependency Upgrades
---------------------
* SOLR-17471: Upgrade Lucene to 9.12.1. (Pierre Salagnac, Christine Poerschke)
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 52c468445cb..fa05c2797dc 100644
--- a/solr/core/src/java/org/apache/solr/metrics/SolrMetricManager.java
+++ b/solr/core/src/java/org/apache/solr/metrics/SolrMetricManager.java
@@ -448,19 +448,35 @@ public class SolrMetricManager {
}
/**
- * Get (or create if not present) a named registry
+ * Get (or create if not present) a named registry. This method always
creates and persists a new
+ * registry in case it does not already exist.
*
* @param registry name of the registry
* @return existing or newly created registry
*/
public MetricRegistry registry(String registry) {
+ return registry(registry, true);
+ }
+
+ /**
+ * Get (or create if not present) a named registry.
+ *
+ * @param registry name of the registry.
+ * @param create When false and the registry does not exist, we return null
instead of creating a
+ * new one.
+ */
+ public MetricRegistry registry(String registry, boolean create) {
registry = enforcePrefix(registry);
if (isSharedRegistry(registry)) {
return SharedMetricRegistries.getOrCreate(registry);
} else {
swapLock.lock();
try {
- return getOrCreateRegistry(registries, registry);
+ if (create) {
+ return getOrCreateRegistry(registries, registry);
+ } else {
+ return registries.get(registry);
+ }
} finally {
swapLock.unlock();
}
@@ -830,7 +846,7 @@ public class SolrMetricManager {
if (tagSegment == null) {
return 0;
}
- MetricRegistry registry = registry(registryName);
+ MetricRegistry registry = registry(registryName, false);
if (registry == null) return 0;
AtomicInteger removed = new AtomicInteger();
registry.removeMatching(
diff --git a/solr/core/src/test/org/apache/solr/cloud/MigrateReplicasTest.java
b/solr/core/src/test/org/apache/solr/cloud/MigrateReplicasTest.java
index 9f8993f2ce6..296dcf001a5 100644
--- a/solr/core/src/test/org/apache/solr/cloud/MigrateReplicasTest.java
+++ b/solr/core/src/test/org/apache/solr/cloud/MigrateReplicasTest.java
@@ -199,7 +199,8 @@ public class MigrateReplicasTest extends SolrCloudTestCase {
// check replication metrics on this jetty - see SOLR-14924
for (JettySolrRunner jetty : cluster.getJettySolrRunners()) {
- if (jetty.getCoreContainer() == null) {
+ if (emptyNode.equals(jetty.getNodeName())) {
+ // No cores on this node, ignore it
continue;
}
SolrMetricManager metricManager =
jetty.getCoreContainer().getMetricManager();
diff --git a/solr/core/src/test/org/apache/solr/cloud/ReplaceNodeTest.java
b/solr/core/src/test/org/apache/solr/cloud/ReplaceNodeTest.java
index 59427ff26df..cb3305d92f7 100644
--- a/solr/core/src/test/org/apache/solr/cloud/ReplaceNodeTest.java
+++ b/solr/core/src/test/org/apache/solr/cloud/ReplaceNodeTest.java
@@ -179,7 +179,8 @@ public class ReplaceNodeTest extends SolrCloudTestCase {
// check replication metrics on this jetty - see SOLR-14924
for (JettySolrRunner jetty : cluster.getJettySolrRunners()) {
- if (jetty.getCoreContainer() == null) {
+ if (emptyNode.equals(jetty.getNodeName())) {
+ // No cores on this node, ignore it
continue;
}
SolrMetricManager metricManager =
jetty.getCoreContainer().getMetricManager();
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 3559a665f91..365a997339f 100644
--- a/solr/core/src/test/org/apache/solr/metrics/SolrCoreMetricManagerTest.java
+++ b/solr/core/src/test/org/apache/solr/metrics/SolrCoreMetricManagerTest.java
@@ -23,14 +23,22 @@ import java.io.IOException;
import java.util.HashMap;
import java.util.Map;
import java.util.Random;
+import java.util.Set;
import java.util.stream.Collectors;
import org.apache.lucene.tests.util.TestUtil;
import org.apache.solr.SolrTestCaseJ4;
import org.apache.solr.common.params.CoreAdminParams;
+import org.apache.solr.common.params.MapSolrParams;
+import org.apache.solr.common.params.SolrParams;
+import org.apache.solr.common.params.UpdateParams;
+import org.apache.solr.core.CoreContainer;
import org.apache.solr.core.PluginInfo;
+import org.apache.solr.core.SolrCore;
import org.apache.solr.core.SolrInfoBean;
import org.apache.solr.metrics.reporters.MockMetricReporter;
+import org.apache.solr.request.SolrQueryRequestBase;
import org.apache.solr.schema.FieldType;
+import org.apache.solr.update.CommitUpdateCommand;
import org.junit.After;
import org.junit.Before;
import org.junit.Test;
@@ -177,4 +185,33 @@ public class SolrCoreMetricManagerTest extends
SolrTestCaseJ4 {
assertEquals("solr.core.collection1", registryName);
assertNull(leaderRegistryName);
}
+
+ /** Check the metric registry specific to a core is removed once the core is
unloaded. */
+ @Test
+ public void testNoRegistryAfterUnload() throws Exception {
+
+ String coreRegistryName;
+
+ CoreContainer cc = h.getCoreContainer();
+ SolrMetricManager metricManager = cc.getMetricManager();
+
+ SolrCore core = cc.create("to-unload", Map.of("configSet", "minimal"));
+ coreRegistryName = core.getSolrMetricsContext().getRegistryName();
+
+ Set<String> names = metricManager.registryNames();
+ assertTrue("missing registry", names.contains(coreRegistryName));
+
+ // Commit and wait for searcher to ensure the searcher is created. This is
required to make sure
+ // the core inner thread does not create it *after* we asked the container
to unload the core.
+ SolrParams params = new MapSolrParams(Map.of(UpdateParams.WAIT_SEARCHER,
"true"));
+ core.getUpdateHandler()
+ .commit(new CommitUpdateCommand(new SolrQueryRequestBase(h.getCore(),
params) {}, false));
+
+ cc.unload("to-unload");
+
+ // Make sure the core metric registry does not exist anymore once we fully
removed the core
+ // from the container
+ names = metricManager.registryNames();
+ assertFalse(names.contains(coreRegistryName));
+ }
}
