Spill reduce/read profiling split — #280 step 0 (measurement enablement)

[espg]

Refs #280. Step 0: before choosing how to parallelize the spill reducer, the profile has to show whether the fold is reduce-CPU-bound or read-I/O-bound (post-#279 the reduce is vectorized numpy, so this may have flipped).

What

Adds spill_reduce_s to the spill profile — wall spent in the block fold's compute (_group_columns + build_tdigest + merge), split from the existing spill_read_s (read-back I/O). Populated in the multi-block regime (_fold_block); the single-block exact path has no fold.

Why this gates #280

The A/B/pipelining decision hinges on the split (measured on the 158-granule CONUS slab, not 88S):

• If spill_reduce_s ≪ spill_read_s → compute parallelism is pointless; the lever is I/O (output-preserving prefetch pipelining).
• If spill_reduce_s is significant → coupled parallel reduce (reduce_workers=P ⇄ block_bytes/P, since one partition reduce already peaks at ~60% mem) earns its keep.

Also relevant: one partition's reduce peak is ~0.6×mem (_BUILD_MULT=3, partition_bytes=0.2×mem), so partition-parallelism is not free/scheduling-only — it requires shrinking blocks, which changes (in-tolerance) digest bytes. That's why we measure before building.

Tested

• test_multi_block_reduce_time_is_captured (reduce_s > 0 on a forced-multi-block run) + updated test_profile_carries_spill_instrumentation key set. pytest tests/test_spill.py → 50 passed; ruff clean.

Next (pending the measurement)

Run the 158g CONUS slab with this and read the reduce/read split → pick coupled-parallel vs pipelining → implement Flavor A. Not byte-affecting; safe to land as the measurement prerequisite.

[espg]

🤖 from Claude (review)

Reviewed the reduce/read profiling split against #280 step 0's stated intent and the repo conventions. The change is small (+24 lines), correct, and lands the measurement it claims. Verified in detail:

Timing-split correctness — clean. In _fold_block, t1 is taken immediately after spill_read_s is charged, and spill_reduce_s is charged at the end of the per-partition loop. The reduce window therefore covers exactly _group_columns + the build/merge loop, sequential after read_partition — read-wall and reduce-wall are disjoint, with no double-counting and no path folded into the wrong bucket. spill_read_s semantics are unchanged (the group/build compute was previously untimed, not part of spill_read_s), so this is purely additive instrumentation.

Thread-safety — fine. spill_reduce_s is written only inside _fold_block, which runs either on the overlap reducer thread or on the main thread (final still-open block in _chunk_outputs_merged), never concurrently — _join_reducer precedes both the next close and the final fold. The += is GIL-safe with a single writer, and worker.py reads the value only after the aggregate phase completes.

Downstream threading — consistent everywhere. spill_reduce_s is a _s-suffixed float, so it flows generically through worker.py phase_timings, telemetry's seconds-vs-_bytes split (telemetry.py:161), the per-key merge sum (telemetry.py:234), and schema.py's dict[str, float]. No hardcoded keyset anywhere drops it; the exact-set assertion in test_profile_carries_spill_instrumentation was updated to include it, and since worker.py writes the key unconditionally under spill_mode, single-block runs still carry it (value 0.0).

Conventions — met. spill.py is ~795 lines (well under the ~1000 limit), no new dependency, terse and well-commented, naming mirrors spill_read_s/spill_write_s, and a new multi-block test covers the populated path.

One diff-scoped suggestion (non-blocking), inline: the single-block profile test asserts spill_reduce_s >= 0 where it is deterministically 0.0; tightening to == 0.0 would pin the single-block-no-fold half of the split. See the inline comment.

Two awareness notes (out of scope for this PR, for @espg):
(1) spill_reduce_s measures only the multi-block fold's reduce-CPU; the single-block pooled reduce (in the untimed _chunk_outputs_exact machinery) is not captured. This matches the stated #280 target (the 158-granule CONUS slab is multi-block), so it is correct for the measurement's purpose — flagging only so the profile is read as "multi-block fold reduce", not "all reduce".
(2) Under overlap=True, _fold_block runs on the reducer thread concurrently with main-thread network reads, so spill_reduce_s + spill_read_s overlap the read phase in wall-clock and are not additive with total wall. The ratio reduce:read within the fold (what #280 step 0 needs) stays valid since the two are sequential inside _fold_block. This is pre-existing behavior for spill_read_s, not introduced here.

No blocking issues. The split is real and the profile prerequisite is sound.

---

Generated by Claude Code

[espg]

🤖 from Claude

Status note — this PR carries waiting but has never had a comment saying why. Recording it now so the state is legible from the thread rather than only from the body.

What it is waiting for: a measurement, not a review decision and not more code. Step 0 is complete and landed — spill_reduce_s is instrumented and split from spill_read_s. What gates #280 is running the 158-granule CONUS slab with this instrumentation and reading the reduce/read split:

• spill_reduce_s ≪ spill_read_s → compute parallelism is pointless; the lever is output-preserving prefetch pipelining.
• spill_reduce_s significant → coupled parallel reduce (reduce_workers=P ⇄ block_bytes/P) earns its keep.

That run needs a live fleet dispatch, which a routine cannot do (CLAUDE.md §1). So this sits until someone runs it — no routine run can clear it.

Two things worth knowing before that happens:

1. The branch is conflicted against main. GitHub reports mergeable_state: dirty, and the head is 148 commits behind. It has not been touched since 2026-07-21. I have not resolved the conflict — the PR is waiting on you, and the measurement may well change what the follow-on work looks like, so rebasing now could be wasted effort. Say the word and a run will merge main in.
2. CI has no meaningful signal on this head. aba9fa7 carries only two check runs — the docs check (success) and a skipped run. There is no test (3.12) / test (3.13) / ruff / build result on it at all; those runs predate the current workflow set or have aged out. It is not red, but it is not green either — a push (a main merge would do it) is what would produce a real result.

No code changed this run. Label stays implement + waiting.

---

Generated by Claude Code