Aggs: Avoid OOMs by accounting memory on cardinality agg reduction phase by ivancea · Pull Request #152773 · elastic/elasticsearch

ivancea · 2026-07-02T15:26:06Z

Fixes #150290

Adds the CB'd BigArrays to HyperLogLogPlus class in InternalCartinality
Clones it on get() with a non-accounting BigArrays. InternalAggregations don't have a safe open-close lifecycle, and keeping it there would leak memory

elasticsearchmachine · 2026-07-02T15:26:33Z

Hi @ivancea, I've created a changelog YAML for you.

github-actions · 2026-07-02T15:27:26Z

🔍 Preview links for changed docs

⏳ Building and deploying preview... View progress

This comment will be updated with preview links when the build is complete.

github-actions · 2026-07-02T15:29:03Z

ℹ️ Important: Docs version tagging

👋 Thanks for updating the docs! Just a friendly reminder that our docs are now cumulative. This means all 9.x versions are documented on the same page and published off of the main branch, instead of creating separate pages for each minor version.

We use applies_to tags to mark version-specific features and changes.

Expand for a quick overview

When to use applies_to tags:

✅ At the page level to indicate which products/deployments the content applies to (mandatory)
✅ When features change state (e.g. preview, ga) in a specific version
✅ When availability differs across deployments and environments

What NOT to do:

❌ Don't remove or replace information that applies to an older version
❌ Don't add new information that applies to a specific version without an applies_to tag
❌ Don't forget that applies_to tags can be used at the page, section, and inline level

🤔 Need help?

Check out the cumulative docs guidelines
Reach out in the #docs Slack channel

ivancea · 2026-07-02T15:30:51Z

