timely-util: ColumnChunk, a pool-spillable Chunk implementation by DAlperin · Pull Request #37955 · MaterializeInc/materialize

DAlperin · 2026-07-29T18:47:03Z

Motivation

Part of the buffer-managed dataflow state design (doc/developer/design/20260610_buffer_managed_state.md). The pool merged in #37718 and its configuration in #37719. This PR adds the chunk type that spills through it. Stacked on #37954 (the UnloadChunk trait); follow-ups adopt it in storage's upsert stash and compute's arrange sites, staged with CI on #37805.

Part of CPU-184.

Description

Three commits, reviewable separately.

Monotone columnar ship threshold. The ship signal was a 10% window below each 2 MiB boundary. A single record wider than the window steps clear over it, the signal un-fires, and a chunk can grow past the pool's largest size class (8 MiB), where a spilled body degrades to permanently resident. The threshold is now monotone at 10% under 2 MiB. This touches live paths (ColumnBuilder's ship point, merge/extract cuts, Column::at_capacity): rows under ~200 KiB serialized behave identically, wider rows now ship at the first boundary instead of drifting.
ColumnChunk. Differential's Chunk over Column-shaped updates: sorted, consolidated (D, T, R) runs with the merge/extract/advance/settle transducers, plus the UnloadChunk implementation for (K, V) data (locate from key fences, gallop-based extraction). Grading is by serialized bytes rather than the record-count TARGET, since record count does not bound bytes for variable-width data.
Pool spilling and depth hints. A chunk is Resident (Rc-shared Column) or Spilled (serialized body in the process pool, with record count and first/last fences resident). settle is the commit point: bodies at or above 64 KiB spill when the compute or storage gate is set and a pool is configured. Reads are copy-out and call-scoped, which is what lets the pool evict with no reader accounting. Each chunk carries a generational depth (fresh 0, merge output one past its deepest input) that becomes the pool's eviction-band hint, so repeatedly merged, colder data evicts first. The UnloadChunk probe path reads spilled bodies for the scope of one call and deliberately does not re-admit them.

Nothing in production sets the spill gates yet. The storage and compute wiring comes in the follow-up PRs, so apart from commit 1 this is inert until then.

Verification

Property tests drive the trait methods the way the differential harness does and compare against brute-force references, in resident and force-spilled variants (batcher round trip, seal partitioning at intermediate frontiers, advance, extraction with straddled keys). Deterministic large-data tests cover the cut paths the proptests cannot reach: advance's multi-chunk cut, the giant-group carry, extract's mid-chunk cut on both sides. The threshold change carries a regression test that fails against the windowed predicate. The spill-gate matrix (compute/storage OR, no clobber) runs against a real installed pool, and the spill round trip is asserted byte-identical, including re-spilling an already-serialized body. The full stack runs CI on #37805.

linear-code · 2026-07-29T18:47:07Z

CPU-184

DAlperin · 2026-07-30T18:21:54Z

+        if let Some(pool) = spill_pool() {
+            let len_bytes = column.length_in_bytes();
+            if len_bytes >= SPILL_MIN_BYTES {
+                let view = column.borrow();
+                let records = view.len();
+                let mut first = D::Container::default();
+                let mut last = D::Container::default();
+                first.push(view.0.get(0));
+                last.push(view.0.get(records - 1));
+                let handle = spill_column(column, &pool, len_bytes, ChunkHints { depth });
+                return ColumnChunk::Spilled(Rc::new(SpilledBody {
+                    records,
+                    first,
+                    last,
+                    depth,
+                    handle,
+                }));
+            }
+        }
+        ColumnChunk::Resident(Rc::new(column), depth)


an adversarial caller could do a bad job merging and leave lots of < SPILL_MIN_BYTES chunks around, but probably fine to just... trust they won't do that

DAlperin · 2026-07-30T18:38:44Z

+        let [col_a, col_b] = &mut cols;
+        for (col, pos, depth, spilled, queue) in [
+            (col_a, positions[0], depths[0], &mut spill_a, in1),
+            (col_b, positions[1], depths[1], &mut spill_b, in2),
+        ] {


better names or a comment

DAlperin · 2026-07-30T18:45:08Z

+        let mut keep_col: Column<(D, T, R)> = Column::default();
+        let mut ship_col: Column<(D, T, R)> = Column::default();
+        while pos < len {
+            col.extract(&mut pos, frontier, residual, &mut keep_col, &mut ship_col);


follow up note (not for this PR) we should rewrite the underlying extract to be two pass, just the T column first then per range copies.

DAlperin · 2026-07-30T18:50:23Z

+    /// copied once on arrival, keeping the run linear. Advancing is
+    /// lattice-monotone but not order-monotone, so each group's advanced
+    /// times are re-sorted before adjacent equal times fold.
+    fn advance(


Possible follow-up: since the output leaves are addressed independently, a group that folds nothing (no time collisions, no zeroed diffs) could bulk extend_from_self the D leaf over the whole group range and push only the advanced times/diffs per record, instead of re-pushing group_d per survivor. Singleton groups (the common case for mostly-unique D) could skip the scratch/sort round-trip entirely.

The ship signal was a 10% window below each 2 MiB boundary, so a single record wider than the window stepped clear over it and the container kept growing: merge and extract cuts, the builder ship point, and Column::at_capacity could all let a chunk outgrow the buffer pool's largest size class, past which a spilled body degrades to permanently resident. The threshold is now monotone at 10% under 2 MiB, which behaves identically for record-at-a-time growth and cuts wide-record chunks at the first check. The builder's inline copy of the check now calls the shared predicate.

…ates

frankmcsherry

Seems generally sound. I did not read the merge/extract/advance/settle code for correctness. My comments are mostly small nits about possible improvements, none of which should block the work but which might improve some thinking about it.

Grading is by serialized bytes rather than the record-count TARGET, since record count does not bound bytes for variable-width data

Probably minor, but the overly paranoid could worry about whether this maintains the properties that the merge batcher wants, around balance in merge inputs (to get logarithmic behavior, rather than potentially quadratic). I .. don't actually know myself, and I think it's tied up in "non-unit-cost sorting/merging" and .. yeah. No idea. But wanted to flag that it's a thing to briefly ponder and the probably write off. :D

frankmcsherry · 2026-08-04T15:47:20Z

-    let words = indexed::length_in_words(borrow);
-    let round = (words + (SHIP_WORDS - 1)) & !(SHIP_WORDS - 1);
-    round - words < round / 10
+    indexed::length_in_words(borrow) >= SHIP_WORDS - SHIP_WORDS / 10


Nit, but .. @antiguru pointed out that the / 10 specifically could be stressful in that LLVM doesn't manage to remove the integer divide through magic, and perhaps / 16 could be better for a thing that gets called on each push. It's also totally fine to have a different take on capacity, but .. this existed because the conventional "did I hit a threshold" approach results in one full buffer and one mostly empty buffer. Another approach could be for columnar to grow a pop() method, which would be a bunch of typing but not too much thinking.

SHIP_WORDS is const, the compiler should unfold this into a simple comparison against a static value.

frankmcsherry · 2026-08-04T15:49:50Z

+/// Route this thread's chunk spills through `pool` (or back to the global
+/// resolution with `None`).


Nit, but the comments for the other methods start with Enable or disable which seems great, and these "claim" to do something concrete like route some data around. It seems like it may be more "Set or unset the pool through which chunk spills are routed" or similar.

frankmcsherry · 2026-08-04T15:51:27Z

+/// the largest body it ever carried (heap no pool gauge can see).
+const SCRATCH_RETAIN_WORDS: usize = 1 << 18;
+
+/// Run `f` over this thread's read scratch, cleared of any previous use.


Very minor nit, but it's not really "over" in that it is FnOnce and just gets the whole list, once.

frankmcsherry · 2026-08-04T15:54:28Z

+/// Compute's leg of the process spill gate. See [`set_compute_spill_enabled`].
+static COMPUTE_SPILL_ENABLED: AtomicBool = AtomicBool::new(false);
+
+/// Storage's leg of the process spill gate. See [`set_storage_spill_enabled`].
+static STORAGE_SPILL_ENABLED: AtomicBool = AtomicBool::new(false);


I'm a bit into reading this, and I think I would benefit from a description of the thread-locals that are maintained, and their goals. It's maybe clear enough, but I'm learning about the intent as I see methods defined, then scrolling up to make sure it tracks doccomments, imagining future call sites, etc.

frankmcsherry · 2026-08-04T17:11:27Z

+    /// The first data item, as a one-element container.
+    first: D::Container,
+    /// The last data item, as a one-element container.
+    last: D::Container,


Minor, but was curious why two single element containers, rather than one two element container. Is it important?

claude listened to me when I told it to do it that way :eek: will fix

frankmcsherry · 2026-08-04T17:13:39Z

+
+/// A sorted, consolidated run of `(D, T, R)` updates, resident or spilled.
+///
+/// Every chunk carries a generational depth, fixed at creation: fresh chunks


I'm not sure how the generation is used at this point, but wanted to flag that if it is incremented on merge, it could certainly overflow, even without 2^256 updates: "backstop" snapshot layers get remerged fairly regularly, because due to consolidation they do not necessarily grow in size, and will soon remerge with a similarly sized batch.

merge uses saturating_addm and I think saturation is the right behavior here

frankmcsherry · 2026-08-04T17:24:44Z

+    /// Merge the two fronts through their shared horizon with
+    /// [`Column::merge_from`]: gallop bulk-copies for disjoint runs, semigroup
+    /// consolidation on equal `(data, time)`, output cut at the ship
+    /// threshold. The exhausted front retires. A survivor consumed partway is
+    /// rewritten and pushed back. A survivor not consumed at all goes back as
+    /// it was, in particular a spilled body is neither rebuilt nor re-spilled.
+    ///
+    /// Fronts whose data ranges are disjoint never load at all: the resident
+    /// fence entries decide, and the lower front moves to the output verbatim.


Maybe a nit, but reads like a restatement of the trait method documentation, rather than an enrichment for the implementation here. Replaces the trait implementation, and perhaps this text is preferred, but wanted to flag that it doesn't seem to say much other than what is required of its implementation by the trait.

frankmcsherry

Seems generally sound. I did not read the merge/extract/advance/settle code for correctness. My comments are mostly small nits about possible improvements, none of which should block the work but which might improve some thinking about it.

Grading is by serialized bytes rather than the record-count TARGET, since record count does not bound bytes for variable-width data

Probably minor, but the overly paranoid could worry about whether this maintains the properties that the merge batcher wants, around balance in merge inputs (to get logarithmic behavior, rather than potentially quadratic). I .. don't actually know myself, and I think it's tied up in "non-unit-cost sorting/merging" and .. yeah. No idea. But wanted to flag that it's a thing to briefly ponder and the probably write off. :D

DAlperin · 2026-08-04T17:56:31Z

@frankmcsherry tftr! I think the merge batcher behavior should be fine. Chunk (physical) sizing doesn't play into the batchers merge decisions. The chains just look at the cumulative summed record counts, regardless of the internal chunk layout. So each record is still merged O(log n) times. Byte grading does disagree with the DD usage which is strictly based on record count but (a ~2MB chunk could have more than TARGET records)... I'm not too bothered by that if you aren't? That assumption seems to only be used by the is_graded and the pack helper which aren't used here.

Base automatically changed from dov/unload-chunk to main July 30, 2026 15:40

DAlperin force-pushed the dov/column-chunk branch from 957ea65 to 9736cb9 Compare July 30, 2026 15:40

DAlperin commented Jul 30, 2026

View reviewed changes

DAlperin force-pushed the dov/column-chunk branch from 9736cb9 to a2170a0 Compare July 30, 2026 19:00

timely-util: ColumnChunk, differential's Chunk over Column-shaped upd…

fc296e8

…ates

DAlperin force-pushed the dov/column-chunk branch from a2170a0 to c0774e1 Compare July 30, 2026 19:37

timely-util: ColumnChunk pool-backed spilling and depth hints

08396b3

DAlperin force-pushed the dov/column-chunk branch from c0774e1 to 08396b3 Compare July 30, 2026 19:52

DAlperin marked this pull request as ready for review August 3, 2026 14:02

DAlperin requested a review from a team as a code owner August 3, 2026 14:02

frankmcsherry reviewed Aug 4, 2026

View reviewed changes

frankmcsherry self-requested a review August 4, 2026 17:29

frankmcsherry approved these changes Aug 4, 2026

View reviewed changes

timely-util: address review comments on ColumnChunk

4ee45ea

DAlperin merged commit 4bd7623 into main Aug 4, 2026
123 checks passed

DAlperin deleted the dov/column-chunk branch August 4, 2026 18:53

		/// Route this thread's chunk spills through `pool` (or back to the global
		/// resolution with `None`).

Uh oh!

Conversation

DAlperin commented Jul 29, 2026 • edited by github-actions Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Motivation

Description

Verification

Uh oh!

linear-code Bot commented Jul 29, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

frankmcsherry left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

frankmcsherry left a comment

Choose a reason for hiding this comment

Uh oh!

DAlperin commented Aug 4, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

DAlperin commented Jul 29, 2026 •

edited by github-actions Bot

Loading