-
Notifications
You must be signed in to change notification settings - Fork 25.6k
Aggregations cancellation after collection #120944
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
Merged
not-napoleon
merged 25 commits into
elastic:main
from
not-napoleon:aggregations-cancellation-after-collection
Feb 6, 2025
Merged
Changes from all commits
Commits
Show all changes
25 commits
Select commit
Hold shift + click to select a range
b78f195
can it really be this simple?
not-napoleon 5751f45
Update docs/changelog/120944.yaml
not-napoleon ca451b8
More correct exception type
not-napoleon 18870c4
[CI] Auto commit changes from spotless
250bc9b
clean up some deprecations
not-napoleon 696fdfc
Merge remote-tracking branch 'refs/remotes/not-napoleon/aggregations-…
not-napoleon 32f079c
[CI] Auto commit changes from spotless
a4cabaf
Aggs: test super deep
nik9000 ec66b50
add cancellation case to aggs tests
not-napoleon 8181260
Merge remote-tracking branch 'refs/remotes/not-napoleon/aggregations-…
not-napoleon bdc6516
some testing around reduce time cancellation
not-napoleon 7eb1966
[CI] Auto commit changes from spotless
7e213b6
remove the nocommits
not-napoleon 6454286
Merge remote-tracking branch 'refs/remotes/not-napoleon/aggregations-…
not-napoleon a0485b5
Merge branch 'main' into aggregations-cancellation-after-collection
not-napoleon c8024e7
Fix license
nik9000 0231c5a
Merge remote-tracking branch 'refs/remotes/not-napoleon/aggregations-…
nik9000 7d811a0
Update docs/changelog/120944.yaml
not-napoleon 4a45481
oops, something was using that one
not-napoleon f2b240f
allow tests to opt out of reduce cancellation testing
not-napoleon 6f51925
Merge branch 'main' into aggregations-cancellation-after-collection
not-napoleon 8359e4e
Merge branch 'main' into aggregations-cancellation-after-collection
nik9000 2869310
Explain
nik9000 dc526a8
Merge remote-tracking branch 'refs/remotes/not-napoleon/aggregations-…
nik9000 e11ac1e
Merge branch 'main' into aggregations-cancellation-after-collection
not-napoleon File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| pr: 120944 | ||
| summary: Aggregations cancellation after collection | ||
| area: Aggregations | ||
| type: bug | ||
| issues: | ||
| - 108701 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: It is not clear to me why we are handling auto-date histogram in a special way. Could we add a comment explaining the why?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure! Its just that it spends a while spinning around doing extra stuff before it's ready for the normal build.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And I found it from an actual bug report of folks that were trying to terminate aggs and the thread was stuck here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you have not one but three typos in that sentence :D