Skip to content

SAI-6021 : Fix slow node detection metric context leaks #311

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
patsonluk wants to merge 3 commits into
base: fs/branch_9_7
Choose a base branch
from
Open

SAI-6021 : Fix slow node detection metric context leaks #311

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
64f39d3
Fixed the metrics context for slow node detection logic
patsonluk Jan 20, 2026
5c2e3c1
Fixed double expanded scope for SlowNodeDetector
patsonluk Jan 28, 2026
9983ffc
./gradlew tidy
patsonluk Jan 29, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 6 additions & 19 deletions solr/core/src/java/org/apache/solr/handler/component/SlowNodeDetector.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ class SlowNodeDetector implements SolrMetricProducer {
private final int maxSlowResponsePercentage;
private final int minShardCountPerRequest;
private final int slowLatencyThreshold;
private SolrMetricsContext metricsContext;

/**
* @param latencyDropRatioThreshold identify as a latency drop point when current latency is < 0.5
Expand Down Expand Up @@ -213,29 +214,15 @@ public String toString() {

@Override
public void initializeMetrics(SolrMetricsContext parentContext, String scope) {
String nodeRegistry = SolrMetricManager.getRegistryName(SolrInfoBean.Group.node);
SolrMetricManager manager = parentContext.getMetricManager();
manager.registerGauge(
parentContext,
nodeRegistry,
slowNodes::keySet,
parentContext.getTag(),
SolrMetricManager.ResolutionStrategy.REPLACE,
"slowNodes",
scope);
manager.registerGauge(
parentContext,
nodeRegistry,
slowNodes::size,
parentContext.getTag(),
SolrMetricManager.ResolutionStrategy.REPLACE,
"slowNodeCount",
scope);
metricsContext = parentContext.getChildContext(this);

metricsContext.gauge(slowNodes::keySet, true, "slowNodes", scope);
metricsContext.gauge(slowNodes::size, true, "slowNodeCount", scope);
}

@Override
public SolrMetricsContext getSolrMetricsContext() { // using the same context as parent
return null;
return metricsContext;
Copy link

@cursor cursor bot Jan 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Misleading comment on method returning child context

Low Severity

The comment // using the same context as parent in getSolrMetricsContext() is inaccurate. The method now returns a dedicated child metricsContext instead of the parent or null, which could mislead future maintainers.

Fix in Cursor Fix in Web

}

static class Builder {
Expand Down
22 changes: 12 additions & 10 deletions .../core/src/java/org/apache/solr/handler/component/TimeLimitingHttpShardHandlerFactory.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
package org.apache.solr.handler.component;

import com.codahale.metrics.Counter;
import java.io.IOException;
import java.lang.invoke.MethodHandles;
import java.util.concurrent.ExecutorService;
import org.apache.solr.common.util.ExecutorUtil;
Expand Down Expand Up @@ -45,7 +46,6 @@ public class TimeLimitingHttpShardHandlerFactory extends HttpShardHandlerFactory
private static final String DRY_RUN_CONFIG_KEY = "dryRun";

private SlowNodeDetector slowNodeDetector;
private SolrMetricsContext solrMetricsContext;
Counter cancelledSlowNodeRequests;
Counter cancelledDryRunSlowNodeRequests;
private ExecutorService executorService;
Expand Down Expand Up @@ -153,26 +153,28 @@ public void init(PluginInfo info) {
@Override
public void initializeMetrics(SolrMetricsContext parentContext, String scope) {
super.initializeMetrics(parentContext, scope);
solrMetricsContext = parentContext.getChildContext(this);
String expandedScope = SolrMetricManager.mkName(scope, SolrInfoBean.Category.QUERY.name());
cancelledSlowNodeRequests =
solrMetricsContext.counter("cancelledSlowNodeRequests", expandedScope);
getSolrMetricsContext().counter("cancelledSlowNodeRequests", expandedScope);
cancelledDryRunSlowNodeRequests =
solrMetricsContext.counter("cancelledDryRunSlowNodeRequests", expandedScope);
getSolrMetricsContext().counter("cancelledDryRunSlowNodeRequests", expandedScope);

if (slowNodeDetector != null) {
slowNodeDetector.initializeMetrics(solrMetricsContext, expandedScope);
slowNodeDetector.initializeMetrics(getSolrMetricsContext(), expandedScope);
}
}

@Override
public SolrMetricsContext getSolrMetricsContext() {
return solrMetricsContext;
}

@Override
public void close() {
super.close();
if (slowNodeDetector != null) {
try {
slowNodeDetector.close();
} catch (IOException e) {
log.warn("Failed to close the slowNodeDetector", e);
}
}

if (executorService != null) {
ExecutorUtil.shutdownNowAndAwaitTermination(executorService);
}
Expand Down
Loading