Repository navigation
Conversation
QA LLM Review1. MEDIUM -- Taking a typed body races eager backing and aborts the replica
With DetailsBefore Normal merges and batch sealing reach this through Exclude chunks without a resident body from eager backing, or complete the free transition under the original state lock. |
Drives AccountedChunkBatcher the way an arrange site does and compares where committed chunks live: no pool, a pool with no budget, and a pool with a quarter-size budget evicting inline or on spill threads. Every arm's sealed output is checksummed against the resident arm, so the pressure arms double as an end-to-end eviction check.
Spilling a chunk serialized its body into a pool slot at commit and every read copied it back out, whether or not the pool ever came under budget pressure. A batcher whose state fits in memory paid for both copies on every merge. The pool now accepts a TypedBody: the owner's in-memory form, charged against the budget at its serialized length and serialized only when it has to leave that form (eviction, eager backing, a copy-out read). Reads borrow it under the chunk's state lock, which eviction only ever try_locks, and a sole owner takes it back with no copy. ColumnChunk spills through it. A merge no longer keeps an extra reference to typed-resident inputs, since that reference forced a duplicate on every into_body, and time bounds are computed on first use and filled whenever the pool serializes the body. Prototype: PooledBody asserts Send + Sync unsafely, with a TODO for real bounds on the containers.
Eviction serializes a typed body under the chunk's state lock, and serialization fills the shared time-bounds cell. chunk_time_bounds took the state lock from inside the cell's initializer, the opposite order, so a read racing a spill thread on the same chunk deadlocked. On staging this hung one worker of every index replica at the 0.05 budget during SF30 hydration. Fill the cell under the state lock instead, and add a test that races bounds reads against spill-thread eviction; it hangs on the previous order.
A pooled body moves to and is borrowed from the pool's spill threads, so it must be Send and Sync. Replace the unsafe impls with a PoolColumnar bound, a Columnar type whose containers and values cross threads, on the chunk paths that spill typed bodies and on their callers in compute and the upsert stash.
take_typed emptied the chunk and released the state lock before the handle's drop set freed. In that window eager backing could select the UnbackedResident chunk with no body, move it to WriteInFlight, and the spill thread hit the unreachable arm finding nothing to serialize. Set freed under the lock that takes the body, so queue visitors skip it.
cb297da to
aa6a12a
Compare
Motivation
A chunk batcher that spills to the buffer pool pays for the pool even when nothing is ever evicted. On trunk, spilling serializes each chunk body into a pool slot at commit, and every read copies it back out.
What this changes
The pool accepts a
TypedBody: the owner's in-memory form, charged against the budget at its serialized length. The pool serializes it only when it has to leave that form: eviction, eager backing, or a copy-out read. Reads borrow it under the chunk's state lock. Eviction only evertry_locks a chunk, so a borrowed body is skipped rather than waited on. A sole owner takes the body back with no copy.ColumnChunkspills through it, with two follow-on fixes the profiles called for:mergekept an extraRcto each spilled input so an untouched survivor could return without a rewrite. For a typed-resident body that extra reference forcedinto_bodyto duplicate it. The reference is now kept only for bodies that are not typed-resident, and a typed survivor is taken and re-inserted.extractwithout a decode. Filling them takes the chunk's state lock before the bounds cell, the same order eviction uses.time_bounds_race_spill_thread_evictioncovers the deadlock the opposite order caused on staging.Spill threads take and borrow typed bodies, so the chunk paths that spill require
PoolColumnar: aColumnartype whose values and containers areSend + Sync.MzDataimplies it, and the upsert stash's time and order types carry it.The first commit adds
src/timely-util/benches/chunk_pool.rs, which drivesAccountedChunkBatcherwith no pool, a pool with no budget, and a quarter-size budget evicting inline or on two spill threads. Each arm's sealed output is checksummed against the resident arm.Local results
Single-threaded, macOS, same bench on trunk and on this branch:
The fits-in-memory arm now matches resident, and the pressure arms are 6% to 13% faster than trunk. A rerun at the final commit gave resident 151 / 749 ms and pool 147 / 750 ms.
Staging results
TPC-H SF30 on M.1-3xlarge with the
misc/experiments/ooc-hydrationharness (#40515), at budget 0.5 and RSS target 1.0, where nothing is evicted. Four trials per arm, one phase at a time, run on a trunk image (527f9b78) and on this branch (71dc6b48). Each cell is the median over trials of the slowest object's hydration time.columnationis the production configuration, andchunkedis the chunk batcher with no pool.Pool over
columnation:The branch cuts the pool's fits-in-memory overhead by 4 to 7 points in every phase. Against
chunkedit leaves about 2% in idx and mv_second, inside the spread between trials. The larger mv_first number is mostly placement: some replicas hydrate every object about 65 s late, which looks like a delayed start rather than slower work, and the pool arm's second trial drew such a slot on both images. Compared slot class to slot class, mv_first's pool overhead is 2% to 7%, in line with the other phases.An earlier SF30 run measured 0.77x to 0.83x of production at budget 0.5. That idx figure came from two trials 30% apart, and its MV figures shared the environment with SF150 runs, so the trunk overhead here is the better baseline.
Follow-ups
chunk_pool. Every insert takes a shared depth-band queue lock and callsenforce_budget, which the single-threaded bench does not contend.try_spill_ref) still serializes at spill.Tests
mz-orepool tests cover typed chunks through borrow and take, ranged reads, inline eviction, re-admission, eager backing, take after backing, spill-thread eviction, a free while the write is queued, budget enforcement on insert, and the over-size fallback.spill_align_round_tripnow evicts through a zero-budget pool to keep checking that a serialized body re-serializes byte-identically.time_bounds_race_spill_thread_evictionreads time bounds while spill threads evict, and hung on the previous lock order.Generated with Claude Code