Skip to content

Add opt-in join-the-flight ByteStream write dedup - #2591

Closed
erneestoc wants to merge 2 commits into
TraceMachina:mainfrom
erneestoc:ec/bytestream-write-dedup
Closed

Add opt-in join-the-flight ByteStream write dedup#2591
erneestoc wants to merge 2 commits into
TraceMachina:mainfrom
erneestoc:ec/bytestream-write-dedup

Conversation

@erneestoc

@erneestoc erneestoc commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

Problem

N concurrent ByteStream.Write streams for the same digest are N full
uploads over the wire and N full drains through the store stack. Any build
whose actions fan out identical artifacts (e.g. hundreds of actions each
producing the same runtime library copies or plists) pays this as an upload
storm: the clients race FindMissingBlobs before any upload commits, all
see "missing", and all upload.

A community profile of a ~9k-action iOS build measured client→CAS upload as
the single largest overhead bucket (7,468s wall across 7,245 actions), with
~228 actions producing identical outputs.

Mechanism

New opt-in ByteStreamConfig.experimental_write_dedup. For each new upload
(resumed streams bypass dedup to preserve resume semantics):

  • Leading upload (first writer of a digest): claims a per-instance
    single-flight slot keyed by DigestInfo, then runs one existence check.
    If the digest is already durable it responds immediately with the
    spec-mandated early WriteResponse (committed_size = full size, or -1
    for compressed-blobs uploads). Otherwise it uploads exactly as before.
  • Waiters (concurrent same-digest uploads): never touch the store. They
    drain-and-discard their request stream while awaiting the leading
    upload's durable commit, then respond with the same early WriteResponse.
    Draining is load-bearing: unread request bytes would pin the HTTP/2
    connection flow-control window and deadlock a leading upload sharing
    the connection (found by benchmark; the naive don't-read design hangs).
  • Durability is never faked: waiters are acked only after the leader's
    Store::update() returns. If the leading upload fails or is cancelled at
    any await point, a send-on-drop guard broadcasts a retryable ABORTED to
    all waiters (retryable for both Bazel's and NativeLink's retriers) and one
    retry becomes the new leader. The flight is removed from the map before
    the broadcast, so late arrivals always start a fresh flight.

The REAPI spec explicitly defines this behavior ("if another client has
already completed the upload — which may occur in the middle of a single
upload if another client uploads the same blob concurrently — the request
will terminate immediately … the client should not attempt to retry").

This is the wire-dedup half of the upload-path work; #2592 (worker output
upload batching) is the companion that batches small worker outputs into
BatchUpdateBlobs — the two partition upload traffic and compose additively.

Measured (M-series, real gRPC stack, in-process transport, byte-verified)

N concurrent same-digest uploads, dedup off → on:

shape wall server-received bytes store updates
256KiB ×228 simultaneous 23ms → 9ms 59.9MB → 17.1MB (3.5×) 228 → 1
2MiB ×64 simultaneous 42ms → 37ms 134MB → 122MB 64 → 1
256KiB ×228 staggered burst 21ms → 9ms 60.1MB → 16.9MB (3.6×) 229 → 1
2MiB ×64 staggered burst 42ms → 3ms (14×) 136.5MB → 8.8MB (15.5×) 65 → 1
  • Store work collapses to exactly one update() per digest in every shape —
    on a real worker fast tier that is one fsync instead of hundreds.
  • Wire savings scale with how staggered the storm is: stragglers cancel
    after their first chunks. Single-chunk blobs (≤64KiB) see storage-side
    dedup only (their payload rides in the first message).
  • Non-duplicate uploads: one extra has() per new upload, no other change.

Safety analysis

  • Flag default-off; disabled path byte-identical (all 29 pre-existing
    bytestream tests unchanged and green).
  • Resume (persist_stream_on_disconnect_timeout_s) untouched: uploads whose
    UUID is already tracked bypass dedup entirely. A failed leader still parks
    for resume; if it resumes while a retry-elected leader is mid-flight the
    store-level dedup (Flush in detect_duplicate_upload #2528 et al.) covers the double write.
  • GrpcStore proxy shortcut is upstream of the dedup hook, so pure-proxy
    instances defer dedup to the terminal CAS server; a GrpcStore client
    uploading against a dedup server tolerates the early response (covered by
    test).
  • Cancellation at any leader await point releases waiters via the guard's
    Drop (same pattern as FlushCoalescer's send-on-drop).

Tests

  • write_dedup_completes_early_for_existing_blob
  • write_dedup_fresh_upload_unchanged
  • write_dedup_joins_inflight_upload (waiter blocks until leader commits)
  • write_dedup_waiter_retries_after_leader_failure (ABORTED + retry wins)
  • write_dedup_compressed_early_complete_returns_negative_one
  • grpc_store_client_tolerates_early_complete (bench file)
  • Full suites: cargo 34/34; bazel service+config 19/19 incl. clippy
    pedantic + nightly rustfmt aspects; cargo check --tests --workspace.

Metrics: write_dedup group — flights_joined, early_completes,
leader_failures, bytes_saved.

Review-round additions (8-angle adversarial review, pre-PR)

Fixed:

  • GrpcStore::update now drains its unconsumed reader after an
    early-completed write RPC. Empirically verified (64MiB mid-stream test):
    without this, the upload RPC succeeds but the upstream sender coupled to
    the reader fails with "receiver disconnected", so proxy chains
    (fast_slow{slow: grpc}) failed deterministically for blobs that already
    exist upstream. This is a latent bug on main against ANY early-completing
    CAS (buildbarn, bazel-remote); this PR both triggers and fixes it.
  • write_dedup.leader_failures only counts failures with waiters actually
    joined (previously every routine failed upload with the flag on counted).
  • Metric epilogue deduplicated into record_write_result; single flight
    settlement site in the waiter drain loop.
  • bytes_saved documented as an approximate upper bound (first-message
    payload can't be subtracted; compressed waiters counted at uncompressed
    size).

Known limitations (documented, follow-ups):

  • Waiter UUIDs are not registered in active_uploads, so
    QueryWriteStatus for a disconnected waiter reports no progress and the
    client restarts from offset 0 (bytes are drained, not stored, so resume
    has nothing to resume into). Follow-up: uuid→flight side map.
  • Waiters fate-share the leading upload's pace: many fast clients behind
    one slow leader can hit their own RPC deadlines and retry-loop until the
    leader settles. Opt-in flag; workloads with highly asymmetric client
    bandwidth on identical large blobs should keep it off or we add a size
    cap knob as a follow-up.
  • A leader that disconnects but resumes via
    persist_stream_on_disconnect_timeout_s can overlap with a retry-elected
    new leader (two full uploads; storage dedup reconciles — correctness
    holds, bandwidth win temporarily inverts under flaky leaders).
  • The leader pays one has() per new upload. For FindMissingBlobs-guarded
    clients this is usually a miss; it is also exactly the path that produced
    the 15.5× staggered-storm win. Compose with existence_cache to make the
    check cheap.
  • A waiter whose own stream errors mid-drain still gets the flight outcome
    (success if the leader committed) — permitted by the REAPI early-
    termination contract; noted because the same malformed stream without a
    concurrent flight would error.

🤖 Generated with Claude Code

https://claude.ai/code/session_01UXVtatcR9YMecBiu9RjwpC


This change is Reviewable

Review update (2026-07-22)

The implementation was hardened after adversarial review:

  • Deduplication now runs before the GrpcStore proxy shortcut, so proxy-backed CAS paths participate correctly.
  • Flight keys include the requested digest function, preventing SHA-256/BLAKE3 cross-function collisions.
  • GrpcStore early-completion paths detach a cancellation-safe drain of the caller's remaining stream, preserving proxy-chain behavior without waiting indefinitely.
  • Added regressions for proxy deduplication, digest-function collisions, stalled producers, and cancellation.

The flag remains opt-in. Waiters still share the leader's outcome and do not provide resumable progress of their discarded upload stream.

Focused Cargo and Bazel ByteStream/GrpcStore suites, formatting checks, and git diff --check pass.

@vercel

vercel Bot commented Jul 22, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
nativelink Ready Ready Preview, Comment Jul 22, 2026 10:57pm
nativelink-aidm Ready Ready Preview, Comment Jul 22, 2026 10:57pm

Request Review

N concurrent ByteStream Write streams for the same digest are N full
uploads over the wire and N full drains through the store stack. With the
new opt-in ByteStreamConfig.experimental_write_dedup, exactly one
concurrent upload per digest is elected the leading upload; concurrent
duplicates drain-and-discard their stream while waiting and are acked
with the REAPI early WriteResponse only once the leading upload durably
commits. Uploads of already-durable digests complete early after a single
existence check. If the leading upload fails or is cancelled, waiters
receive a retryable ABORTED via a send-on-drop guard and one retry
becomes the new leader. Resumed streams bypass dedup to preserve resume
semantics.

Waiters must drain their request stream: unread bytes would pin the
HTTP/2 connection flow-control window and deadlock a leading upload
sharing the connection (found by benchmark).

Measured (real gRPC stack, byte-verified, dedup off -> on): 228x256KiB
simultaneous same-digest uploads: server-received bytes 59.9MB -> 17.1MB
(3.5x), store updates 228 -> 1; 64x2MiB staggered burst: 136.5MB ->
8.8MB (15.5x), wall 42ms -> 3ms. Metrics group write_dedup:
flights_joined, early_completes, leader_failures, bytes_saved.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UXVtatcR9YMecBiu9RjwpC
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.

1 participant