rkrishn7 commented on issue #24603: URL: https://github.com/apache/datafusion/issues/24603#issuecomment-6699745307
> I think the current groups accumulator physical design is only optimized for throughput, so it's really hard to make eviction efficient. Reusing the `GroupsAccumulator` s can make both batched and streaming execution harder to evolve. @2010YOUY01 I think you're right that it's quite hard to make eviction efficient. It might be possible with primitive state that gets stored in something like a Vec/bitmap. But it gets harder with cases like variable-width state (e.g. values addressed by offsets into a shared buffer). > The existing Accumulator interface seems even more suitable than GroupsAccumulator to extend for streaming use cases. Unfortunately we'd lose most of the advantages of `GroupsAccumulator` that way. For high-volume, append-only streams, throughput and memory per group matter as much as in batch execution, and the overhead of one `Accumulator` per group adds up as the number of groups grows. We saw this in #27150 which replaces the per-group `Accumulator`s behind `GroupsAccumulatorAdapter` with a native `GroupsAccumulator`. However, per-group removal may be the wrong primitive to expose, given how many implementations lay out their state for throughput. TTL (which prompted this issue) can be handled in other ways. Though, I still think streaming execution benefits a lot from `GroupsAccumulator`, so I don't want to give up on it entirely. Will think a bit more if there's a narrower API we could expose that's helpful here. -- 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]
