Skip to content
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.

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

Stop capturing the BucketAggregator when building internal sub-aggregations #104758

Closed
wants to merge 3 commits into from

Conversation

iverase
Copy link
Contributor

@iverase iverase commented Jan 25, 2024

I notice that when building internal subaggregation using the method BucketAggregator#buildSubAggsForBuckets, we are capturing the current BucketAggregator when we create the Abstract list. This means we are holding for much longer the object which can be pretty big, as seen in this heap dump extract:

image

This PR adds a static inner class to avoid capturing the BucketAggregator.

@elasticsearchmachine elasticsearchmachine added the Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) label Jan 25, 2024
@elasticsearchmachine
Copy link
Collaborator

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

@elasticsearchmachine
Copy link
Collaborator

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

Copy link
Contributor

@alex-spies alex-spies left a comment

Choose a reason for hiding this comment

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

LGTM, but probably best if Nik or Mark take a look as well.

@original-brownbear
Copy link
Member

@iverase see my version of that PR: #104762 :) I think we can be a little lazier there as well :) not sure how much that helps that much in addition to this one but shorter lifetimes are always nice maybe.

@iverase
Copy link
Contributor Author

iverase commented Jan 25, 2024

@original-brownbear let's go with your approach, every bit helps :)

@iverase iverase closed this Jan 25, 2024
@iverase iverase deleted the BucketsAggregator branch January 25, 2024 14:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Analytics/Aggregations Aggregations >bug Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) v8.13.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants