Skip to content
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

Only execute one final reduction in InternalAutoDateHistogram #45359

Merged
merged 4 commits into from
Aug 12, 2019

Conversation

polyfractal
Copy link
Contributor

Because auto-date-histo can perform multiple reductions while merging buckets, we need to ensure that the intermediate reductions are done with a finalReduce set to false to prevent Pipeline aggs from generating their output.

This is done by creating a "fake" non-final reduce context for the intermediate merging. Once all the buckets have been merged and the output is stable, a mostly-noop final reduction can be performed which will allow pipelines to generate their output.

Closes #44914

Because auto-date-histo can perform multiple reductions while
merging buckets, we need to ensure that the intermediate reductions
are done with a `finalReduce` set to false to prevent Pipeline aggs
from generating their output.

Once all the buckets have been merged and the output is stable,
a mostly-noop reduction can be performed which will allow pipelines
to generate their output.
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-analytics-geo

Copy link
Contributor

@pcsanwald pcsanwald left a comment

Choose a reason for hiding this comment

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

one small comment but the rest of it LGTM

@polyfractal
Copy link
Contributor Author

@elasticmachine update branch

@polyfractal polyfractal merged commit e8e20c9 into elastic:master Aug 12, 2019
polyfractal added a commit that referenced this pull request Aug 12, 2019
Because auto-date-histo can perform multiple reductions while
merging buckets, we need to ensure that the intermediate reductions
are done with a `finalReduce` set to false to prevent Pipeline aggs
from generating their output.

Once all the buckets have been merged and the output is stable,
a mostly-noop reduction can be performed which will allow pipelines
to generate their output.
@polyfractal
Copy link
Contributor Author

I reverted this commit because testReduceRandom() was consistently showing incorrect bucket counts compared to the max_buckets breaker in about 1/100 tests.

Unsure of the cause, but it was starting to look like it's not just a test bug but an actual problem with the PR. Will revisit and open a new PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

auto_date_histogram fails where date_histogram does not
3 participants