kumarlokesh commented on PR #15594: URL: https://github.com/apache/datafusion/pull/15594#issuecomment-2784505817
> Hello @kumarlokesh. Thank you for working on this. I have 2 questions/concerns. Let's discuss on them a bit to get a future-proof design > > 1. There are also `runtime_env: Arc<RuntimeEnv>` in `SessionState` and `TaskContext` structs, and this `RuntimeEnv` has a field of `pub memory_pool: Arc<dyn MemoryPool>`. These MemoryPool implementations have some configurations about memory limit as well. Keeping 2 different things for the same purposes always bring trouble. So, I wonder can we somehow connect those 2 points. > 2. Do you plan to practice this configuration in any part of the execution? While bringing such a feature, implementing at least one use-case makes things more clear and becomes a guidance for the further developments. @berkaysynnada Thank you for the feedback! 1. Have revised the PR to ensure runtime environment configuration like memory-limit is exposed through the already defined RuntimeEnv -> memory_pool setting. 2. As a user can set the configuration `datafusion.runtime.memory_limit` (and other available runtime environment configurations) through SQL interface, I think having tests around this behaviour should be a good starting point. Can't think of another good use case besides this. Thoughts? -- 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: github-unsubscr...@datafusion.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org For additional commands, e-mail: github-h...@datafusion.apache.org
