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

Rate aggregation doesn't always use the right bucket size #63703

Closed
nik9000 opened this issue Oct 14, 2020 · 4 comments · Fixed by #65429
Closed

Rate aggregation doesn't always use the right bucket size #63703

nik9000 opened this issue Oct 14, 2020 · 4 comments · Fixed by #65429
Assignees
Labels
:Analytics/Aggregations Aggregations >bug Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo)

Comments

@nik9000
Copy link
Member

nik9000 commented Oct 14, 2020

I was hacking on something vaguely related and ran into problems with the range aggregation calling bucketSize with a bucket that has never been collected. The "keyword sandwitch" test (yum) does this. It'll pass bucket=2 when the rate only contains 2 buckets. I think it is picking up the bucket ordinal from its parent agg - a terms agg.

@nik9000 nik9000 added >bug :Analytics/Aggregations Aggregations needs:triage Requires assignment of a team area label labels Oct 14, 2020
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-analytics-geo (:Analytics/Aggregations)

@elasticmachine elasticmachine added the Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) label Oct 14, 2020
@imotov imotov self-assigned this Oct 15, 2020
@gwbrown gwbrown removed the needs:triage Requires assignment of a team area label label Oct 22, 2020
@imotov imotov changed the title Range aggregation doesn't always use the right bucket size Rate aggregation doesn't always use the right bucket size Oct 29, 2020
@imotov
Copy link
Contributor

imotov commented Nov 11, 2020

I have been investigating this for a few weeks, and would like to add an update and propose some ways forward.

The problem: As you can see from the comment in the original issue in order to correctly calculate the rate we sometimes need to know the actual start value for the bucket from which we can deduce the bucket size. When I implemented the rate aggregation I thought I have a solution, but it turned out the method I found didn't really work. In the situation when we have some intermediate aggregations between date_histogram and rate aggregation it's not possible to determine the current size of the bucket that we calculate the rate for. In other words all numbers marked as QT in the comment linked above are bogus or can cause an exception. Just to clarify it's not an issue if date_histogram is a direct parent of the rate histogram.

The solution: I don't have any good solutions at the moment.

Workarounds: since the real solution might take a while, we would like to figure out what would be the best workaround for variable bucket sizes for grandparents. There are 2 possible ways to approach it:

  1. If we cannot calculate the exact number of days in the interval we can throw an exception saying that this is not supported, please use fixed_interval in the date histogram instead,
  2. We can pretend that all months have 30.4375 days and all years have 365.25 days or something like this and
    a. just documenting it or
    b. throw a deprecation warning that rate might be imprecise since it is based on 30 days month instead of actual number of days in this actual month

cc: @wylieconlon, @dgieselaar, @exekias, @simianhacker

@dgieselaar
Copy link
Member

I'd prefer an exception in this case. FWIW, we have not yet started to use the rate aggregation due to missing histogram support, but when we will use it, it will likely use fixed_interval and be a direct child of the date_histogram aggregation.

@linyaru
Copy link

linyaru commented Nov 23, 2020

some user input from internal analytics: more inclined to agree with option #1 in which an exception is thrown and the onus is put on the user to define the fixed interval

imotov added a commit that referenced this issue Nov 25, 2020
…#65429)

In some cases when the rate aggregation is not a child of a date histogram
aggregation, it is not possible to determine the actual size of the date
histogram bucket. In this case the rate aggregation now throws an exception.

Closes #63703
imotov added a commit that referenced this issue Nov 25, 2020
…#65429) (#65502)

In some cases when the rate aggregation is not a child of a date histogram
aggregation, it is not possible to determine the actual size of the date
histogram bucket. In this case the rate aggregation now throws an exception.

Closes #63703
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Analytics/Aggregations Aggregations >bug Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants