[I] Sort shuffle writer ignores memory-pool rejection and never spills under pool pressure [datafusion-ballista]

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

andygrove opened a new issue, #2060:
URL: https://github.com/apache/datafusion-ballista/issues/2060

   `SortShuffleWriterExec` registers its `MemoryConsumer` with `.with_can_spill(true)`, but discards the result of the reservation grow ([`writer.rs`](https://github.com/apache/datafusion-ballista/blob/main/ballista/core/src/execution_plans/sort_shuffle/writer.rs)):
   
   ```rust
   // Mirror the growth in the runtime pool reservation so the pool
   // sees this writer's memory usage. Best-effort: if the pool is
   // bounded and rejects the grow, that's fine — the absolute
   // counter below still triggers a spill.
   let _ = reservation.try_grow(growth);
   buffered_bytes = buffered_bytes.saturating_add(growth);
   
   if buffered_bytes >= memory_limit {
       // spill
   }
   ```
   
   The comment argues the rejection is safe to ignore because `buffered_bytes` still bounds RSS. That reasoning does not hold: `buffered_bytes` is a private per-task counter compared against `memory_limit_per_task_bytes`. It knows nothing about the state of the shared `MemoryPool`, so a rejected grow produces **no spill at all**.
   
   Two consequences follow.
   
   **1. The writer under-reports its usage to the shared pool.** The batch has already been pushed into `BufferedBatches` by this point. When the grow is rejected, the writer holds that memory while the pool has no record of it, so other consumers sharing the pool see bytes as available that are not. This is the same class of accounting gap as #2031, but self-inflicted rather than caused by an untracked allocator.
   
   **2. Pool pressure produces no spill.** Registering with `can_spill(true)` is a contract: the pool rejects a grow to ask the consumer to spill. This writer declines to respond, so the only thing that can ever trigger a spill is its own per-task budget.
   
   This directly undermines the approach proposed in #2031, whose remedy is to *\"reject memory-pool growth before real usage exceeds the budget, so DataFusion spills instead of OOMing\"*. That guard cannot work on this operator: the writer buffers straight through a rejection.
   
   ### Reproduction
   
   Give the writer a per-task budget far above the payload (so the private counter can never fire) and a pool too small to admit a single batch, so every grow is rejected. The writer spills zero times and buffers the entire input.
   
   ### Proposed fix
   
   Treat a rejected grow as a spill trigger alongside the existing per-task budget. Spilling flushes the buffered batches to disk and frees the reservation, so no retry is needed — the writer holds nothing afterwards.


-- 
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.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: [I] Sort shuffle writer ignores memory-pool rejection and never spills under pool pressure [datafusion-ballista]

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

andygrove closed issue #2060: Sort shuffle writer ignores memory-pool rejection and never spills under pool pressure
URL: https://github.com/apache/datafusion-ballista/issues/2060


-- 
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