andygrove opened a new pull request, #2143:
URL: https://github.com/apache/datafusion-ballista/pull/2143

   # Which issue does this PR close?
   
   N/A - no issue filed for this. It is a small observability fix found while 
reading spill logs.
   
   # Rationale for this change
   
   The sort shuffle writer spills for two independent reasons:
   
   1. The runtime `MemoryPool` rejects a reservation grow, meaning the executor 
as a whole is out of memory budget.
   2. The per-task buffered-bytes budget 
(`ballista.shuffle.sort_based.memory_limit_per_task_bytes`) is reached, meaning 
this one writer hit its own configured cap while the pool still had room.
   
   Both cases logged the same thing:
   
   ```
   Sort shuffle spill: job=... stage=... input_partition=... spilled N bytes in 
M batches (K events) under memory pressure; ...
   ```
   
   "Under memory pressure" is only accurate for the first case, and the two 
have different remedies. Pool rejection means the executor needs more memory or 
fewer concurrent tasks. Budget exhaustion means `memory_limit_per_task_bytes` 
is set too low for this workload. Anyone tuning a spilling query had no way to 
tell which one they were looking at.
   
   # What changes are included in this PR?
   
   - Add a `SpillTrigger` enum (`MemoryPool`, `TaskBudget`, `Both`) with a 
`classify` constructor. The spill condition in the write loop is now driven by 
`SpillTrigger::classify(...)`, so the branch that decides to spill and the 
cause that gets logged come from the same value and cannot drift apart. Both 
triggers can fire on the same input batch, hence the `Both` variant rather than 
picking a winner.
   - Add `SpillTriggerCounts` to tally events per trigger over the life of the 
task, rendering only the triggers that actually fired.
   - The end-of-task WARN now reports the breakdown and the configured budget:
   
   ```
   Sort shuffle spill: job=j1 stage=2 input_partition=0 spilled 812345 bytes in 
24 batches (6 events); triggered by: memory pool rejection x4, per-task buffer 
budget x2 (per-task buffer budget: 268435456 bytes); repart_time=... 
spill_time=... write_time=...
   ```
   
     The budget renders as `disabled` when `memory_limit_per_task_bytes` is 0, 
which is the documented way to leave the pool as the sole trigger.
   - The per-event DEBUG line also names its trigger.
   - Correct the doc comments in `spill.rs` and on the `spill_count` metric 
that repeated the same inaccurate "memory pressure" claim.
   - Two unit tests cover the classification matrix and the summary rendering, 
including an assertion that the two causes never render with identical wording.
   
   # Are there any user-facing changes?
   
   Log output only. The WARN emitted when a sort shuffle task spills has a new 
format and names the trigger. No public API, configuration, or behavior 
changes. Anything parsing that log line for the literal string "under memory 
pressure" would need updating.
   


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

Reply via email to