Skip to content

cluster / persist: Shared batches in the sink - #35411

Closed
bkirwi wants to merge 2 commits into
MaterializeInc:mainfrom
bkirwi:persist-share
Closed

bkirwi wants to merge 2 commits into
MaterializeInc:mainfrom
bkirwi:persist-share

Conversation

@bkirwi

@bkirwi bkirwi commented Mar 11, 2026 •

Copy link
Copy Markdown
Contributor

A shared-state approach to writing batches in the MV sink, inspired by some of the concerns in #35263.

Seems worthwhile - CI doesn't cover the high-worker-count / medium-sized dataset that well, but I'll pull some numbers from staging. If y'all like it, we can do the same for the storage persist sink... the way that code is structured means it involves slightly more plumbing, but I don't see any real obstacles.

@github-actions

Copy link
Copy Markdown
Contributor

Thanks for opening this PR! Here are a few tips to help make the review process smooth for everyone.

PR title guidelines

  • Use imperative mood: "Fix X" not "Fixed X" or "Fixes X"
  • Be specific: "Fix panic in catalog sync when controller restarts" not "Fix bug" or "Update catalog code"
  • Prefix with area if helpful: compute: , storage: , adapter: , sql:

Pre-merge checklist

  • The PR title is descriptive and will make sense in the git log.
  • This PR has adequate test coverage / QA involvement has been duly considered. (trigger-ci for additional test/nightly runs)
  • If this PR includes major user-facing behavior changes, I have pinged the relevant PM to schedule a changelog post.
  • 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).

@bkirwi
bkirwi force-pushed the persist-share branch 13 times, most recently from 630c9a0 to bcb8d38 Compare March 16, 2026 21:30
@bkirwi bkirwi changed the title [wip] persist: Shared batches in the sink cluster / persist: Shared batches in the sink Mar 16, 2026
@bkirwi
bkirwi marked this pull request as ready for review March 16, 2026 21:45
@bkirwi
bkirwi requested review from a team as code owners March 16, 2026 21:45
@bkirwi
bkirwi requested a review from antiguru March 25, 2026 16:53
bkirwi added 2 commits March 25, 2026 15:29
Should come to the same thing, avoids registering a writer, and sets us
up for shared batch writing in the (near) future.
@antiguru

Copy link
Copy Markdown
Member

Superseded by #37591.

@antiguru antiguru closed this Oct 13, 2026
antiguru added a commit that referenced this pull request Oct 14, 2026
Continues #35411 (draft by @bkirwi), rebased onto `main`.

With many workers each MV sink worker writes its own small batch per
interval, so the sink emits many small parts and a large consensus state
diff. This coalesces the parts the workers in one process write for a
batch interval into a single, larger shared batch.

`SharedBatches` hands out a `SharedBatchBuilder` per batch id. Handles
for the same id feed one process-global builder task, and the last
handle to finish receives the batch. Both MV sinks use it, best-effort.
On an 8-worker replica this cut persist blob PUTs ~67% and consensus
state-diff bytes ~54%, at unchanged consensus command count.

The single builder task serializes encoding and upload, which slowed
hydration of a 9.5 GB MV from 26 s to 33 s. Each worker therefore stages
its updates for an interval and only contributes them to the shared
batch while they stay below `compute_mv_sink_shared_batch_max_bytes` (1
MiB in these measurements). Past that, the worker writes its own batch,
and the append operator appends both kinds together. With the threshold,
hydration and 5M-row inserts match the per-worker path, while 1000-row
writes drop from ~7.8 to ~1.05 data parts per append. Writes small
enough for persist's inline path now cost one blob write per append
instead of none, at about +10 ms p50 latency and 2.5x fewer compaction
parts.

`compute_mv_sink_shared_batch_max_bytes` is the only knob. It defaults
to 0, which disables sharing, and production enables it through a staged
parameter rollout. CI runs it at several values, including 0. Both sinks
read it per batch interval, so a change applies without re-rendering the
sink.

Best-effort means a worker only merges into the shared batch when its
write overlaps the others still building in the same interval, so parts
per append land around 1.4, where the ideal is 1.0. The barrier that
closes that gap is split into the follow-up #37617.

The async (v1) and sync (v2) sinks share the batch-writing code in
`sink::shared_batch`. Unlike @bkirwi's draft, the v1 path is behind the
same flag, since v1 is the default sink.

Original authorship by @bkirwi is preserved in the first commit.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Ben Kirwin <[email protected]>
Co-authored-by: Claude Opus 4.8 (1M context) <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants