Repository navigation
Conversation
Contributor
|
Thanks for opening this PR! Here are a few tips to help make the review process smooth for everyone. PR title guidelines
Pre-merge checklist
|
bkirwi
force-pushed
the
persist-share
branch
13 times, most recently
from
March 16, 2026 21:30
630c9a0 to
bcb8d38
Compare
bkirwi
marked this pull request as ready for review
March 16, 2026 21:45
Should come to the same thing, avoids registering a writer, and sets us up for shared batch writing in the (near) future.
Member
|
Superseded by #37591. |
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]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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.