sunchao commented on code in PR #6014: URL: https://github.com/apache/datafusion-comet/pull/6014#discussion_r4048627256
########## 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: ### Correctness [P2] Show native reservations as charged to Spark's accounting Could we place `NATRES` under Spark-visible accounting, or label it as visible to both Spark and Comet? This page covers off-heap mode, where both `fair_unified` and `greedy_unified` forward successful `try_grow` reservations through JNI to [`CometTaskMemoryManager.acquireMemory`](https://github.com/apache/datafusion-comet/blob/0f9cbaa225b82ffb6e6e9d79c4235004cb30e0d9/spark/src/main/java/org/apache/spark/CometTaskMemoryManager.java#L55-L68). That calls Spark's `acquireExecutionMemory` and updates the native consumer's reported usage. For example, a successful 128 MiB native reservation consumes 128 MiB of Spark's off-heap execution budget, even though Rust allocates the actual buffers. The current separate “Comet's native pool only” group tells readers that these reservations are invisible to Spark, contradicting the earlier shared-budget explanation and allocator diagram. The unreserved native allocations belong outside Spark's accounting, but the declared reservations do not . Please make the new “no JVM metric counts it” paragraph consistent with this distinction too. -- 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]
