andygrove commented on code in PR #6014: URL: https://github.com/apache/datafusion-comet/pull/6014#discussion_r4048879132
########## docs/source/contributor-guide/memory_management.md: ########## @@ -289,6 +388,29 @@ hard ceiling on the sum of everything in the container. That cgroup counts, amon - Comet's JVM-side Arrow buffers (`CometArrowAllocator`), - page cache charged to the cgroup by the container's file I/O, including spill files. +Everything the cgroup counts, and who accounts for each part: + +```mermaid +flowchart TB + subgraph CG["pod cgroup memory.max, kernel OOM kill above this"] + subgraph SEEN["visible to Spark's accounting"] + HEAP["JVM heap<br>execution and storage<br>spark.executor.memory"] + TUNG["Spark Tungsten off-heap<br>TaskMemoryManager"] + SHUFP["Comet JVM shuffle pages<br>CometUnifiedShuffleMemoryAllocator"] + end + subgraph DECL["declared to Comet's native pool only"] Review Comment: You're right, and the diagram contradicted this page's own "the budget is shared even though the memory is not" paragraph a few sections up. I've moved the reservation node into the Spark-visible group and labelled it `declared to Spark over JNI, never measured`, which keeps allocation location and budget accounting distinct without needing a group of its own. The caption underneath now says Spark's accounting covers that group in two different senses: the JVM heap, Tungsten pages and shuffle pages are reported by the JVM code that allocated them, whereas a native reservation is a number an operator declared before allocating, so the budget really is spent but the reservation is only a lower bound on the bytes behind it. Three other places made the same claim, so I fixed those too. The visibility cell for the native allocator in the "Who allocates what" table now reads "Reservations only" rather than "No", the native-heap bullet points forward to the shared-budget paragraph instead of stopping at "no JVM metric counts it", and the overview no longer says native allocations are invisible to Spark's off-heap accounting. Rebuilt with Sphinx: still 61 warnings, unchanged from before the edit, both fences still render as diagrams rather than code blocks, and prettier is clean. -- 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]
