2010YOUY01 commented on PR #22862: URL: https://github.com/apache/datafusion/pull/22862#issuecomment-4668820522
> > AggregateExec is the only violation I'm aware of today, though I'm not sure such a property can always be guaranteed given the diverse requirements of downstream projects. > > I also think we should design the memory accounting changes more intentionally, understand how we want the memory accounting feature to work and deduplicate the issues, e.g. I've raised a similar issue in #22526 Yes, I agree. I believe the root cause of many existing memory-limited query bugs is that the current memory tracking protocol is ambiguous, and the behavior of operators and the memory pool is inconsistent. We should definitely think through a better design for both operators and the memory pool, and then specify a clear protocol, so we can coordinate efforts around this issue. > For this particular issue I think `get_sliced_size` would be a better option, see: [#22526 (comment)](https://github.com/apache/datafusion/issues/22526#issuecomment-4621836931) I'm not sure which solution is better at this point. This requires deeper, more holistic thinking about the overall spilling-query design and may challenge some of our existing assumptions. I'll spend more time thinking about it later. Regarding this PR, I don't think it a major architectural commitment. Even if we later decide to switch to a `get_sliced_size` approach, that change can be easily made entirely within this module. It also doesn't seem to make the system worse in the meantime. So I'm inclined to merge it first, while leaving it open for a few more days for additional discussion. -- 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]
