timely-util: whole-chunk extract pass-through via resident time bounds by DAlperin · Pull Request #38254 · MaterializeInc/materialize

DAlperin · 2026-08-17T18:16:58Z

Motivation

A seal walks every stashed chunk to partition updates against the frontier, and for a spilled chunk that walk previously loaded and decoded the whole body even when no update in it could possibly be extracted (or when all of them must be). During sustained ingest the stash backlog is mostly frontier-disjoint chunks, so per seal this decoded the entire backlog to move one boundary chunk. The August benchmark campaign measured the two stash patches together as a 35% mean and ~2x p90 reduction in co-tenant probe latency at 50cc.

Spilled bodies now carry their time bounds as resident metadata (an antichain lower bound and the maximal time elements). extract_into uses them for two whole-chunk fast paths: ship the chunk untouched when the frontier is past all of its times, keep it untouched (folding its lower bound into the residual frontier) when none of its times are ready. Only chunks the frontier genuinely straddles are loaded, decoded, and split.

Tips for reviewer

The residual-frontier fold is the subtle part for partial orders: a kept chunk contributes its lower bound, not its individual times, which is sound because extraction only needs an upper bound on what remains.
Both fast paths have dedicated tests, including a frontier-disjoint pass-through case.

Checklist

This PR has adequate test coverage / QA involvement has been duly considered. (trigger-ci for additional test/nightly runs)
This PR has an associated up-to-date design doc, is a design doc (template), or is sufficiently small to not require a design.
If this PR evolves an existing $T ⇔ Proto$T mapping (possibly in a backwards-incompatible way), then it is tagged with a T-proto label.
If this PR will require changes to cloud orchestration or tests, there is a companion cloud PR to account for those changes that is tagged with the release-blocker label (example).
If this PR includes major user-facing behavior changes, I have pinged the relevant PM to schedule a changelog post.

antiguru

Seems fine, left some comments inline, please address before merging. I tried something like this a few years ago before we had columns and the likes, and for the limited set of cases I considered it wasn't magically better, but with the I/O cost, the cost model changes, and this seems to make a lot more sense.

antiguru · 2026-08-19T06:40:35Z

