Add opt-in zstd wire compression to GrpcStore transfers - #2596
Merged
MarcusSorealheis merged 9 commits intoJul 30, 2026
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
NativeLink's internal hops never used REAPI compressed-blobs: GrpcStore built plain blobs/ resource names for every upload and read, so worker output uploads and input fetches shipped raw bytes in both directions regardless of any compression setting (the zstd support from the server work is accept/advertise only). New opt-in GrpcSpec.experimental_remote_cache_compression: uploads and full-blob downloads of blobs at or above 64KiB stream through zstd (level 1; higher levels measured slower with no meaningful wire reduction) using compressed-blobs/zstd resource names. Ranged reads and small blobs keep the identity path. Downloads decode with size and digest verification; a retryable mid-stream failure resumes via the identity path at the already-forwarded uncompressed offset. The shared streaming codecs move from nativelink-service to nativelink-util (the service re-exports them), since nativelink-store cannot depend on the service crate. Also fixes a latent decoder bug found by the new tests: a blob whose decompressed size is an exact multiple of the 128KiB decoder output buffer made the loop poll the finished decoder once more, and the new-frame input hint made the EOF check misreport a fully decoded stream as "ended in the middle of a zstd frame" — spuriously failing compressed uploads of exact-multiple-size blobs from any client. Measured (token-bucket throttled real TCP, byte-verified, off -> on): 96MiB 3.7:1-compressible blob @1gbps: upload 712ms -> 203ms and download 706ms -> 204ms (3.5x), wire 101MB -> 27MB; @100Mbps 24MiB: 1975ms -> 494ms (4.0x). Incompressible payloads: identical wall time, no size penalty. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UXVtatcR9YMecBiu9RjwpC
erneestoc
force-pushed
the
ec/grpc-wire-compression
branch
from
July 22, 2026 02:57
ca07947 to
77690da
Compare
erneestoc
marked this pull request as draft
July 22, 2026 19:19
Hoist the two function-local enums above the first statements of their functions and restore alphabetical order in the service test suite list. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UXVtatcR9YMecBiu9RjwpC
Member
|
This has been tested and is looking good for our customers. |
Member
|
@amankrx I think we want to try and get it in a |
Member
MarcusSorealheis
marked this pull request as ready for review
July 26, 2026 16:32
Match the identity path's idiom from the cast_possible_wrap cleanup on main: saturating try_into instead of a bare usize-to-i64 cast. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UXVtatcR9YMecBiu9RjwpC
MarcusSorealheis
approved these changes
Jul 27, 2026
walter-zeromatter
added a commit
to Reactor-Inc/nativelink
that referenced
this pull request
Jul 30, 2026
Follow-ups to the wire-compression work added in TraceMachina#2596. `get_part_compressed` classified pump-stage failures with `is_retryable_code`, but pump errors come from our own writer and `buf_channel` reports a dropped receiver as `Internal`, which counts as retryable. A consumer that hung up mid-download therefore returned `Ok(Some(forwarded))` and made `get_part` issue a second, full-size identity Read streaming the remainder into a channel nobody was reading. Pump failures are now always terminal: they mean either the consumer went away or the decoder aborted, and the decoder's verdict is already reported by the arm above it since `try_join!` polls decode first. The fallback also treated a late failure from another stage as a reason to resume even when the download had already completed and been verified. The pump now records that it forwarded the decoder's EOF -- which the decoder only sends after the blob passes its size and digest checks -- and a late error after that point resolves as success instead of resuming into an already-closed writer. On a genuine resume, the identity request now asks for exactly the undelivered tail. It previously kept the caller's original `length`, which the entry guard only permits when it covers the whole blob, so the resumed request ran past the end of the blob and relied on the server clamping it. The compressed upload path is now taken only for real digest keys: a string key's `into_digest()` hashes the key itself, which the remote's mandatory digest verification would reject. The background drain spawn is also skipped when the encoder finished cleanly and the reader is already at EOF. Tests: new cases in the service suite for the aborted-consumer and resume paths. The resume path had no coverage at all -- the fake ByteStream now honors `read_offset`/`read_limit` so the resumed range can be asserted byte-exactly, and grew a mid-frame abort mode plus a mixed-entropy payload helper, since `make_content` compresses far enough that multi-megabyte blobs fit in one wire chunk. Resume is asserted both with `length: None` and with an explicit `length` covering the blob; only the latter over-requested before this change, since `read_limit: 0` already means "to the end" on the wire. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This was referenced Jul 30, 2026
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
2 participants
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.
Problem
NativeLink accepts and advertises REAPI
compressed-blobs/zstd(#2527),but never sends it: every
GrpcStoreresource name is plainblobs/,so worker→CAS output uploads and worker←CAS input downloads ship raw bytes
in both directions regardless of any compression setting. On
bandwidth-limited links this leaves the full compression ratio on the
table for NativeLink's own traffic.
Mechanism
Opt-in
GrpcSpec.experimental_remote_cache_compression. Uploads andfull-blob downloads of blobs ≥64KiB use
compressed-blobs/zstd(clientencodes at zstd level 1 — measured 3.7GB/s single-core, never the
bottleneck at ≤1Gbps; level 3 bought nothing on the test corpus). Ranged
reads and small blobs stay identity. Downloads digest-verify at EOF;
decoder-detected corruption is terminal (
InvalidArgument), whileretryable mid-stream transport failures resume via an identity read at the
forwarded offset. Compressed uploads are explicitly non-resumable
mid-stream (the server contract): a transient failure fails fast with the
transport error after one attempt instead of burning the retry budget on
replays the server must reject. The streaming codec moved
nativelink-service→nativelink-util(service re-exports; server keepslevel 3).
Measured (token-bucket throttled real TCP, through this feature path, byte-verified both directions)
3.5–4.4× wall on compressible artifacts at both link speeds and in both
directions; zero wall or size penalty on incompressible data.
Notes
Fix zstd upload decoder rejecting exact-buffer-multiple blobs #2594 (in the relocated util file here); whichever merges second
resolves trivially.
GrpcStore::update; this branchrebases after Add opt-in join-the-flight ByteStream write dedup #2591 and inherits its early-completion drain in the
compressed path.
without compression enabled the first RPC fails fast with
InvalidArgument(documented + tested).experimental_chunked_uploads(Add opt-in CDC chunked uploads to GrpcStore #2595) wins for blobs aboveits threshold; combined-flags test lands in whichever merges second.
Tests
9 feature tests incl. fails-without-fix regressions for non-resumable
uploads (old code: 4 attempts then terminal
InvalidArgument; new: 1attempt, prompt transport error) and early-completion return, plus a
deterministic corrupt-download-is-terminal property test. Suites:
cargo check --tests --workspaceclean; bazel store+config+service+util68/68 with clippy pedantic + nightly rustfmt aspects.
🤖 Generated with Claude Code
https://claude.ai/code/session_01UXVtatcR9YMecBiu9RjwpC
This change is
Review update (2026-07-22)
The wire-compression path was hardened after adversarial review:
Focused Cargo and Bazel wire-compression suites, formatting checks, and
git diff --checkpass. The feature remains opt-in; compressed uploads remain non-resumable by design.