gianm commented on PR #13206: URL: https://github.com/apache/druid/pull/13206#issuecomment-1280032437
> Can we go further? Any reason to create buckets that will be empty? If we're doing time-based grouping, and data is time ordered, we can create buckets on the fly as we read the data. That's exactly what I meant by "let the cursor drive the bucketing". I meant only generate buckets based on timestamps we actually see. (Unless the user specifically requests that empty buckets be zero-filled.) > For this, the set of buckets need not be materialized, just enumerated. In general, today, we don't materialize the buckets; we just enumerate them. The issue we see is that even with this approach, the amount of time it takes to enumerate buckets can be prohibitively large. (People raise bugs saying that queries "hang". They don't actually hang, but it seems that way to the user, due to the large number of buckets that are being enumerated.) By letting the cursor drive the bucketing, we can avoid this completely for the case where we aren't zero-filling. The zero-fill case still poses an issue, but we can address that some other way (limit on number of zero-filled buckets)? > For fixed-length intervals, (week or less), the time floor should be a simple mod-then-subtract. For variable-length intervals, the logic is more complex. Can we split the implementations for those two cases? Week-or-less is super fast, Month-or-more pays the extra compute cost? Ah. In practice the main perf hit isn't from the evaluation speed of the `timestamp_floor` function itself. It does have some logic to fast-path common granularities in the way you mention. The bigger issues are that we don't take advantage of the fact that the segments are sorted by `__time` and the `timestamp_floor` function is monotonic. Two things we should do in order to get perf matching the way we handle `granularity`: - the group-by engine should do a streaming aggregation when the first group-by dimension is `timestamp_floor(__time, ...)` - the computation of `timestamp_floor` should take advantage of monotonicity: when we encounter a timestamp, we can compute the start and end of its bucket. The start should be re-used as the return value of `timestamp_floor` for any value of `__time` up til the end: there is no need to actually compute the function for each row. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
