andygrove opened a new pull request, #6250:
URL: https://github.com/apache/datafusion-comet/pull/6250
## Which issue does this PR close?
No issue. This replaces #5998, which was the first item of #5997 and took a
different approach. The notes on #5997 explain why that approach was dropped.
#6167, which asks for resident memory in the same log line, is related.
## Rationale for this change
Comet's JVM-side Arrow buffers (`CometArrowAllocator`) are off-heap memory
that no memory pool tracks, so they have to fit in
`spark.executor.memoryOverhead`. The memory tuning guide sizes that overhead
from the executor's periodic memory usage log. The log reported only native
allocation and pool reservations, though, so the JVM Arrow part was left to the
margin.
#5998 tried to charge these buffers to Spark's off-heap pool instead. That
turned native spills into task failures: Spark cannot make Comet's native
consumer give memory back, so the JVM allocation that feeds a spilling native
operator its next batch was refused first. This PR makes the memory visible
instead of bounding it.
## What changes are included in this PR?
- Each memory usage log line also reports the Arrow memory Comet holds on
the JVM side: the total charged to `CometArrowAllocator`, and the part of it
charged to `CometArrowImportAllocator`. These are the figures tracing already
reports as `jvm_arrow_allocated` and `jvm_arrow_imported`. A line from a local
run:
```
Comet native memory usage: allocated 34.8 MiB, reserved 66.2 MiB (4 native
plans, 4 memory pools); JVM Arrow allocated 18.1 MiB, 0.0 MiB of it imported
from native
```
- The container warning now counts the Arrow memory the JVM allocated
itself, which is the total less the imported part. Imported buffers are left
out because native code allocated them, so `allocated` already counts them.
- The JVM figure is added before the pools' reservations are subtracted. A
native operator that holds on to a JVM batch, such as a sort buffering a cached
scan's output, reserves it, so those bytes are in both the JVM figure and
`reserved` but not in `allocated`.
- Subtracting first and adding the JVM figure afterwards would count them
twice. In the line above, `reserved` is larger than `allocated` partly for this
reason.
- The memory tuning guide's sizing recipe uses the same definition of
untracked memory, and no longer leaves JVM Arrow buffers to the margin.
- The `spark.comet.memory.logInterval` description, the memory management
guide and the tracing guide mention the new figures. The memory management
guide also records why charging these buffers to Spark's pool was dropped.
## How are these changes tested?
- `CometExecIteratorLifecycleSuite` covers the change.
- The log line includes the JVM Arrow figures.
- `JvmArrowMemory.of` reads a root allocator and its import child, keeping
the imported part apart.
- The warning counts Arrow memory the JVM allocated, leaves imported
memory to `allocated`, and counts a JVM batch that native reserves only once.
Putting back the subtract-then-add formula fails that last case.
- I checked the output end to end with a local run: 4 tasks, a sort within
partitions over a Comet-cached table, and `spark.comet.memory.logInterval=1s`.
It logged the line quoted above.
--
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]
