andygrove opened a new pull request, #6029:
URL: https://github.com/apache/datafusion-comet/pull/6029
## Which issue does this PR close?
Closes #6028.
## Rationale for this change
The cgroup diagram added in #6014 groups everything the container counts by
who accounts for it,
but says nothing about which setting sizes each region. That leaves the two
facts the section goes
on to state as prose only: that `spark.memory.offHeap.size` is inside the
pod limit rather than
headroom on top of it, and that `spark.executor.memoryOverhead` is the
container's only slack.
Both are properties of how the configuration maps onto the regions, and the
mapping is uneven in a
way that is easy to get wrong. `spark.memory.offHeap.size` alone sizes three
regions, including
Comet's native reservations, which are neither off-heap in Spark's sense nor
allocated by the JVM.
`spark.executor.memoryOverhead` sizes none of them; it buys no budget any
consumer can draw on and
only widens the container enough to absorb the unaccounted group.
## What changes are included in this PR?
Three configuration values are added as inputs to the existing diagram,
rather than adding a third
diagram to the page:
```mermaid
flowchart TB
subgraph CFG["what you configure, summing to the pod limit"]
EM["spark.executor.memory"]
MO["spark.executor.memoryOverhead"]
OH["spark.memory.offHeap.size"]
end
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"]
TUNG["Spark Tungsten off-heap<br>TaskMemoryManager"]
SHUFP["Comet JVM shuffle pages<br>CometUnifiedShuffleMemoryAllocator"]
NATRES["Comet native heap, reserved<br>operators that call
try_grow<br>declared to Spark over JNI, never measured"]
end
subgraph NONE["accounted by nobody"]
NATUND["Comet native heap, undeclared<br>kernels, array builders,
decompression<br>Parquet metadata, object_store, tokio"]
ARROWR["Comet JVM Arrow<br>CometArrowAllocator, unbounded"]
NONHEAP["JVM non-heap<br>metaspace, code cache, thread stacks<br>GC
structures, Netty direct buffers"]
PAGEC["page cache charged to the cgroup<br>file I/O, including spill
files"]
FRAG["allocator overhead<br>fragmentation, padding<br>jemalloc
retained and dirty pages"]
end
end
EM --> HEAP
OH --> TUNG
OH --> SHUFP
OH -->|"scaled by spark.comet.exec.memoryPool.fraction"| NATRES
MO -.->|"no budget, just slack"| NONE
```
Also:
- a paragraph after the diagram stating the asymmetry the edges show;
- `spark.executor.memory` dropped from the JVM heap node, now that an edge
carries it.
## How are these changes tested?
Docs only, no code paths touched.
`npx prettier "docs/source/contributor-guide/memory_management.md" --check`
passes, which is what
the Preflight job runs.
The diagram is **not** verified under `mmdc`. Since #6021 the docs build
draws these fences with
mermaid-cli, so a syntax error would now fail the build rather than degrade
to raw text, and I could
not install `mmdc` locally. The rendered block above is GitHub drawing the
same source, which
confirms it parses but not that `mmdc` accepts it. Worth a docs build before
merging.
--
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]
