ES|QL:Fix Tdigest percentiles circuit breaker

[hawkhuang-collab]

Fixes: #1639

Important Notes

This fix is targeting data node OOMs and will solve the the incident and we've also made the coordinator side that no memory tracking explicit. Good news is, typically the data node will trim data to topN, for example top 25 buckets only and thus what coordinator node received is rather small and practically it is safe.

Problem

AbstractTDigestPercentilesAggregator creates one HistogramUnionState sketch per collected bucket. Three bugs made this memory invisible to the circuit breaker and prevented proper cleanup.

1. NOOP_BREAKER during collection. Every sketch was created with NOOP_BREAKER:

state = HistogramUnionState.create(HistogramUnionState.NOOP_BREAKER, executionHint, compression);

NoopCircuitBreaker ignores every byte. All TDigest memory (centroid arrays, sort buffers) was invisible to the request breaker. Under a high-cardinality TERMS + PERCENTILES query the heap fills up and the node OOMs rather than the breaker throwing CircuitBreakingException.

2. Bytes never released after buildAggregation. doClose() called Releasables.close(states), which frees the backing ObjectArray container but never calls HistogramUnionState.close() on the elements inside it. Breaker bytes leaked permanently.

3. Reducer inherited a closed breaker. getLeaderReducer() called HistogramUnionState.createUsingParamsFrom(shard_state), copying the shard's breaker into the merge accumulator. If that breaker was a PreallocatedCircuitBreaker already torn down before reduction ran, any allocation would throw.

Fix

1. Pass context.breaker() instead of NOOP_BREAKER

context is a protected final field from AggregatorBase. Passing the real REQUEST breaker wires up the full tracking chain:

HistogramUnionState.create(context.breaker())
  -> stores this.breaker
  -> getOrInitializeTDigestState() -> TDigestState.create(breaker, ...)
    -> new MemoryTrackingTDigestArrays(breaker)
      -> every centroid array allocation charges the real REQUEST breaker

MemoryTrackingTDigestArrays was already wired correctly through HistogramUnionState. It was just never reached because NOOP_BREAKER was passed at the top.

2. Return bytes in takeState(), not in doClose()

There is a timing constraint that shapes the fix: InternalAggregation has no Releasable lifecycle (see #99815). By the time the result is serialized, the PreallocatedCircuitBreaker wrapping the REQUEST breaker is already closed. Any addWithoutBreaking() call at that point throws IllegalStateException: already closed. The only window to return bytes is inside buildAggregation, while the aggregation context is still open.

buildAggregation() calls takeState() instead of getState(). takeState() nulls the slot in states (so doClose() skips it) and returns the bytes immediately:

context.breaker().addWithoutBreaking(-state.ramBytesUsed());

This runs while the aggregation context is still open. The state's data stays intact for serialization and reduction; only the breaker accounting is released.

3. Fix doClose() to close elements individually

try {
    for (long i = 0; i < states.size(); i++) {
        HistogramUnionState state = states.get(i);
        if (state != null) {
            Releasables.closeWhileHandlingException(state);
        }
    }
} finally {
    Releasables.close(states);
}

closeWhileHandlingException keeps going if one slot fails. doClose() now only handles the failure path (states not handed off via takeState()). The null guard on states itself covers the case where the breaker trips inside the constructor before the field is assigned.

4. Fix the reducer to use NOOP_BREAKER explicitly

getLeaderReducer() now passes NOOP_BREAKER explicitly to createUsingParamsFrom(). For same node reduction, the shard's PreallocatedCircuitBreaker is already closed before reduction runs, so any charge would throw IllegalStateException.
AggregatorReducer.close() is a no-op, so a live breaker would charge bytes that are never returned.

Background

Issue #99815 ("Integrate TDigestState with circuit breakers") was closed noting that the ES|QL path was fixed but the _search agg path "would require bigger refactors." MemoryTrackingTDigestArrays and the breaker threading inside HistogramUnionState now exist and are complete which make this PR much easier.

To Consider

MovingPercentilesPipelineAggregator, AbstractBoxplotAggregator and AbstractTDigestOrExponentialMergingAggregator also still using NOOP_BREAKER, they could OOM the same way. Separated follow up PRs.

[elasticsearchmachine]

Pinging @elastic/es-analytical-engine (Team:Analytics)

[GalLalouche]

It does?

[GalLalouche]

As discussed offline, the assignment being after super() is only telling half the story: super() registers this in the constructor, so doClose can be called before the constructor of this class finishes.

1. Please update the above comment to reflect this.
2. Please update AggregatorBase#doClose javadoc to reflect that this method may be called before the constructor has finished.

[hawkhuang-collab]

Yeah, updated

[GalLalouche]

Gal's Minion 🤖: No argument with the scoping. Could you open a follow-up issue before this merges, though? Right now both deferrals only live in places that vanish on merge — this thread and the "To Consider" section.

Worth covering two separate things:

1. Coordinator-side accounting: getLeaderReducer and the AbstractInternalTDigestPercentiles(StreamInput) path, plus MovingPercentilesPipelineAggregator. Agreed that topN trimming bounds this in practice.
2. The shard-level siblings from "To Consider": AbstractBoxplotAggregator.getExistingOrNewHistogram and AbstractTDigestOrExponentialMergingAggregator.getExistingOrNewHistogram are copies of the method being fixed here, NOOP_BREAKER and all, and their doClose() closes only the ObjectArray, never the elements. Those sit on the in-scope side of your own line — same data node, same per-bucket sketch, same high-cardinality terms shape — so boxplot can still OOM the node after this merges.

Linking the issue from the inline comment on this line would help too; as written it says what NOOP_BREAKER does here, but not that the gap is deliberate.

[hawkhuang-collab]

Mute these lookup join test case, they are revealed by our BWC test PR, make change here to get the build pass, but they have nothing to do with this PR.

[elasticsearchmachine]

💚 Backport successful

Status	Branch	Result
✅	9.4
✅	9.5