This is an automated email from the ASF dual-hosted git repository.
markrmiller pushed a commit to branch branch_9x
in repository https://gitbox.apache.org/repos/asf/solr.git
The following commit(s) were added to refs/heads/branch_9x by this push:
new 636880bbf88 SOLR-18284: Load-average and memory circuit breakers no
longer stampede or trip on transient pre-GC heap peaks (#4550)
636880bbf88 is described below
commit 636880bbf883e01039c6cff831b0ec02a4566fe5
Author: Mark Robert Miller <[email protected]>
AuthorDate: Thu Jun 25 11:05:21 2026 -0500
SOLR-18284: Load-average and memory circuit breakers no longer stampede or
trip on transient pre-GC heap peaks (#4550)
---
...R-18284-loadaverage-circuit-breaker-caching.yml | 8 +
.../SOLR-18284-memory-circuit-breaker-post-gc.yml | 17 ++
.../circuitbreaker/AveragingMetricProvider.java | 80 ---------
.../solr/util/circuitbreaker/CircuitBreaker.java | 18 ++
.../circuitbreaker/CircuitBreakerRegistry.java | 9 +
.../circuitbreaker/LoadAverageCircuitBreaker.java | 11 +-
.../util/circuitbreaker/MemoryCircuitBreaker.java | 197 ++++++++++++++-------
.../solr/util/circuitbreaker/TtlSampledMetric.java | 107 +++++++++++
.../apache/solr/util/BaseTestCircuitBreaker.java | 8 +-
.../TestHeapPressureCircuitBreakers.java | 111 ++++++++++++
.../TestLoadAverageCircuitBreakerSampling.java | 175 ++++++++++++++++++
.../deployment-guide/pages/circuit-breakers.adoc | 15 ++
12 files changed, 611 insertions(+), 145 deletions(-)
diff --git
a/changelog/unreleased/SOLR-18284-loadaverage-circuit-breaker-caching.yml
b/changelog/unreleased/SOLR-18284-loadaverage-circuit-breaker-caching.yml
new file mode 100644
index 00000000000..e2e87bce365
--- /dev/null
+++ b/changelog/unreleased/SOLR-18284-loadaverage-circuit-breaker-caching.yml
@@ -0,0 +1,8 @@
+# See https://github.com/apache/solr/blob/main/dev-docs/changelog.adoc
+title: LoadAverageCircuitBreaker now caches its sampled value for a short TTL,
so it stops re-polling the OS load average per request at high RPS.
+type: fixed
+authors:
+ - name: Mark Robert Miller
+links:
+ - name: SOLR-18284
+ url: https://issues.apache.org/jira/browse/SOLR-18284
diff --git a/changelog/unreleased/SOLR-18284-memory-circuit-breaker-post-gc.yml
b/changelog/unreleased/SOLR-18284-memory-circuit-breaker-post-gc.yml
new file mode 100644
index 00000000000..8a62c54a12b
--- /dev/null
+++ b/changelog/unreleased/SOLR-18284-memory-circuit-breaker-post-gc.yml
@@ -0,0 +1,17 @@
+# See https://github.com/apache/solr/blob/main/dev-docs/changelog.adoc
+title: >
+ MemoryCircuitBreaker now measures post-GC live heap data, so it no longer
trips when the heap is
+ full of collectible garbage.
+ Earlier versions sampled MemoryMXBean.getHeapMemoryUsage().getUsed() on a
30-second moving average
+ (6 samples), which trends toward max between collections during normal
operation; if you tuned a
+ threshold against that behavior you may need to revisit it.
+ This breaks subclasses of MemoryCircuitBreaker: the deprecated
MemoryCircuitBreaker(int, int)
+ constructor is removed, and the protected getAvgMemoryUsage() hook is
renamed to
+ getCurrentMemoryUsage() (it no longer averages anything). The now-unused
public
+ AveragingMetricProvider class is also removed.
+type: fixed
+authors:
+ - name: Mark Robert Miller
+links:
+ - name: SOLR-18284
+ url: https://issues.apache.org/jira/browse/SOLR-18284
diff --git
a/solr/core/src/java/org/apache/solr/util/circuitbreaker/AveragingMetricProvider.java
b/solr/core/src/java/org/apache/solr/util/circuitbreaker/AveragingMetricProvider.java
deleted file mode 100644
index 60161e98181..00000000000
---
a/solr/core/src/java/org/apache/solr/util/circuitbreaker/AveragingMetricProvider.java
+++ /dev/null
@@ -1,80 +0,0 @@
-/*
- * Licensed to the Apache Software Foundation (ASF) under one or more
- * contributor license agreements. See the NOTICE file distributed with
- * this work for additional information regarding copyright ownership.
- * The ASF licenses this file to You under the Apache License, Version 2.0
- * (the "License"); you may not use this file except in compliance with
- * the License. You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-package org.apache.solr.util.circuitbreaker;
-
-import com.google.common.util.concurrent.AtomicDouble;
-import java.io.Closeable;
-import java.util.concurrent.Executors;
-import java.util.concurrent.ScheduledExecutorService;
-import java.util.concurrent.TimeUnit;
-import org.apache.solr.common.util.ExecutorUtil;
-import org.apache.solr.common.util.SolrNamedThreadFactory;
-import org.apache.solr.logging.CircularList;
-
-/** Averages the metric value over a period of time */
-public class AveragingMetricProvider implements Closeable {
- private final CircularList<Double> samplesRingBuffer;
- private ScheduledExecutorService executor;
- private final AtomicDouble currentAverageValue = new AtomicDouble(-1);
-
- /**
- * Creates an instance with an executor that runs every sampleInterval
seconds and averages over
- * numSamples samples.
- *
- * @param metricProvider metric provider that will provide a value
- * @param numSamples number of samples to calculate average for
- * @param sampleInterval interval between each sample
- */
- public AveragingMetricProvider(
- MetricProvider metricProvider, int numSamples, long sampleInterval) {
- this.samplesRingBuffer = new CircularList<>(numSamples);
- executor =
- Executors.newSingleThreadScheduledExecutor(
- new SolrNamedThreadFactory(
- "AveragingMetricProvider-" +
metricProvider.getClass().getSimpleName()));
- executor.scheduleWithFixedDelay(
- () -> {
- samplesRingBuffer.add(metricProvider.getMetricValue());
- currentAverageValue.set(
- samplesRingBuffer.toList().stream()
- .mapToDouble(Double::doubleValue)
- .average()
- .orElse(-1));
- },
- 0,
- sampleInterval,
- TimeUnit.SECONDS);
- }
-
- /**
- * Return current average. This is a cached value, so calling this method
will not incur any
- * calculations
- */
- public double getMetricValue() {
- return currentAverageValue.get();
- }
-
- @Override
- public void close() {
- ExecutorUtil.shutdownAndAwaitTermination(executor);
- }
-
- /** Interface to provide the metric value. */
- public interface MetricProvider {
- double getMetricValue();
- }
-}
diff --git
a/solr/core/src/java/org/apache/solr/util/circuitbreaker/CircuitBreaker.java
b/solr/core/src/java/org/apache/solr/util/circuitbreaker/CircuitBreaker.java
index cfbe0bb0cc3..9c6228acd2d 100644
--- a/solr/core/src/java/org/apache/solr/util/circuitbreaker/CircuitBreaker.java
+++ b/solr/core/src/java/org/apache/solr/util/circuitbreaker/CircuitBreaker.java
@@ -25,6 +25,7 @@ import java.util.Set;
import java.util.stream.Collectors;
import org.apache.solr.client.solrj.SolrRequest.SolrRequestType;
import org.apache.solr.common.SolrException;
+import org.apache.solr.common.util.EnvUtils;
import org.apache.solr.common.util.NamedList;
import org.apache.solr.util.SolrPluginUtils;
import org.apache.solr.util.plugin.NamedListInitializedPlugin;
@@ -122,4 +123,21 @@ public abstract class CircuitBreaker implements
NamedListInitializedPlugin, Clos
return SolrException.ErrorCode.TOO_MANY_REQUESTS;
}
}
+
+ /**
+ * Creates a per-instance {@link TtlSampledMetric} for a breaker's expensive
sample (OS load
+ * average, post-GC heap-pool walk). The TTL comes from {@link
+ * CircuitBreakerRegistry#SYSPROP_SAMPLE_TTL_MS} (default {@value
+ * CircuitBreakerRegistry#DEFAULT_SAMPLE_TTL_MS} ms) and is read once here,
at construction.
+ *
+ * <p>Per-instance rather than shared: breakers are subclassed (notably in
tests) to override the
+ * sampler, and a single shared cache would hand one instance's sample to
the others. The TTL
+ * still bounds the underlying sample rate per instance.
+ */
+ protected static <T> TtlSampledMetric<T> newSampleCache() {
+ return new TtlSampledMetric<>(
+ EnvUtils.getPropertyAsLong(
+ CircuitBreakerRegistry.SYSPROP_SAMPLE_TTL_MS,
+ CircuitBreakerRegistry.DEFAULT_SAMPLE_TTL_MS));
+ }
}
diff --git
a/solr/core/src/java/org/apache/solr/util/circuitbreaker/CircuitBreakerRegistry.java
b/solr/core/src/java/org/apache/solr/util/circuitbreaker/CircuitBreakerRegistry.java
index 075aefc23dd..16b2f9c752d 100644
---
a/solr/core/src/java/org/apache/solr/util/circuitbreaker/CircuitBreakerRegistry.java
+++
b/solr/core/src/java/org/apache/solr/util/circuitbreaker/CircuitBreakerRegistry.java
@@ -62,6 +62,15 @@ public class CircuitBreakerRegistry implements Closeable {
public static final String SYSPROP_QUERY_LOADAVG = SYSPROP_PREFIX +
"query.loadavg";
public static final String SYSPROP_WARN_ONLY_SUFFIX = ".warnonly";
+ /**
+ * Default TTL (ms) of cached load-average / heap samples consulted by {@link
+ * LoadAverageCircuitBreaker} and {@link MemoryCircuitBreaker}. Override
per-process via the
+ * {@value #SYSPROP_SAMPLE_TTL_MS} system property.
+ */
+ public static final long DEFAULT_SAMPLE_TTL_MS = 1000L;
+
+ public static final String SYSPROP_SAMPLE_TTL_MS = SYSPROP_PREFIX +
"sample.ttl.ms";
+
private static boolean globalsInitialized = false;
private static final Map<SolrRequestType, List<CircuitBreaker>>
globalCircuitBreakerMap =
new ConcurrentHashMap<>();
diff --git
a/solr/core/src/java/org/apache/solr/util/circuitbreaker/LoadAverageCircuitBreaker.java
b/solr/core/src/java/org/apache/solr/util/circuitbreaker/LoadAverageCircuitBreaker.java
index 64b786aa533..a9513539aa5 100644
---
a/solr/core/src/java/org/apache/solr/util/circuitbreaker/LoadAverageCircuitBreaker.java
+++
b/solr/core/src/java/org/apache/solr/util/circuitbreaker/LoadAverageCircuitBreaker.java
@@ -31,6 +31,11 @@ import org.slf4j.LoggerFactory;
* uses that data to take a decision. We depend on OperatingSystemMXBean which
does not allow a
* configurable interval of collection of data.
*
+ * <p>Because the OS load average is a one-minute moving average, polling the
metric per request is
+ * wasted work. Successive {@link #isTripped()} invocations reuse a sample for
{@link
+ * CircuitBreakerRegistry#SYSPROP_SAMPLE_TTL_MS} (default {@value
+ * CircuitBreakerRegistry#DEFAULT_SAMPLE_TTL_MS} ms).
+ *
* <p>This Circuit breaker is dependent on the operating system, and may
typically not work on
* Microsoft Windows.
*/
@@ -41,6 +46,10 @@ public class LoadAverageCircuitBreaker extends
CircuitBreaker {
private double loadAverageThreshold;
+ // Per-instance (not shared): tests subclass this breaker and override
calculateLiveLoadAverage().
+ // See CircuitBreaker#newSampleCache().
+ private final TtlSampledMetric<Double> loadAverageCache = newSampleCache();
+
// Assumption -- the value of these parameters will be set correctly before
invoking
// getDebugInfo()
private static final ThreadLocal<Double> seenLoadAverage =
ThreadLocal.withInitial(() -> 0.0);
@@ -54,7 +63,7 @@ public class LoadAverageCircuitBreaker extends CircuitBreaker
{
@Override
public boolean isTripped() {
double localAllowedLoadAverage = getLoadAverageThreshold();
- double localSeenLoadAverage = calculateLiveLoadAverage();
+ double localSeenLoadAverage =
loadAverageCache.get(this::calculateLiveLoadAverage);
if (localSeenLoadAverage < 0) {
if (log.isWarnEnabled()) {
diff --git
a/solr/core/src/java/org/apache/solr/util/circuitbreaker/MemoryCircuitBreaker.java
b/solr/core/src/java/org/apache/solr/util/circuitbreaker/MemoryCircuitBreaker.java
index 3888283589e..a5ae8731283 100644
---
a/solr/core/src/java/org/apache/solr/util/circuitbreaker/MemoryCircuitBreaker.java
+++
b/solr/core/src/java/org/apache/solr/util/circuitbreaker/MemoryCircuitBreaker.java
@@ -17,75 +17,134 @@
package org.apache.solr.util.circuitbreaker;
-import java.io.IOException;
import java.lang.invoke.MethodHandles;
import java.lang.management.ManagementFactory;
import java.lang.management.MemoryMXBean;
+import java.lang.management.MemoryPoolMXBean;
+import java.lang.management.MemoryType;
+import java.lang.management.MemoryUsage;
+import java.util.ArrayList;
+import java.util.List;
import java.util.Locale;
-import org.apache.solr.util.RefCounted;
+import java.util.regex.Pattern;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
/**
- * Tracks the current JVM heap usage and triggers if a moving heap usage
average over 30 seconds
- * exceeds the defined percentage of the maximum heap size allocated to the
JVM. Once the average
- * memory usage goes below the threshold, it will start allowing queries again.
+ * Trips when post-GC live heap data exceeds a configured percentage of the
maximum heap size.
*
- * <p>The memory threshold is defined as a percentage of the maximum memory
allocated -- see
- * memThreshold in <code>solrconfig.xml</code>.
+ * <p>"Live data" means heap occupancy measured immediately after a garbage
collection, read from
+ * {@link MemoryPoolMXBean#getCollectionUsage()} on the old/tenured heap pool.
This is the only heap
+ * reading that distinguishes live data from garbage waiting to be collected,
and reading it is the
+ * point of this breaker: raw {@link MemoryMXBean#getHeapMemoryUsage()}
reports whole-heap occupancy
+ * including uncollected garbage, which on a generational collector climbs
toward {@code max}
+ * between collections during normal operation. That raw value can sit at 99%
one second before a
+ * harmless GC drops it to 10%, so measuring it trips the breaker on transient
pre-GC peaks the next
+ * collection is about to reclaim. Post-GC live data is what lets the breaker
answer "is the heap
+ * actually full?" instead of "how much garbage has piled up since the last
GC?".
+ *
+ * <p>Pool selection by collector. {@code getCollectionUsage()} is per-pool,
so the breaker must
+ * pick the pool that holds long-lived data — the old/tenured generation — and
collectors name it
+ * differently:
+ *
+ * <ul>
+ * <li><b>G1 / Parallel / Serial / generational ZGC:</b> uses the pool whose
name matches the
+ * word-boundary pattern {@code \b(Old|Tenured)\b} (e.g. {@code "G1 Old
Gen"}, {@code "PS Old
+ * Gen"}, {@code "Tenured Gen"}, {@code "ZGC Old Generation"}).
+ * <li><b>Non-generational ZGC and Shenandoah:</b> a single combined heap
pool with no old-gen
+ * name to match — the breaker sums {@code getCollectionUsage()} across
every {@link
+ * MemoryType#HEAP} pool instead.
+ * </ul>
+ *
+ * <p>Pre-first-GC, {@link MemoryPoolMXBean#getCollectionUsage()} can return
{@code null} on every
+ * pool; in that case the breaker reports {@code 0} live bytes and will not
trip until the JVM has
+ * performed at least one collection on a heap pool.
+ *
+ * <p>Configure a percentage of the maximum heap size; the breaker trips when
post-GC live data
+ * exceeds that percentage.
*/
public class MemoryCircuitBreaker extends CircuitBreaker {
private static final Logger log =
LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+
private static final MemoryMXBean MEMORY_MX_BEAN =
ManagementFactory.getMemoryMXBean();
- // One shared provider / executor for all instances of this class
- private static RefCounted<AveragingMetricProvider> averagingMetricProvider;
+
+ /**
+ * Word-boundary match for the old/tenured generation pool name across
HotSpot collectors.
+ * Examples that match: {@code "G1 Old Gen"}, {@code "PS Old Gen"}, {@code
"Tenured Gen"}, {@code
+ * "ZGC Old Generation"}. Word boundaries prevent false positives such as a
hypothetical pool
+ * literally named {@code "ColdCache"}. Non-generational ZGC and Shenandoah
expose a single
+ * combined heap pool whose name matches none of these — the fallback path
sums every HEAP pool
+ * instead.
+ */
+ private static final Pattern OLD_GEN_NAME =
Pattern.compile("\\b(Old|Tenured)\\b");
+
+ /**
+ * Lazily-initialized snapshot of the JVM heap pools. Pool resolution is
deferred to the first
+ * call to {@link #samplePostGcLiveBytes()} — i.e. the first request after
the breaker is in use —
+ * rather than performed at class load. This avoids any risk that a
collector lazily creates its
+ * old-generation pool after Solr's class loader has already touched {@link
MemoryCircuitBreaker}
+ * but before the GC subsystem has fully initialized. Once resolved the
snapshot is stable for the
+ * life of the JVM (the {@code MemoryPoolMXBean} list is fixed).
+ */
+ private static final class HeapPools {
+ static final List<MemoryPoolMXBean> OLD_GEN;
+ static final List<MemoryPoolMXBean> ALL_HEAP;
+
+ static {
+ List<MemoryPoolMXBean> oldGen = new ArrayList<>(2);
+ List<MemoryPoolMXBean> allHeap = new ArrayList<>(4);
+ List<String> heapNames = new ArrayList<>(4);
+ for (MemoryPoolMXBean pool : ManagementFactory.getMemoryPoolMXBeans()) {
+ if (pool.getType() != MemoryType.HEAP) {
+ continue;
+ }
+ allHeap.add(pool);
+ String name = pool.getName();
+ heapNames.add(name);
+ if (name != null && OLD_GEN_NAME.matcher(name).find()) {
+ oldGen.add(pool);
+ }
+ }
+ OLD_GEN = List.copyOf(oldGen);
+ ALL_HEAP = List.copyOf(allHeap);
+ if (OLD_GEN.isEmpty() && ALL_HEAP.size() > 1) {
+ // Recognized non-generational collectors (non-generational ZGC,
Shenandoah) expose a
+ // single combined heap pool, so the fallback sum is just that one
pool. More than one heap
+ // pool with no old-gen name is an unrecognized layout: summing
post-GC usage across all of
+ // them can include young-generation data and over-report live heap,
which would make the
+ // breaker trip on garbage the next GC reclaims. Warn so it is
diagnosable rather than
+ // silent.
+ log.warn(
+ "No heap pool name matched the old/tenured pattern {} but the JVM
exposes {} heap "
+ + "pools {}; MemoryCircuitBreaker will sum post-GC usage
across all of them, which "
+ + "can over-report live heap on this unrecognized collector
layout.",
+ OLD_GEN_NAME.pattern(),
+ ALL_HEAP.size(),
+ heapNames);
+ }
+ }
+ }
private long heapMemoryThreshold;
+ // Per-instance (not shared): tests subclass this breaker and override
getCurrentMemoryUsage() (or
+ // samplePostGcLiveBytes()). See CircuitBreaker#newSampleCache().
+ private final TtlSampledMetric<Long> heapLiveCache = newSampleCache();
+
private static final ThreadLocal<Long> seenMemory =
ThreadLocal.withInitial(() -> 0L);
private static final ThreadLocal<Long> allowedMemory =
ThreadLocal.withInitial(() -> 0L);
- /** Creates an instance which averages over 6 samples during last 30
seconds. */
public MemoryCircuitBreaker() {
- this(6, 5);
- }
-
- /**
- * Constructor that allows override of sample interval for which the memory
usage is fetched. This
- * is provided for testing, not intended for general use because the average
metric provider
- * implementation is the same for all instances of the class.
- *
- * @param numSamples number of samples to calculate average for
- * @param sampleInterval interval between each sample
- */
- protected MemoryCircuitBreaker(int numSamples, int sampleInterval) {
super();
- synchronized (MemoryCircuitBreaker.class) {
- if (averagingMetricProvider == null ||
averagingMetricProvider.getRefcount() == 0) {
- averagingMetricProvider =
- new RefCounted<>(
- new AveragingMetricProvider(
- () -> MEMORY_MX_BEAN.getHeapMemoryUsage().getUsed(),
- numSamples,
- sampleInterval)) {
- @Override
- protected void close() {
- get().close();
- }
- };
- }
- averagingMetricProvider.incref();
- }
}
public MemoryCircuitBreaker setThreshold(double thresholdValueInPercentage) {
long currentMaxHeap = MEMORY_MX_BEAN.getHeapMemoryUsage().getMax();
-
if (currentMaxHeap <= 0) {
throw new IllegalArgumentException("Invalid JVM state for the max heap
usage");
}
- double thresholdInFraction = thresholdValueInPercentage / (double) 100;
+ double thresholdInFraction = thresholdValueInPercentage / 100.0;
heapMemoryThreshold = (long) (currentMaxHeap * thresholdInFraction);
if (heapMemoryThreshold <= 0) {
@@ -96,19 +155,50 @@ public class MemoryCircuitBreaker extends CircuitBreaker {
@Override
public boolean isTripped() {
-
- long localAllowedMemory = getCurrentMemoryThreshold();
- long localSeenMemory = getAvgMemoryUsage();
+ long localAllowedMemory = heapMemoryThreshold;
+ long localSeenMemory = getCurrentMemoryUsage();
allowedMemory.set(localAllowedMemory);
-
seenMemory.set(localSeenMemory);
- return (localSeenMemory >= localAllowedMemory);
+ return localSeenMemory >= localAllowedMemory;
}
- protected long getAvgMemoryUsage() {
- return (long) averagingMetricProvider.get().getMetricValue();
+ /**
+ * Returns the post-GC live heap bytes used by {@link #isTripped()}, cached
for {@link
+ * CircuitBreakerRegistry#SYSPROP_SAMPLE_TTL_MS} ms so high-QPS callers
don't repeatedly walk the
+ * heap pool list.
+ *
+ * <p>Tests that inject a synthetic value typically override this method
directly (which bypasses
+ * the cache); tests that want to feed a synthetic sample <em>through</em>
the cache should
+ * override {@link #samplePostGcLiveBytes()} instead.
+ */
+ protected long getCurrentMemoryUsage() {
+ return heapLiveCache.get(this::samplePostGcLiveBytes);
+ }
+
+ /**
+ * Sum of {@code getCollectionUsage().getUsed()} across the old/tenured heap
pool, falling back to
+ * the union of all {@link MemoryType#HEAP} pools when no pool name matches
{@link #OLD_GEN_NAME}
+ * (non-generational ZGC, Shenandoah). Pool resolution is deferred to the
first call to this
+ * method — see {@link HeapPools}.
+ *
+ * <p>Visible for subclassing in tests that want to feed a synthetic sample
through the TTL cache.
+ * Most tests override {@link #getCurrentMemoryUsage()} instead and bypass
the cache entirely.
+ *
+ * @return post-GC live bytes, or {@code 0} if no GC has yet run on a heap
pool
+ */
+ protected long samplePostGcLiveBytes() {
+ List<MemoryPoolMXBean> pools =
+ HeapPools.OLD_GEN.isEmpty() ? HeapPools.ALL_HEAP : HeapPools.OLD_GEN;
+ long total = 0;
+ for (MemoryPoolMXBean pool : pools) {
+ MemoryUsage cu = pool.getCollectionUsage();
+ if (cu != null) {
+ total += cu.getUsed();
+ }
+ }
+ return total;
}
@Override
@@ -120,19 +210,6 @@ public class MemoryCircuitBreaker extends CircuitBreaker {
+ allowedMemory.get();
}
- private long getCurrentMemoryThreshold() {
- return heapMemoryThreshold;
- }
-
- @Override
- public void close() throws IOException {
- synchronized (MemoryCircuitBreaker.class) {
- if (averagingMetricProvider != null &&
averagingMetricProvider.getRefcount() > 0) {
- averagingMetricProvider.decref();
- }
- }
- }
-
@Override
public String toString() {
return String.format(
diff --git
a/solr/core/src/java/org/apache/solr/util/circuitbreaker/TtlSampledMetric.java
b/solr/core/src/java/org/apache/solr/util/circuitbreaker/TtlSampledMetric.java
new file mode 100644
index 00000000000..1bd56e059fd
--- /dev/null
+++
b/solr/core/src/java/org/apache/solr/util/circuitbreaker/TtlSampledMetric.java
@@ -0,0 +1,107 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.solr.util.circuitbreaker;
+
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.concurrent.atomic.AtomicReference;
+import java.util.function.Supplier;
+
+/**
+ * Time-bounded, single-flight cache around a single expensive metric sample.
The underlying sampler
+ * is invoked at most once per TTL window: when the cached value is stale,
exactly one caller runs
+ * the sampler while every other concurrent caller returns the most recent
published value rather
+ * than queueing behind the refresh or piling onto the sampler.
+ *
+ * <p>Used by {@link LoadAverageCircuitBreaker} and {@link
MemoryCircuitBreaker} so that high-QPS
+ * admission control cannot stampede the OS load-average syscall or the
post-GC heap-pool walk: even
+ * under thousands of concurrent {@code isTripped()} callers, the underlying
sampler is invoked at
+ * most once per TTL window.
+ *
+ * <p>This is a hand-rolled class rather than an off-the-shelf cache (e.g.
Caffeine) on purpose: we
+ * cache a single primitive, not a keyed map, and we want a synchronous
refresh with no executor and
+ * no shared async pool on the admission-control hot path. Caffeine's {@code
refreshAfterWrite} runs
+ * on {@code ForkJoinPool.commonPool()} unless given an executor, which would
land the heap-pool
+ * walk or load-average syscall on a shared pool and turn "one sample per TTL
window" into "one
+ * in-flight async refresh." The branch-level behavior is documented inline in
{@link
+ * #get(Supplier)}.
+ *
+ * <p><b>Exception behavior:</b> if the {@code source} supplier throws, the
exception propagates to
+ * the calling thread and no new sample is published. Any previously-published
value remains and
+ * other concurrent callers continue to see it; the next caller to find the
entry stale will retry
+ * the supplier. The {@code refreshing} flag is always released, so a thrown
sampler does not wedge
+ * the single-flight latch.
+ */
+final class TtlSampledMetric<T> {
+
+ private final long ttlNanos;
+ private final AtomicReference<Sample<T>> sample = new AtomicReference<>();
+ private final AtomicBoolean refreshing = new AtomicBoolean(false);
+
+ TtlSampledMetric(long ttlMs) {
+ this.ttlNanos = TimeUnit.MILLISECONDS.toNanos(ttlMs);
+ }
+
+ T get(Supplier<T> source) {
+ long now = System.nanoTime();
+ Sample<T> s = sample.get();
+ if (s != null && (now - s.nanos) < ttlNanos) {
+ return s.value;
+ }
+ if (s != null) {
+ // Stale value present: single-flight refresh — exactly one thread runs
the sampler;
+ // every other concurrent caller returns the most recent published value
immediately.
+ if (refreshing.compareAndSet(false, true)) {
+ try {
+ T v = source.get();
+ sample.set(new Sample<>(System.nanoTime(), v));
+ return v;
+ } finally {
+ refreshing.set(false);
+ }
+ }
+ // Re-read so we return whatever the winning thread may have just
published, rather than the
+ // older snapshot we captured at the top of this call. Once set, the
sample reference never
+ // reverts to null, so this is non-null here.
+ return sample.get().value;
+ }
+ // No sample yet — one-time cold path. Synchronize so the first wave of
concurrent callers
+ // does not all run the sampler. The monitor is held across the sampler
invocation; this is
+ // acceptable because the cold path runs at most once per successful
publish, and only callers
+ // that arrive before any value has been published are affected.
+ synchronized (this) {
+ s = sample.get();
+ if (s != null) {
+ return s.value;
+ }
+ T v = source.get();
+ sample.set(new Sample<>(System.nanoTime(), v));
+ return v;
+ }
+ }
+
+ private static final class Sample<T> {
+ final long nanos;
+ final T value;
+
+ Sample(long nanos, T value) {
+ this.nanos = nanos;
+ this.value = value;
+ }
+ }
+}
diff --git
a/solr/core/src/test/org/apache/solr/util/BaseTestCircuitBreaker.java
b/solr/core/src/test/org/apache/solr/util/BaseTestCircuitBreaker.java
index 7c6c2256a82..090a686c13b 100644
--- a/solr/core/src/test/org/apache/solr/util/BaseTestCircuitBreaker.java
+++ b/solr/core/src/test/org/apache/solr/util/BaseTestCircuitBreaker.java
@@ -356,11 +356,11 @@ public abstract class BaseTestCircuitBreaker extends
SolrTestCaseJ4 {
private static class FakeMemoryPressureCircuitBreaker extends
MemoryCircuitBreaker {
public FakeMemoryPressureCircuitBreaker() {
- super(1, 1);
+ super();
}
@Override
- protected long getAvgMemoryUsage() {
+ protected long getCurrentMemoryUsage() {
return Long.MAX_VALUE;
}
}
@@ -369,12 +369,12 @@ public abstract class BaseTestCircuitBreaker extends
SolrTestCaseJ4 {
private AtomicInteger count;
public BuildingUpMemoryPressureCircuitBreaker() {
- super(1, 1);
+ super();
this.count = new AtomicInteger(0);
}
@Override
- protected long getAvgMemoryUsage() {
+ protected long getCurrentMemoryUsage() {
int localCount = count.getAndIncrement();
if (localCount >= 4) {
diff --git
a/solr/core/src/test/org/apache/solr/util/circuitbreaker/TestHeapPressureCircuitBreakers.java
b/solr/core/src/test/org/apache/solr/util/circuitbreaker/TestHeapPressureCircuitBreakers.java
new file mode 100644
index 00000000000..a15fae81dce
--- /dev/null
+++
b/solr/core/src/test/org/apache/solr/util/circuitbreaker/TestHeapPressureCircuitBreakers.java
@@ -0,0 +1,111 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.solr.util.circuitbreaker;
+
+import java.lang.management.ManagementFactory;
+import java.lang.management.MemoryPoolMXBean;
+import java.lang.management.MemoryType;
+import java.util.concurrent.atomic.AtomicInteger;
+import org.apache.solr.SolrTestCase;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+
+/**
+ * Verifies that {@link MemoryCircuitBreaker} reads post-GC live data — not
the raw, garbage-
+ * inflated heap usage — so it does not trip on transient pre-collection peaks
that GC will reclaim.
+ */
+public class TestHeapPressureCircuitBreakers extends SolrTestCase {
+
+ @Before
+ public void setUpProps() {
+ System.setProperty(CircuitBreakerRegistry.SYSPROP_SAMPLE_TTL_MS, "0");
+ }
+
+ @After
+ public void tearDownProps() {
+ System.clearProperty(CircuitBreakerRegistry.SYSPROP_SAMPLE_TTL_MS);
+ }
+
+ @Test
+ public void memoryBreakerReadsRealHeapPools() {
+ // Sanity: the in-process JVM exposes at least one heap pool.
samplePostGcLiveBytes() must
+ // return a non-negative value computed from real pool data, regardless of
which collector
+ // is active. (Pre-first-GC, getCollectionUsage() can be zero on every
pool — accept that.)
+ boolean hasHeapPool = false;
+ for (MemoryPoolMXBean pool : ManagementFactory.getMemoryPoolMXBeans()) {
+ if (pool.getType() == MemoryType.HEAP) {
+ hasHeapPool = true;
+ break;
+ }
+ }
+ assertTrue("JVM must expose at least one HEAP MemoryPoolMXBean",
hasHeapPool);
+
+ long bytes = new MemoryCircuitBreaker().samplePostGcLiveBytes();
+ assertTrue("post-GC live bytes must be non-negative, got " + bytes, bytes
>= 0);
+ }
+
+ @Test
+ public void memoryBreakerTripsWhenOverThreshold() {
+ MemoryCircuitBreaker breaker =
+ new MemoryCircuitBreaker() {
+ @Override
+ protected long getCurrentMemoryUsage() {
+ return Long.MAX_VALUE; // simulate "heap is exhausted post-GC"
+ }
+ };
+ breaker.setThreshold(50.0);
+ assertTrue(breaker.isTripped());
+ }
+
+ @Test
+ public void memoryBreakerDoesNotTripWhenUnderThreshold() {
+ MemoryCircuitBreaker breaker =
+ new MemoryCircuitBreaker() {
+ @Override
+ protected long getCurrentMemoryUsage() {
+ return 0L; // simulate "post-GC live data is empty"
+ }
+ };
+ breaker.setThreshold(50.0);
+ assertFalse(breaker.isTripped());
+ }
+
+ @Test
+ public void samplePostGcLiveBytesIsCachedThroughGetCurrentMemoryUsage() {
+ // With a long TTL, getCurrentMemoryUsage() should consult
samplePostGcLiveBytes() at most once
+ // across many isTripped() invocations — i.e. the cache wraps the
overridable hook, not just
+ // the static heap-pool walk.
+ System.setProperty(CircuitBreakerRegistry.SYSPROP_SAMPLE_TTL_MS, "60000");
+ AtomicInteger calls = new AtomicInteger();
+ MemoryCircuitBreaker breaker =
+ new MemoryCircuitBreaker() {
+ @Override
+ protected long samplePostGcLiveBytes() {
+ calls.incrementAndGet();
+ return 0L;
+ }
+ };
+ breaker.setThreshold(50.0);
+ for (int i = 0; i < 50; i++) {
+ breaker.isTripped();
+ }
+ assertEquals(
+ "samplePostGcLiveBytes() must be invoked at most once per TTL window",
1, calls.get());
+ }
+}
diff --git
a/solr/core/src/test/org/apache/solr/util/circuitbreaker/TestLoadAverageCircuitBreakerSampling.java
b/solr/core/src/test/org/apache/solr/util/circuitbreaker/TestLoadAverageCircuitBreakerSampling.java
new file mode 100644
index 00000000000..07fe90dbb6e
--- /dev/null
+++
b/solr/core/src/test/org/apache/solr/util/circuitbreaker/TestLoadAverageCircuitBreakerSampling.java
@@ -0,0 +1,175 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.solr.util.circuitbreaker;
+
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicInteger;
+import org.apache.solr.SolrTestCase;
+import org.apache.solr.common.util.ExecutorUtil;
+import org.apache.solr.common.util.SolrNamedThreadFactory;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+
+/**
+ * Verifies that {@link LoadAverageCircuitBreaker} caches the result of {@code
+ * calculateLiveLoadAverage()} for the configured TTL so that high-QPS
admission control does not
+ * re-poll {@code OperatingSystemMXBean.getSystemLoadAverage()} per request.
+ */
+public class TestLoadAverageCircuitBreakerSampling extends SolrTestCase {
+
+ @Override
+ @Before
+ public void setUp() throws Exception {
+ super.setUp();
+ // Long TTL so successive isTripped() calls within one test stay in the
same cache window.
+ System.setProperty(CircuitBreakerRegistry.SYSPROP_SAMPLE_TTL_MS, "60000");
+ }
+
+ @After
+ public void tearDownProps() {
+ System.clearProperty(CircuitBreakerRegistry.SYSPROP_SAMPLE_TTL_MS);
+ }
+
+ @Test
+ public void successiveIsTrippedSharesOneSample() {
+ AtomicInteger calls = new AtomicInteger();
+ LoadAverageCircuitBreaker breaker =
+ new LoadAverageCircuitBreaker() {
+ @Override
+ protected double calculateLiveLoadAverage() {
+ calls.incrementAndGet();
+ return 1.0;
+ }
+ };
+ breaker.setThreshold(0.5);
+
+ for (int i = 0; i < 100; i++) {
+ assertTrue("breaker should be tripped (1.0 >= 0.5)",
breaker.isTripped());
+ }
+ assertEquals(
+ "Underlying calculateLiveLoadAverage() must be invoked once per TTL
window, not per call",
+ 1,
+ calls.get());
+ }
+
+ @Test
+ public void zeroTtlDisablesCache() {
+ System.setProperty(CircuitBreakerRegistry.SYSPROP_SAMPLE_TTL_MS, "0");
+ AtomicInteger calls = new AtomicInteger();
+ LoadAverageCircuitBreaker breaker =
+ new LoadAverageCircuitBreaker() {
+ @Override
+ protected double calculateLiveLoadAverage() {
+ calls.incrementAndGet();
+ return 1.0;
+ }
+ };
+ breaker.setThreshold(0.5);
+
+ for (int i = 0; i < 5; i++) {
+ breaker.isTripped();
+ }
+ assertEquals("With zero TTL, every call re-evaluates", 5, calls.get());
+ }
+
+ /**
+ * Stampede scenario: many concurrent callers find the cache stale at the
same instant. Single-
+ * flight refresh must ensure the underlying sampler is invoked at most once
across all of them.
+ * Without this guarantee, a burst of concurrent requests would each pin a
CPU on the load-
+ * average syscall, which is the bug this breaker exists to prevent — not
cause.
+ */
+ @Test
+ public void concurrentStampedeRunsSamplerAtMostOnce() throws Exception {
+ // Force every call to find the cache stale.
+ System.setProperty(CircuitBreakerRegistry.SYSPROP_SAMPLE_TTL_MS, "0");
+
+ final AtomicInteger samplerInvocations = new AtomicInteger();
+ final AtomicInteger inFlight = new AtomicInteger();
+ final AtomicInteger maxInFlight = new AtomicInteger();
+
+ LoadAverageCircuitBreaker breaker =
+ new LoadAverageCircuitBreaker() {
+ @Override
+ protected double calculateLiveLoadAverage() {
+ int now = inFlight.incrementAndGet();
+ maxInFlight.accumulateAndGet(now, Math::max);
+ try {
+ // Hold the "syscall" long enough that, without single-flight,
every concurrent
+ // caller would pile in.
+ Thread.sleep(20);
+ } catch (InterruptedException ie) {
+ Thread.currentThread().interrupt();
+ } finally {
+ inFlight.decrementAndGet();
+ }
+ samplerInvocations.incrementAndGet();
+ return 1.0;
+ }
+ };
+ breaker.setThreshold(0.5);
+
+ // Prime the cache so the stampede hits the stale-refresh path (not the
cold path).
+ breaker.isTripped();
+ int primed = samplerInvocations.get();
+
+ final int threads = 64;
+ final CountDownLatch start = new CountDownLatch(1);
+ final CountDownLatch done = new CountDownLatch(threads);
+ ExecutorService pool =
+ ExecutorUtil.newMDCAwareFixedThreadPool(
+ threads, new
SolrNamedThreadFactory("TestLoadAverageCircuitBreakerSampling"));
+ try {
+ for (int i = 0; i < threads; i++) {
+ pool.submit(
+ () -> {
+ try {
+ start.await();
+ breaker.isTripped();
+ } catch (InterruptedException ie) {
+ Thread.currentThread().interrupt();
+ } finally {
+ done.countDown();
+ }
+ });
+ }
+ start.countDown();
+ assertTrue("all threads completed", done.await(30, TimeUnit.SECONDS));
+ } finally {
+ pool.shutdownNow();
+ }
+
+ assertEquals(
+ "Single-flight: at most one sampler invocation runs at a time across
the stampede",
+ 1,
+ maxInFlight.get());
+ // We deliberately don't assert an exact stampedeInvocations count here.
Single-flight
+ // guarantees at-most-one *concurrent* sampler invocation; under load a
thread that arrives
+ // after the winning thread releases the CAS legitimately runs the sampler
again. Asserting
+ // the count would make the test timing-sensitive on slow CI shards. The
maxInFlight==1
+ // assertion above captures the actual invariant.
+ int stampedeInvocations = samplerInvocations.get() - primed;
+ assertTrue(
+ "Sanity: stampede produced at least one sampler invocation (saw "
+ + stampedeInvocations
+ + ")",
+ stampedeInvocations >= 1);
+ }
+}
diff --git
a/solr/solr-ref-guide/modules/deployment-guide/pages/circuit-breakers.adoc
b/solr/solr-ref-guide/modules/deployment-guide/pages/circuit-breakers.adoc
index 0d177cb56a9..13db608df90 100644
--- a/solr/solr-ref-guide/modules/deployment-guide/pages/circuit-breakers.adoc
+++ b/solr/solr-ref-guide/modules/deployment-guide/pages/circuit-breakers.adoc
@@ -74,6 +74,16 @@ return a HTTP 503 code instead of the HTTP 429 code used by
the new circuit brea
This circuit breaker tracks JVM heap memory usage and rejects incoming
requests with a 429 error code if the heap usage exceeds a configured
percentage of maximum heap allocated to the JVM (-Xmx).
The main configuration for this circuit breaker is controlling the threshold
percentage at which the breaker will trip.
+The heap-usage signal is read from `MemoryPoolMXBean.getCollectionUsage()` on
the old/tenured generation pool — that is, JVM heap usage *immediately after*
the most recent collection.
+This is the only heap reading that distinguishes live data from garbage
waiting to be reclaimed; on a generational collector, raw `used` heap climbs
toward `max` between collections during normal operation, which is the
steady-state shape of a healthy JVM rather than a problem worth tripping on.
+
+For non-generational collectors (non-generational ZGC, Shenandoah) where the
JVM exposes a single combined heap pool, the breaker sums
`getCollectionUsage()` across all `HEAP`-typed pools instead.
+
+[NOTE]
+====
+Until the JVM has performed at least one collection on a heap pool,
`getCollectionUsage()` may return `null` on every pool and the breaker will
report `0` live bytes — meaning it will not trip. On a freshly started node
with a small workload this can take some time, particularly under ZGC or
Shenandoah.
+====
+
To enable and configure the JVM heap usage based circuit breaker, add the
following:
.Per collection in `solrconfig.xml`
@@ -158,6 +168,11 @@ SOLR_CIRCUITBREAKER_QUERY_LOADAVG=8.0
The triggering threshold is a floating point number matching load average.
The example circuit breaker above will trip when the load average is equal to
or greater than 8.0.
+[NOTE]
+====
+The OS load average is itself a moving average (typically over one minute), so
this breaker caches the sampled value for a short TTL and shares it across
concurrent requests; under high QPS the underlying `getSystemLoadAverage()`
syscall is invoked at most once per TTL window. The TTL defaults to 1000 ms and
is configurable per-process with the `solr.circuitbreaker.sample.ttl.ms` system
property (or the equivalent `SOLR_CIRCUITBREAKER_SAMPLE_TTL_MS` environment
variable). The TTL is read [...]
+====
+
[NOTE]
====
The System Load Average Circuit breaker behavior is dependent on the operating
system, and may not work on some operating systems like Microsoft Windows. See
https://docs.oracle.com/en/java/javase/17/docs/api/java.management/java/lang/management/OperatingSystemMXBean.html#getSystemLoadAverage()[JavaDoc]
for more.
