[PR] feat: report JVM Arrow memory in the executor memory usage log [datafusion-comet]

["andygrove (via GitHub)" <gi...@apache.org>]

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: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org

Re: [PR] feat: report JVM Arrow memory in the executor memory usage log [datafusion-comet]

["andygrove (via GitHub)" <gi...@apache.org>]

andygrove merged PR #6250:
URL: https://github.com/apache/datafusion-comet/pull/6250


-- 
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: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org