-
Notifications
You must be signed in to change notification settings - Fork 24.5k
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
Fix a sneaky bug in rare_terms #51868
Conversation
Pinging @elastic/es-analytics-geo (:Analytics/Aggregations) |
When the `rare_terms` aggregation contained another aggregation it'd break them. Most of the time. This happened because the process that it uses to remove buckets that turn out not to be rare was incorrectly merging results from multiple leaves. This'd cause array index out of bounds issues. We didn't catch it in the test because the issue doesn't happen on the very first bucket. And the tests generated data in such a way that the first bucket always contained the rare terms. Randomizing the order of the generated data fixed the test so it caught the issue. Closes elastic#51020
0bd711c
to
ba913e3
Compare
The bwc test failure is because I haven't backported the fix. I'll suppress it. |
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.
Sneaky sneaky! Thanks for tracking this down! ❤️
Thanks for taking care of this this quickly! |
When the `rare_terms` aggregation contained another aggregation it'd break them. Most of the time. This happened because the process that it uses to remove buckets that turn out not to be rare was incorrectly merging results from multiple leaves. This'd cause array index out of bounds issues. We didn't catch it in the test because the issue doesn't happen on the very first bucket. And the tests generated data in such a way that the first bucket always contained the rare terms. Randomizing the order of the generated data fixed the test so it caught the issue. Closes elastic#51020
When the `rare_terms` aggregation contained another aggregation it'd break them. Most of the time. This happened because the process that it uses to remove buckets that turn out not to be rare was incorrectly merging results from multiple leaves. This'd cause array index out of bounds issues. We didn't catch it in the test because the issue doesn't happen on the very first bucket. And the tests generated data in such a way that the first bucket always contained the rare terms. Randomizing the order of the generated data fixed the test so it caught the issue. Closes elastic#51020
When the `rare_terms` aggregation contained another aggregation it'd break them. Most of the time. This happened because the process that it uses to remove buckets that turn out not to be rare was incorrectly merging results from multiple leaves. This'd cause array index out of bounds issues. We didn't catch it in the test because the issue doesn't happen on the very first bucket. And the tests generated data in such a way that the first bucket always contained the rare terms. Randomizing the order of the generated data fixed the test so it caught the issue. Closes #51020
When the `rare_terms` aggregation contained another aggregation it'd break them. Most of the time. This happened because the process that it uses to remove buckets that turn out not to be rare was incorrectly merging results from multiple leaves. This'd cause array index out of bounds issues. We didn't catch it in the test because the issue doesn't happen on the very first bucket. And the tests generated data in such a way that the first bucket always contained the rare terms. Randomizing the order of the generated data fixed the test so it caught the issue. Closes #51020
I'm finished backporting but I'm leaving the |
Now that elastic#51868 is fully backported we can run its tests in the backwards compatibility tests.
Now that elastic#51868 is fully backported we can run its tests in the backwards compatibility tests.
Now that #51868 is fully backported we can run its tests in the backwards compatibility tests.
Now that #51868 is fully backported we can run its tests in the backwards compatibility tests.
All done! |
When the
rare_terms
aggregation contained another aggregation it'dbreak them. Most of the time. This happened because the process that it
uses to remove buckets that turn out not to be rare was incorrectly
merging results from multiple leaves. This'd cause array index out of
bounds issues. We didn't catch it in the test because the issue doesn't
happen on the very first bucket. And the tests generated data in such a
way that the first bucket always contained the rare terms. Randomizing
the order of the generated data fixed the test so it caught the issue.
Closes #51020