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]