+                    ? null
+                    : reduceContext.bigArrays().breakerService().getBreaker(CircuitBreaker.REQUEST);
+                if (breaker != null) {
+                    breaker.addEstimateBytesAndMaybeBreak(cloneBytes, "cardinality reduce result clone");


This will peak memory usage by x2 for this HLL+. Classes like CardinalityAggregator do this same thing, without the peak accounting:

elasticsearch/server/src/main/java/org/elasticsearch/search/aggregations/metrics/CardinalityAggregator.java

Lines 177 to 180 in 032daf9

// We need to build a copy because the returned Aggregation needs remain usable after

// this Aggregator (and its HLL++ counters) is released.

AbstractHyperLogLogPlusPlus copy = counts.clone(owningBucketOrdinal, BigArrays.NON_RECYCLING_INSTANCE);

return new InternalCardinality(name, copy, metadata());

Options I see:

Double accounting for the cloned HLL+ without bigarrays (May trip)

No double accounting (May OOM)

Manual accounting (May get outdated or be inconsistent, but we can fix it after each call by calling ramBytesUsed() on the HLL+)

Double counting these is fine with me.

It's not great, but we'll do our best with _search aggs these days.

Copilot

Pull request overview

This PR fixes an aggregation reduction memory-accounting gap in InternalCardinality by ensuring HyperLogLog++ merge allocations are tracked by the REQUEST circuit breaker during coordinator reduction, preventing untracked growth that can lead to OOMs.

Changes:

Use reduceContext.bigArrays() (rather than BigArrays.NON_RECYCLING_INSTANCE) when constructing the coordinator-side HyperLogLogPlusPlus used for reduction, so merges are REQUEST-breaker aware.
Add peak-memory accounting for the final “result clone” step and ensure the reduction state is released via AggregatorReducer#close().
Add focused tests validating reduction trips the REQUEST breaker and that clone-time peak accounting is enforced.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

File	Description
server/src/main/java/org/elasticsearch/search/aggregations/metrics/InternalCardinality.java	Route reduction allocations through `reduceContext.bigArrays()` and add breaker-charged peak accounting for cloning + proper release on close.
server/src/main/java/org/elasticsearch/search/aggregations/metrics/HyperLogLogPlusPlus.java	Add `ramBytesUsed()` helpers to support clone peak-memory estimation.
server/src/test/java/org/elasticsearch/search/aggregations/metrics/InternalCardinalityTests.java	Add tests asserting REQUEST breaker usage during reduction and peak-memory accounting during result cloning.
docs/changelog/152773.yaml	Add changelog entry for the fix.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

elasticsearchmachine · 2026-07-03T09:43:15Z

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

craigtaverner

LGTM, with some questions/suggestions.

craigtaverner · 2026-07-03T10:26:01Z

+issues:
+ - 150290
+pr: 152773
+summary: "Aggs: Account memory on `InternalCardinality` reduction"


As a changelog entry this is perhaps a bit cryptic. Is there a way to describe this so that users understand the change better? Perhaps "Improve memory usage protections on aggregations in the coordinator"?

craigtaverner · 2026-07-03T10:41:53Z

+                try {
+                    AbstractHyperLogLogPlusPlus result = toRelease.clone(0, BigArrays.NON_RECYCLING_INSTANCE);
+                    reduced = null;
+                    Releasables.close(toRelease);


Curious why we don't just close reduced here since toRelease is the same reference? Is it that the close can fail, and we don't want to repeat the close on line 125 again?

Yes, it's to avoid that double-close.

That said, I'll improve it a bit by moving the temp var assignation here, and add a micro-comment:

HyperLogLogPlusPlus toRelease = reduced; reduced = null; Releasables.close(toRelease);

craigtaverner · 2026-07-03T10:45:48Z

+        }
+        CircuitBreakerService breakerService = LimitedBreaker.service(
+            CircuitBreaker.REQUEST,
+            ByteSizeValue.ofBytes(reducedHllBytes * 3 / 2)


Any value in testing that we don't get an exception if the CB limit is more than 2x the reducedHllBytes?

Not bad to avoid over-counting I suppose. Added!

elasticsearchmachine · 2026-07-06T15:44:25Z

💔 Backport failed

Status	Branch	Result
✅	9.4
❌	8.19	Commit could not be cherrypicked due to conflicts

You can use sqren/backport to manually backport by running backport --upstream elastic/elasticsearch --pr 152773

…ase (#152773) (#152991) Fixes #150290 - Adds the CB'd `BigArrays` to `HyperLogLogPlus` class in `InternalCartinality` - Clones it on get() with a non-accounting `BigArrays`. `InternalAggregation`s don't have a safe open-close lifecycle, and keeping it there would leak memory

…ase (#152773) (#152995) Fixes #150290 - Adds the CB'd `BigArrays` to `HyperLogLogPlus` class in `InternalCartinality` - Clones it on get() with a non-accounting `BigArrays`. `InternalAggregation`s don't have a safe open-close lifecycle, and keeping it there would leak memory

…ase (elastic#152773) Fixes elastic#150290 - Adds the CB'd `BigArrays` to `HyperLogLogPlus` class in `InternalCartinality` - Clones it on get() with a non-accounting `BigArrays`. `InternalAggregation`s don't have a safe open-close lifecycle, and keeping it there would leak memory

…ion phase (#153246) * Aggs: Avoid OOMs by accounting memory on cardinality agg reduction phase (#152773) Fixes #150290 - Adds the CB'd `BigArrays` to `HyperLogLogPlus` class in `InternalCartinality` - Clones it on get() with a non-accounting `BigArrays`. `InternalAggregation`s don't have a safe open-close lifecycle, and keeping it there would leak memory * Undo extra fix

Aggs: Account memory on InternalCardinality reduction

bdddf9c

ivancea requested review from Copilot, julian-elastic and nik9000 July 2, 2026 15:26

ivancea added >bug :Analytics/Aggregations Aggregations Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) v9.5.0 labels Jul 2, 2026

Update docs/changelog/152773.yaml

84db49f

Copilot started reviewing on behalf of ivancea July 2, 2026 15:26 View session

ivancea commented Jul 2, 2026

View reviewed changes

Copilot AI reviewed Jul 2, 2026

View reviewed changes

[CI] Auto commit changes from spotless

3293d8c

ivancea marked this pull request as ready for review July 3, 2026 09:42

Merge branch 'main' into aggs-prevent-hll-oom

8ce1ab9

craigtaverner approved these changes Jul 3, 2026

View reviewed changes

ivancea added 2 commits July 3, 2026 15:00

Simplify anti-double-closing logic

d4835ca

Add positive test

653a139

ivancea changed the title ~~Aggs: Account memory on InternalCardinality reduction~~ Aggs: Avoid OOMs by account memory on cardinality agg reduction phase Jul 3, 2026

elasticsearchmachine and others added 3 commits July 3, 2026 13:19

[CI] Auto commit changes from spotless

b878aa6

Improved changelog

c62c212

Merge branch 'main' into aggs-prevent-hll-oom

19c0170

ivancea added auto-backport Automatically create backport pull requests when merged v9.4.4 v9.3.8 v8.19.19 labels Jul 6, 2026

ivancea removed the v9.3.8 label Jul 6, 2026

ivancea changed the title ~~Aggs: Avoid OOMs by account memory on cardinality agg reduction phase~~ Aggs: Avoid OOMs by accounting memory on cardinality agg reduction phase Jul 6, 2026

Fixed changelog

2db890a

ivancea merged commit c27d292 into elastic:main Jul 6, 2026
6 of 9 checks passed

ivancea deleted the aggs-prevent-hll-oom branch July 6, 2026 15:42

ivancea mentioned this pull request Jul 6, 2026

[9.4] Aggs: Avoid OOMs by accounting memory on cardinality agg reduction phase (#152773) #152991

Merged

elasticsearchmachine added the backport pending label Jul 6, 2026

ivancea mentioned this pull request Jul 6, 2026

[8.19] Aggs: Avoid OOMs by accounting memory on cardinality agg reduction phase #152995

Merged

ivancea removed the backport pending label Jul 6, 2026

ivancea mentioned this pull request Jul 8, 2026

[9.3] Aggs: Avoid OOMs by accounting memory on cardinality agg reduction phase #153246

Merged

ivancea added the v9.3.8 label Jul 8, 2026

	// We need to build a copy because the returned Aggregation needs remain usable after
	// this Aggregator (and its HLL++ counters) is released.
	AbstractHyperLogLogPlusPlus copy = counts.clone(owningBucketOrdinal, BigArrays.NON_RECYCLING_INSTANCE);
	return new InternalCardinality(name, copy, metadata());

Conversation

ivancea commented Jul 2, 2026

Uh oh!

elasticsearchmachine commented Jul 2, 2026

Uh oh!

github-actions Bot commented Jul 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🔍 Preview links for changed docs

Uh oh!

github-actions Bot commented Jul 2, 2026

ℹ️ Important: Docs version tagging

When to use applies_to tags:

What NOT to do:

🤔 Need help?

Uh oh!

ivancea Jul 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

elasticsearchmachine commented Jul 3, 2026

Uh oh!

craigtaverner left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

elasticsearchmachine commented Jul 6, 2026

💔 Backport failed

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

github-actions Bot commented Jul 2, 2026 •

edited

Loading

ivancea Jul 2, 2026 •

edited

Loading