[BUG] Circuit breaker G1OverLimitStrategy can trap heap above threshold

[bowenlan-amzn]

Describe the bug

The G1OverLimitStrategy allocates humongous objects in a loop to trigger GC when heap exceeds the circuit breaker threshold (Humongous allocations can trigger GC when IHOP threshold is exceeded reference). However, when the breaker limit is low, this loop can allocate too much too fast, keeping Old Gen memory elevated instead of helping it recover.

We observed a production incident where a node's heap increased from 39.5GB to 93.9GB and did not recover for 30-60 minutes, even though the memory-intensive workload had completed within seconds. JFR shows most humongous allocations after the spike come from G1OverLimitStrategy.

JVM settings

-XX:+UseG1GC
-XX:InitiatingHeapOccupancyPercent=30
-XX:G1ReservePercent=25
Heap: 125GB
G1 Region Size: 32MB

After raising indices.breaker.total.limit from 70% to 85%, the problem never recurred.

This works because:

• After spike to 92GB, heap is BELOW 106GB (85% of 125GB)
• Circuit breaker over limit strategy doesn't fire
• GC has time to recover without additional pressure

Relevant code:

OpenSearch/server/src/main/java/org/opensearch/indices/breaker/HierarchyCircuitBreakerService.java

Lines 604 to 619 in f6c78d7

	int allocationCount = Math.toIntExact((maxHeap - memoryUsed.baseUsage) / g1RegionSize + 1);
	// allocations of half-region size becomes single humongous alloc, thus taking up a full region.
	int allocationSize = (int) (g1RegionSize >> 1);
	long maxUsageObserved = memoryUsed.baseUsage;
	for (; allocationIndex < allocationCount; ++allocationIndex) {
	long current = currentMemoryUsageSupplier.getAsLong();
	if (current >= maxUsageObserved) {
	maxUsageObserved = current;
	} else {
	// we observed a memory drop, so some GC must have occurred
	break;
	}
	if (initialCollectionCount != gcCountSupplier.getAsLong()) {
	break;
	}
	localBlackHole += new byte[allocationSize].hashCode();

int allocationCount = Math.toIntExact((maxHeap - memoryUsed.baseUsage) / g1RegionSize + 1);
int allocationSize = (int) (g1RegionSize >> 1);  // 16MB humongous objects
for (; allocationIndex < allocationCount; ++allocationIndex) {
    // ... break conditions ...
    localBlackHole += new byte[allocationSize].hashCode();
}

When the breaker limit is low (e.g., 70%), the threshold is crossed earlier, so memoryUsed.baseUsage is lower when the strategy triggers. This results in a large allocationCount. For example, on a 125GB heap triggered at 88GB: (125-88) / 0.032 + 1 = 1157 iterations × 16MB = ~18GB of potential humongous allocations.

When the heap is fragmented (evacuation failures), the loop allocates faster than GC can reclaim. This creates a feedback loop where the strategy meant to help actually prevents recovery.

Proposed fix

Cap allocationCount to a reasonable maximum:

private static final int MAX_ALLOCATION_ATTEMPTS = 64;  // ~1GB max per invocation
// ...
int allocationCount = Math.min(
    Math.toIntExact((maxHeap - memoryUsed.baseUsage) / g1RegionSize + 1),
    MAX_ALLOCATION_ATTEMPTS
);

This limits each invocation to ~1GB instead of potentially 18GB, reducing the risk of the strategy worsening heap pressure.

Additional Details

Related component

Search:Resiliency

Affected versions

All versions using G1OverLimitStrategy with G1GC.

[kkhatua]

This is an interesting behaviour, @bowenlan-amzn !

Testing does not seem trivial though.

Curious to know what @Bukhtawar, @msfroh and @jainankitk think about the fix.

[sgup432]

We certainly need to rethink this custom based logic to trigger GC. IMO it does not make such sense to have this logic altogether(ie allocating objects) and instead rely on the default GC behavior(along with aggressively killing tasks and stopping new allocations)

Some approaches I can think of:

1. <80-85% circuit breaker limit, we should just not allow new allocations which is the current behavior.
2. With >85-90%, we should instead start cancelling tasks aggressively either via search backpressure or otherwise. Clearing cache(field data cache) as well or disable it if needed.
3. If we want to keep this logic, better to have it when circuit breaker is set >=85% along with capping the number of allocations made as mentioned in the proposed fix section.

[Bukhtawar]

Interesting find, curious what was the Young GC size(G1MaxNewSize)

[bowenlan-amzn]

G1MaxNewSizePercent is 40

[jainankitk]

This is an interesting find @bowenlan-amzn. I can see that the original intent behind this logic was to avoid leaving the used memory at a very high level for an extended duration, triggering the real memory circuit breaker even at low activity levels.

With G1ReservePercent=25%, the adaptive IHOP mechanism should ensure that old/mixed GC kicks in no later than at 75% heap. IMO, the primary issue is allowing 70% limit for circuit breaker even when the IHOP might be as high as 75%.

Also, with indices.breaker.total.use_real_memory set to true (which is default value), the circuit breaker should be set to 95%. Are we setting indices.breaker.total.use_real_memory to false due to which 70% limit is getting picked up?

[bowenlan-amzn]

Are we setting indices.breaker.total.use_real_memory to false due to which 70% limit is getting picked up?

No, use_real_memory is true. Only the breaker limit is updated to 70% and that's a dynamic cluster setting.

[jainankitk]

Only the breaker limit is updated to 70% and that's a dynamic cluster setting.

I am wondering if it makes sense to have reasonable minimum value threshold like 85% on the breaker limit when use_real_memory is true. That being said, I do like the below idea:

This limits each invocation to ~1GB instead of potentially 18GB, reducing the risk of the strategy worsening heap pressure.