Skip to content

Comments

[9.0] Aggregations cancellation after collection (#120944)#121921

Merged
elasticsearchmachine merged 1 commit intoelastic:9.0from
not-napoleon:backport/9.0/pr-120944
Feb 6, 2025
Merged

[9.0] Aggregations cancellation after collection (#120944)#121921
elasticsearchmachine merged 1 commit intoelastic:9.0from
not-napoleon:backport/9.0/pr-120944

Conversation

@not-napoleon
Copy link
Member

Backports the following commits to 9.0:

This PR addresses issues around aggregations cancellation, mentioned in elastic#108701 and other places. In brief, during aggregations collection time, we respect cancellation via the mechanisms in the searcher to poison cancelled queries. But once the aggregation finishes collection, there is no further need to interact with the searcher, so we cannot rely on that for cancellation checking. In particular, deeply nested aggregations can spend a long time constructing the results tree.

Checking for cancellation is a trade off, as the check itself is somewhat expensive (it involves a volatile read), so we want to balance checking often enough that cancelled queries aren't taking up resources for a long time, but not so frequently that it slows down most aggregation queries. Our first attempt to this is to check once when we go to build sub-aggregations, as the worst cases for this that we've seen involve needing to build deep sub-aggregation trees. Checking at sub-aggregation construction time also provides a conveniently centralized method call to add the check to.

---------

Co-authored-by: elasticsearchmachine <infra-root+elasticsearchmachine@elastic.co>
Co-authored-by: Nik Everett <nik9000@gmail.com>
@not-napoleon not-napoleon added :Analytics/Aggregations Aggregations >bug auto-merge-without-approval Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) backport Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) labels Feb 6, 2025
@elasticsearchmachine elasticsearchmachine merged commit e0dac2e into elastic:9.0 Feb 6, 2025
16 checks passed
@not-napoleon not-napoleon deleted the backport/9.0/pr-120944 branch February 6, 2025 17:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

:Analytics/Aggregations Aggregations auto-merge-without-approval Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) backport >bug Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) v9.0.1

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants