andygrove opened a new pull request, #6163:
URL: https://github.com/apache/datafusion-comet/pull/6163

   ## Which issue does this PR close?
   
   Part of #4576. Stacked on #6162, so the diff includes that PR's commits 
until it merges. Only the last commit, `chore: deprecate 
spark.comet.exec.memoryPool.fraction`, belongs to this PR.
   
   ## Rationale for this change
   
   The tuning guide recommends setting `spark.comet.exec.memoryPool.fraction` 
below `1.0` to leave room in the off-heap pool for native memory that Comet 
does not reserve. It cannot do that:
   
   - `greedy_unified` ignores the fraction. It is built with a pool size of 0 
and asks Spark for every byte it reserves.
   - `fair_unified` applies it to each task's pool, limiting each memory 
consumer in the task to `offHeap.size * fraction / num_consumers`. Spark's 
execution pool already limits each of N running tasks to `offHeap.size / N`, 
which is tighter whenever more than one task is running. So on a busy executor 
the fraction never binds, and the tasks together can still acquire the whole 
pool.
   - Spark's own off-heap consumers, non-Comet operators and off-heap storage, 
draw on the same pool with no Comet limit.
   
   The only room Spark leaves for memory outside the pool is 
`spark.executor.memoryOverhead`, and #6162 adds an executor log that measures 
how much of it Comet needs.
   
   ## What changes are included in this PR?
   
   - `spark.comet.exec.memoryPool.fraction` is documented as deprecated, with 
what it actually does. Its behaviour is unchanged: several tests set it very 
low to force spills through the `fair_unified` per-task cap.
   - The driver plugin logs a deprecation warning when the setting is in the 
SparkConf, pointing at `spark.executor.memoryOverhead`.
   - The tuning guide and the memory management contributor guide stop 
recommending it and explain why it does not create headroom.
   
   ## How are these changes tested?
   
   New `CometPluginsMemoryPoolFractionWarningSuite` covers the warning being 
logged when the setting is present and not otherwise. It is registered in both 
the Linux and macOS PR workflows. The rest is documentation.
   


-- 
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]

Reply via email to