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

Aggregations: Adds a new GapPolicy - NONE #11951

Closed
wants to merge 1 commit into from
Closed

Aggregations: Adds a new GapPolicy - NONE #11951

wants to merge 1 commit into from

Conversation

colings86
Copy link
Contributor

This is to allow the skip gap policy to always skip execution if values are missing. The none gap policy will set the value of the missing metric to NaN. In practice the skip and non gap policies work the same in all pipeline aggregators except the bucket_script agg, where skip will not execute the script for buckets where any of the input metrics are missing, and none will pass Double.NaN into the skip as the value of these missing metrics

This is to allow the skip gap policy to always sip execution if values are missing. The none gap policy will set the value of the missing metric to NaN. In practice the skip and non gap policies work the same n all pipeline aggregators except the bucket_script agg where skip will not execute the script for buckets where any of the input metrics are missing, and none will pass Double.NaN into the skip as the value of these missing metrics
@jpountz
Copy link
Contributor

jpountz commented Jun 30, 2015

I'm not too happy with having both none and skip given how close they are. I think we need to pick one.

@clintongormley
Copy link

@jpountz If we have to choose one then it should be skip, as this will be the most common use case for scripting. However, this will prevent users of bucket_scripts from processing buckets with truly null values (distinct from zeroes).

We don't know yet whether this second use case is valid or not. I'd suggest leaving this PR as stalled for now, and we can always revisit it if users need it.

@colings86
Copy link
Contributor Author

@clintongormley we cannot leave this PR as stalled as skip does not work as we intend it to for the bucket_script aggregation currently (it passes NaN into the script rather than skipping execution). Would you rather that I change this PR to just make bucket_script not execute the script if any of the variables are missing in the bucket?

@colings86
Copy link
Contributor Author

Skip gap policy has been fixed in #11970. Closing this for now as we need to think about whether other gap policies are required

@colings86 colings86 closed this Jul 1, 2015
@kevinkluge kevinkluge removed the review label Jul 1, 2015
@colings86 colings86 deleted the enhancement/noneGapPolicy branch July 10, 2015 12:11
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.

None yet

4 participants