Include 'time_series' aggregation tests #351
Conversation
martijnvg
left a comment
martijnvg
left a comment
There was a problem hiding this comment.
This looks good. I do think we need to add: https://github.com/elastic/rally-tracks/pull/348/files#diff-fd03625f346296b8349bb933b96aca0e9bd6d6144a954667c44f25e246b67e79R7
Otherwise we run into maximum number of allowed bucket limit while running the tsdb track.
| "size": 1000 | ||
| "aggs": { | ||
| "ts": { | ||
| "time_series": { |
There was a problem hiding this comment.
I think we should also add a pipeline aggregator here. Since the next change, we're trying to do is pushing down the pipeline aggregator computation to time series aggregator on data node. With that change the response of the time_series agg would change too, it will not return time series buckets but the result of the associated pipeline aggregations.
I think a sum pipeline aggregator is ok here.
martijnvg
left a comment
martijnvg
left a comment
There was a problem hiding this comment.
I think the max bucket limit needs to be increased here, LGTM otherwise.
DJRickyB
left a comment
DJRickyB
left a comment
There was a problem hiding this comment.
Minor suggestion, otherwise this LGTM
Co-authored-by: Rick Boyd <[email protected]>
This reverts commit 1209733.
3 participants
Include Rally tests for
time_seriesaggregations.Resolves #350