+            // The residual must lower-bound every kept time, which is the
+            // chunk's lower bound antichain by construction.
+            for m in time_lower.elements() {
+                residual.insert(m.clone());


Suggested change

residual.insert(m.clone());

residual.insert_ref(m);

antiguru · 2026-08-19T06:41:43Z

    /// Extracting a large chunk at an intermediate frontier cuts both sides
    /// into several chunks and partitions exactly by time.
    #[mz_ore::test]
-    #[cfg_attr(miri, ignore)]


Stray edit.

antiguru · 2026-08-19T06:45:50Z

+    /// The maximal times in the body. Some contained time is
+    /// greater-or-equal to a frontier exactly when some maximal time is,
+    /// which is `extract`'s ship-whole test.
+    time_upper: Vec<T>,


This could be SmallVec because it's at least one element, and mostly one element for non-empty chunks.

antiguru · 2026-08-19T06:47:58Z

+            ColumnChunk::Resident(col, _) => Self::time_bounds(col),
+            ColumnChunk::Spilled(body) => (body.time_lower.clone(), body.time_upper.clone()),


It's a bit unfortunate that we need to clone here. We could avoid it by returning Cow I think? It seems the callers of chunk_time_bounds don't need owned data.

antiguru · 2026-08-19T06:49:27Z

+        let view = column.borrow();
+        let times = view.1;


Suggested change

let view = column.borrow();

let times = view.1;

let (_, times, _) = column.borrow();

Maybe?

antiguru · 2026-08-19T06:53:34Z

+        let mut lower = Antichain::new();
+        let mut upper: Vec<T> = Vec::new();
+        for i in 0..times.len() {
+            let t = T::into_owned(rr::<T>(times.get(i)));


This can be expensive for times with owned allocations. Instead, do what Column::extract does: keep a mutable owned_t: T = T::default(), and use T::copy_from(&mut owned_t, times.get(i)) to get an owned variant, re-using the allocation.

antiguru · 2026-08-19T06:54:46Z

+            for m in time_lower.elements() {
+                residual.insert(m.clone());
+            }


This could be residual.extend(&time_lower) or so.

antiguru · 2026-08-19T07:03:32Z

+        // past the frontier keeps unchanged. Spilled bodies pass through
+        // without a load, a re-commit, or any codec work; only chunks the
+        // frontier actually splits are loaded below.
+        let (time_lower, time_upper) = chunk.chunk_time_bounds();


Note that this possibly duplicates some work that col.extract does subsequently. I don't see a way around this, tho.

### Motivation The upsert v2 (continual feedback) stash moves from its previous representation onto `ChunkBatcher`/`ChunkSpine`: stashed updates live in columnar chunks that participate in the buffer pool, and the feedback arrangement drains through `UnloadChunk` one chunk at a time instead of materializing whole batches. With `enable_upsert_paged_spill` on, stash and arrangement state past the residency budget spills as compressed, budget-accounted extents rather than raw kernel-swapped heap. Benchmarked extensively on AWS (self-managed EKS, MSK source, swap-enabled nodes), August 2026 campaign: * Hydration under pressure, 25cc replica: at 4.5x state-to-memory, v2-spill hydrates in 707 s vs 1187 s for v1 (RocksDB mem-env). At 9x: 1607 s vs 4607 s (v1 goes superlinear, v2 stays near-linear). At 13.5x, v1 and v2-resident crash-loop at the pod swap ceiling and never complete; v2-spill finishes in 2386 s with matching counts. * Swap traffic: v2-spill moves roughly half the swap bytes of v1 at every depth measured. * Steady-state ingest freshness under the same pressure: statistically identical across v1, v2-resident, and v2-spill. * Unpressured regimes: v2-resident tracks v1 within ~10% everywhere; spill costs appear only at a 50cc saturation edge and vanish one size up (or under the two chunk patches below this PR in the stack). The new representation is flagged: `enable_upsert_chunked_stash` (off in production, on and randomized in CI) selects between the `ChunkBatcher` representation above and the previous paged-columnar-merge-batcher + `ValRowSpine` representation, which stays the production default while the chunked flavor earns trust. The operator loop is shared; a small `UpsertStashArm` trait carries the flavor-specific pieces (stash batcher, feedback spine, flush, drain), and the arms are resolved from the config set once at operator construction, so a dataflow keeps its flavor for life. Spilling in either flavor remains gated by `enable_upsert_paged_spill`. ### Tips for reviewer * Stacked on #38253 and #38254 (the two chunk patches the campaign produced); the interesting upsert logic is `upsert_continual_feedback_v2.rs`. * `enable_upsert_paged_spill` is replica-scoped and composes with compute's spill gate as an OR on the process-wide pool. * The unit-test harness runs every scenario under both stash flavors and asserts they produce identical output, so the paged arm is exercised by the same tests as the chunked one. ### Checklist - [ ] This PR has adequate test coverage / QA involvement has been duly considered. ([trigger-ci for additional test/nightly runs](https://trigger-ci.dev.materialize.com/)) - [ ] This PR has an associated up-to-date [design doc](https://github.com/MaterializeInc/materialize/blob/main/doc/developer/design/README.md), is a design doc ([template](https://github.com/MaterializeInc/materialize/blob/main/doc/developer/design/00000000_template.md)), or is sufficiently small to not require a design. - [ ] If this PR evolves [an existing `$T ⇔ Proto$T` mapping](https://github.com/MaterializeInc/materialize/blob/main/doc/developer/command-and-response-binary-encoding.md) (possibly in a backwards-incompatible way), then it is tagged with a `T-proto` label. - [ ] If this PR will require changes to cloud orchestration or tests, there is a companion cloud PR to account for those changes that is tagged with the release-blocker label ([example](MaterializeInc/cloud#5021)). - [x] If this PR includes major [user-facing behavior changes](https://github.com/MaterializeInc/materialize/blob/main/doc/developer/guide-changes.md#what-changes-require-a-release-note), I have pinged the relevant PM to schedule a changelog post.

DAlperin mentioned this pull request Aug 17, 2026

storage/upsert: v2 stash on ChunkBatcher with pool-backed spill #38255

Merged

5 tasks

DAlperin force-pushed the dov/chunk-extract-passthrough branch from 5ae338b to 93f419d Compare August 17, 2026 19:31

DAlperin marked this pull request as ready for review August 18, 2026 21:44

DAlperin requested a review from a team as a code owner August 18, 2026 21:44

antiguru approved these changes Aug 19, 2026

View reviewed changes

DAlperin force-pushed the dov/chunk-extract-passthrough branch 3 times, most recently from fc348d4 to 4b2d65b Compare August 24, 2026 14:31

DAlperin force-pushed the dov/chunk-extract-passthrough branch from 4b2d65b to d087cb9 Compare August 24, 2026 14:47

timely-util: address extract pass-through review comments

15c9ed3

DAlperin force-pushed the dov/chunk-extract-passthrough branch from d087cb9 to 15c9ed3 Compare August 24, 2026 15:00

DAlperin merged commit bd97ff3 into main Aug 24, 2026
83 checks passed

DAlperin deleted the dov/chunk-extract-passthrough branch August 24, 2026 15:25

Navigation Menu

Uh oh!

timely-util: whole-chunk extract pass-through via resident time bounds - #38254

timely-util: whole-chunk extract pass-through via resident time bounds#38254
DAlperin merged 1 commit into
dov/chunk-compress-floorfrom
dov/chunk-extract-passthrough

DAlperin commented Aug 17, 2026 •

edited

Loading

Uh oh!

antiguru left a comment

Uh oh!

antiguru Aug 19, 2026

Uh oh!

antiguru Aug 19, 2026

Uh oh!

antiguru Aug 19, 2026

Uh oh!

antiguru Aug 19, 2026

Uh oh!

antiguru Aug 19, 2026

Uh oh!

antiguru Aug 19, 2026

Uh oh!

antiguru Aug 19, 2026

Uh oh!

antiguru Aug 19, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		ColumnChunk::Resident(col, _) => Self::time_bounds(col),
		ColumnChunk::Spilled(body) => (body.time_lower.clone(), body.time_upper.clone()),

	let view = column.borrow();
	let times = view.1;
	let (_, times, _) = column.borrow();

Uh oh!

Conversation

DAlperin commented Aug 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Motivation

Tips for reviewer

Checklist

Uh oh!

antiguru 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!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

DAlperin commented Aug 17, 2026 •

edited

Loading