Skip to content

Release AggregationContext more eagerly #127484

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

Conversation

original-brownbear
Copy link
Member

@original-brownbear original-brownbear commented Apr 28, 2025

No need to drag a reference to this thing around until the query result is fully released. Track it via a future (so it's unlinked on release and just in general this makes for a much much safer pattern) and release it as soon as we set the aggregations. Only release on aggs release and ref-counting to zero as a backup.
Also, make it clear in naming and interfaces that this is not about a general purpose releasable tracking on the query result.

No need to drag a reference to this thing around until the query result
is fully release. Track it via a future (so it's unlinked on release)
and release it as soon as we set the aggregations.
Only release on aggs release and ref-counting to zero as a backup.
@elasticsearchmachine elasticsearchmachine added Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) v9.1.0 labels Apr 28, 2025
@elasticsearchmachine
Copy link
Collaborator

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

Copy link
Member

@dnhatn dnhatn left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks Armin!

@original-brownbear
Copy link
Member Author

Thanks Nhat!

@original-brownbear original-brownbear merged commit 0ce8448 into elastic:main Apr 29, 2025
17 checks passed
@original-brownbear original-brownbear deleted the release-aggs-context-more-eagerly branch April 29, 2025 08:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Analytics/Aggregations Aggregations >non-issue Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) v9.1.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants