Skip to content
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 17 additions & 2 deletions docs/source/contributor-guide/memory_management.md
Original file line number Diff line number Diff line change
Expand Up @@ -390,13 +390,18 @@ 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:
Everything the cgroup counts, which configuration value sizes it, and who accounts for each part:

```mermaid
flowchart TB
subgraph CFG["what you configure, summing to the pod limit"]

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correctness

[P2] Account for the optional PySpark allowance in the sum

Could you include spark.executor.pyspark.memory as a conditional input, or scope this caption to applications without that allowance? Spark's ResourceProfile.getResourcesForClusterManager adds the configured PySpark amount for a Python application, and the executor container's request and limit use that total. The formula immediately above this diagram already includes it.

For a Python application with 4 GiB heap, 1 GiB overhead, 8 GiB off-heap and 2 GiB PySpark memory, the container limit is 15 GiB. These three inputs sum to 13 GiB. The new unconditional caption therefore understates the configured limit for that supported case.

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<br>spark.executor.memory"]
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"]
Expand All @@ -409,6 +414,11 @@ flowchart TB
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

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correctness

[P2] Qualify the fraction edge for fair_unified

Could you mark this scaling as applying to fair_unified only? The page also covers greedy_unified, but the parser discards the scaled limit for that pool. Its factory passes no size to CometUnifiedMemoryPool, whose try_grow delegates directly to Spark.

For example, with an 8 GiB off-heap pool and fraction 0.5, greedy_unified does not impose the 4 GiB limit this edge suggests. Lowering the fraction therefore provides no additional headroom in that supported mode. I confirmed the parser behavior for fractions 0.25, 0.5 and 1.0 using the actual source. Please qualify the label or explicitly show that greedy_unified uses Spark's shared limit without the fraction.

MO -.->|"no budget, just slack"| NONE
```

Spark's accounting covers the first group, though not in the same sense throughout it. The JVM
Expand All @@ -418,6 +428,11 @@ succeeds only once `CometTaskMemoryManager` has charged Spark's off-heap executi
the budget really is spent, but nothing measured the bytes and the reservation is only a lower bound
on them. The second group is outside every accounting layer.

The configuration maps onto those regions unevenly. `spark.memory.offHeap.size` alone sizes three of
them, 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 that any
consumer can draw on, and only widens the container far enough to absorb the second group.

When the total crosses `memory.max`, the kernel OOM killer kills the process. The failure mode is
significantly worse than a task-level OOM: every task running on that executor dies, every cached
block it held is lost and must be recomputed, and the shuffle files it produced become unavailable
Expand Down