Skip to content

Conversation

@original-brownbear
Copy link
Contributor

@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
Contributor 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