andygrove opened a new issue, #6032: URL: https://github.com/apache/datafusion-comet/issues/6032
## What is the problem the enhancement solves? [#6014](https://github.com/apache/datafusion-comet/pull/6014) and [#6029](https://github.com/apache/datafusion-comet/pull/6029) established, in the contributor guide, where Comet's memory actually lives and which configuration value sizes each part of the executor container. Two of those facts are the ones users get wrong, and neither appears anywhere in the [Tuning Guide](https://datafusion.apache.org/comet/user-guide/latest/tuning.html#memory-tuning): 1. `spark.memory.offHeap.size` is part of the container budget, not headroom on top of it. Raising it to give Comet room raises the pod's memory request by the same amount. 2. `spark.executor.memoryOverhead` is the container's only slack and it sizes no budget any consumer can draw on. It is what absorbs Comet's native usage beyond what its operators reserved. The tuning guide currently opens the memory section with "specify how much memory Comet can use in addition to memory already allocated to Spark", which reads as though Comet's budget sits outside what the container already accounts for. That framing is exactly the misconception the contributor-guide diagram was changed to prevent, and the audience that needs it most never reads the contributor guide. The practical consequence is that the guide gives no way to choose between the three settings that respond to an executor OOM kill, and they do not behave alike. Raising `spark.executor.memoryOverhead` widens the container and leaves Comet's budget alone. Lowering `spark.comet.exec.memoryPool.fraction` leaves the container alone and shrinks what Comet may reserve. Raising `spark.memory.offHeap.size` does both, which is the intuitive response and the least reliable one, because it also raises the ceiling on the unaccounted overshoot that caused the kill. The guide also never says what an OOM looks like from each of the three budgets, so there is nothing to tell a user whether the setting they are about to change is the one that was exceeded. Separately, the `fair_unified` description in the tuning guide documents behavior the pool no longer has. It says the pool "prevents operators from using more than an even fraction of the available memory (i.e. `pool_size / num_reservations`)", but since the DataFusion 53 upgrade the check is against the pool-wide total, so the whole task is capped at `pool_size / num_consumers`. That is [#5961](https://github.com/apache/datafusion-comet/issues/5961), which is a real regression rather than intended behavior, so the description should stay and carry a note about what happens today. ## Describe the potential solution Extend the Memory Tuning section of `docs/source/user-guide/latest/tuning.md` with: - the container sizing sum, and the two facts above stated against it; - a comparison of how the three settings move the container and Comet's budget in different directions; - a short table of what each of the three budgets looks like when it is exceeded, so a user can tell a container kill from heap exhaustion from a task-level `SparkOutOfMemoryError`; - a note on the `fair_unified` description pointing at #5961; - a link to the contributor guide's [Memory Management](https://datafusion.apache.org/comet/contributor-guide/memory_management.html) page for the full accounting picture. ## Additional context The sizing sum itself is Spark's, from `ResourceProfile.getResourcesForClusterManager`, and is already stated in the contributor guide. -- 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]
