Skip to content

Configure zstd passthrough through the compression store - #2

Draft
walter-zeromatter wants to merge 123 commits into
mainfrom
user/wgray/zstd-store
Draft

Configure zstd passthrough through the compression store#2
walter-zeromatter wants to merge 123 commits into
mainfrom
user/wgray/zstd-store

Conversation

@walter-zeromatter

@walter-zeromatter walter-zeromatter commented Jul 15, 2026

Copy link
Copy Markdown

Summary

  • Add zstd as compression_algorithm.zstd on the existing compression store configuration instead of introducing a new public store type.
  • Keep blobs as standard zstd streams at rest and serve eligible --remote_cache_compression clients the stored stream byte-for-byte.
  • Expose zstd wire operations through an immediate store capability, so ByteStream and CAS services do not import or downcast to the concrete implementation.
  • Preserve bounded staging, upload timeouts, digest validation, recompression limits, and identity-client fallback behavior.
  • Update configuration examples and operator documentation for the unified compression-store shape.

Why

Compression is already modeled as a store with an algorithm choice, so zstd belongs on that existing configuration surface alongside LZ4. The zstd implementation remains specialized internally because it can reuse the stored frame on the REAPI wire; that behavior is exposed as a store capability rather than as a service dependency on a new concrete store type.

The capability is intentionally immediate. Configure the compression store as the instance CAS boundary to enable byte-for-byte passthrough. An outer wrapper remains correct but uses the normal decode/re-encode fallback at that boundary.

Configuration

{
  compression: {
    compression_algorithm: {
      zstd: {
        temp_path: "/tmp/nativelink-zstd",
        max_compressed_upload_size: "512MiB",
      },
    },
    backend: {
      memory: {},
    },
  },
}

Use a new or empty dedicated backend namespace for this representation. Rollout and rollback require clearing that cache namespace; there is no in-place migration from identity or LZ4 storage.

Validation

  • bazel test //nativelink-config:unit_test //nativelink-config:integration_tests/json5_test_test //nativelink-util:integration_tests/store_trait_test_test //nativelink-store:integration_tests/zstd_store_test_test //nativelink-service:integration_tests/bytestream_server_test_test //nativelink-service:integration_tests/cas_server_test_test
  • cargo +nightly fmt --all -- --check
  • git diff --check

The focused Bazel suite passes all six targets, including their Clippy aspects. Vale was not available in the local environment, so documentation lint was not run locally.

palfrey and others added 30 commits July 15, 2026 18:14
…a#2553)

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
When `experimental_read_batching` is configured on a gRPC store, full
reads of small blobs are coalesced into BatchReadBlobs RPCs instead of
issuing one ByteStream Read stream per blob. Each ByteStream read
carries a ~1.6ms fixed per-RPC cost; batched reads measured 52us/blob
at 4KiB (30.2x), 4.8x at 32KiB and 1.8x at 256KiB against a real
in-process gRPC server.

The coalescer uses slot-based group commit with no timers: callers
enqueue their read and try_acquire one of `dispatch_slots` semaphore
permits to become a dispatcher. A dispatcher drains up to
`max_batch_bytes` of pending reads (grouped by digest function, with a
per-entry overhead charge to bound entry counts) into a single
BatchReadBlobs request, deduplicates digests and fans response data out
to every waiter of a digest, validates entry size and identity
compression, and keeps draining until the queue is empty. After
releasing its permit the dispatcher re-checks the queue to close the
race with concurrent enqueues. When more than `max_queued_bytes` are
already waiting, new reads bypass batching and use the stream path so
the coalescer never blocks. Per-item failures (e.g. NOT_FOUND) fail
only that read; retryable per-item errors fall back to the ByteStream
path, which re-enters the retry machinery; whole-RPC failures are
broadcast to the batch.

Dispatchers run as detached background tasks holding a strong reference
obtained via a weak self pointer: cancellation of any individual reader
(timeouts, try_join siblings) can neither abort an in-flight batch RPC
nor strand still-queued waiters - a cancelled caller only drops its own
result receiver. Because batched reads share one upstream RPC across
many client requests, `experimental_read_batching` is rejected at
construction when combined with `forward_headers`, whose per-client
values (e.g. credentials) cannot be attached to a shared RPC.

The coalescer exports metrics under the store's read_batcher group:
batches_sent, blobs_batched, queue_bypasses, batched_read_errors and a
queued_bytes gauge (atomic mirror of the queue byte budget).

Partial reads, blobs above `max_blob_size_bytes`, non-digest keys, AC
stores and empty blobs keep the existing behavior. The feature is off
by default and unset config is zero behavior change.

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Co-authored-by: Marcus Eagan <marcuseagan@gmail.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
* Remove quoting around NetApp
* Fix clippy permitted list of idents
Co-authored-by: config reference bot <bot@tracemachina.com>
…eader adapter (TraceMachina#2574)

* test(service): compressed-download encode streams must not starve the blocking pool

A compressed-download encode stream drains at the gRPC client's pace. The
current implementation holds one tokio blocking-pool thread per stream for
the stream's entire lifetime (BufChannelReader blocks waiting for input and
blocking_send parks until the client consumes), so N concurrent downloads
with slow consumers occupy N pool threads and every unrelated spawn_blocking
user queues behind them. This test wires encode streams the way
ByteStreamServer::inner_read_compressed does, holds more of them than the
pool has threads, and asserts a trivial unrelated spawn_blocking closure
still runs. It fails against the current thread-per-stream implementation.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(service): encode compressed downloads asynchronously, off the blocking pool

stream_encode_compressed_download previously ran under spawn_blocking with a
blocking reader over raw_rx and blocking_send into the output channel, so one
tokio blocking-pool thread was parked per compressed download for the whole
stream at the client's drain rate. Concurrent downloads with slow consumers
could exhaust the pool (default cap 512), starving every other spawn_blocking
user — filesystem store I/O, upload decode, credential resolution — and the
per-thread zstd contexts, stacks, and buffers inflated memory accordingly.

The encoder is now an async fn driving zstd's raw streaming API
(ZSTD_compressStream via zstd::stream::raw::Encoder): async recv, inline
per-chunk encode (CPU bounded by the small channel chunk size), async send
for backpressure. No thread is held while waiting on either channel. The
wire format is unchanged: one well-formed zstd frame, identical to what
zstd::stream::read::Encoder emitted. The bytestream_server caller drops its
spawn_blocking wrapper; dropping the read stream still cancels the encode
because the future itself is dropped.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(service): decode compressed uploads asynchronously, off the blocking pool

Same defect shape as the download encode path: stream_decode_compressed_upload
ran under spawn_blocking, blocking on compressed_rx for client bytes and
parking in blocking_send while the store drained, holding one blocking-pool
thread per compressed upload for the stream's lifetime.

The decoder is now an async fn driving zstd's raw streaming API
(ZSTD_decompressStream via zstd::stream::raw::Decoder) with async channel
sends. Validation is unchanged: the per-chunk decoded-size cap (bomb
rejection), the exact final-size check, and the digest check are preserved,
and a stream that ends mid-frame is rejected as InvalidArgument (previously
surfaced as a read error from the blocking decoder).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* refactor(service): remove unused BufChannelReader adapter

The async streaming zstd rewrite left this blocking Read adapter with no
production users; drop it and its unit test.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

---------

Co-authored-by: Walter Gray <walter@0m.dev>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…a#2576)

Co-authored-by: config reference bot <bot@tracemachina.com>
Co-authored-by: Marcus Eagan <marcuseagan@gmail.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: Marcus Eagan <marcuseagan@gmail.com>
* Work towards dealing with redis eviction
* Boot redis subscription manager on start
* Handle eviction events
* Refactor RemoveItemCallback type
* Run remove callbacks for Redis
* Add redis_store_tester support for Redis eviction events
* Fix some build issues
* Add redis store eviction tests
* Add docs about required new redis config
* Better config get parsing
* Fix redis key decoding

* Test with redis_store plus existence cache

---------

Co-authored-by: Marcus Eagan <marcuseagan@gmail.com>
A read that failed with NotFound returned RetryResult::Err, which
bypassed the retrier entirely. This made a read-404 terminal even
though the store's retry config can classify NotFound as retryable
via retry_on_errors, so read-after-write races (an object still
finalizing or being repopulated after eviction) could never be
retried at the store level and had to be absorbed by scheduler
re-dispatch, exhausting max_job_retries under concurrent load.

Emit RetryResult::Retry instead and let the retrier classify the
error. Default behavior is unchanged: the retrier does not retry
NotFound unless retry_on_errors opts in, and has() still maps
NotFound to Ok(None) for existence probes.

Fixes TraceMachina#2582
Co-authored-by: Marcus Eagan <marcuseagan@gmail.com>
)

The FAQ answered conceptual questions (cost, caching, LRE, Rust) but
nothing operational. Add eight question-phrased pages distilled from
the rest of the docs — client wiring, configuration, store selection,
production deployment, observability, troubleshooting, architecture,
and contributing — each grounded in and linking back to the full page
it summarizes.

Also add a "Contributing to Docs or the Website" section to
CONTRIBUTING.md: the old web/platform bun setup/docs/preview workflow
no longer exists, and nothing documented the current Bun + Turborepo
web/ workspace or that it sits outside the Bazel build.

Signed-off-by: Alec Maliwanag <amaliwanag@gmail.com>
MarcusSorealheis and others added 30 commits August 4, 2026 13:47
* Introduce zstd inheritance policy

* update docs
Co-authored-by: Marcus Eagan <marcuseagan@gmail.com>
…a#2666)

Co-authored-by: config reference bot <bot@tracemachina.com>
* Harden the compressed GrpcStore download path

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.
* Clarify release steps in CONTRIBUTING.md

Make explicit that the version bump covers the Cargo.toml of every
nativelink-* crate (including nested ones) plus the regenerated lock
files, noting a standard release touches 17 files. Also replace the
full-changelog regeneration with git cliff --unreleased --prepend so
new release sections are added without rewriting or dropping previous
entries, with tag-sync and additive-diff verification steps.

* Update CONTRIBUTING.md

Co-authored-by: Tom Parker-Shemilt <palfrey@tevp.net>

---------

Co-authored-by: Tom Parker-Shemilt <palfrey@tevp.net>
* Upgrade rules_rs and hermetic_llvm

* fix chunking integration test
…ceMachina#2673)

should_retry relied on a `_ => true` catch-all to cover the retryable
gRPC codes, hiding an exhaustiveness hole: tonic::Code is a plain enum
(not #[non_exhaustive]), so any new status code added upstream would
silently default to retrying, potentially spinning on a permanent error
until max_retries. It was also only caught manually via a comment in
nativelink-error's CodeDef reminding maintainers to update both match
statements in retry.rs.

Replace the wildcard with an exhaustive match over all 17 Code variants.
New variants now fail the build and force an explicit decision, and new
codes default to permanent (no retry) rather than being retried. The
to_error_code match keeps its `_ => ErrorCode::Unknown` arm, since that
is a deliberate forward-compatible mapping for the closed config enum.
Behavior is unchanged for all existing codes.

Co-authored-by: Marcus Eagan <marcuseagan@gmail.com>
The rules_rust rustfmt aspect (nightly/2026-03-24) wanted three
reformattings in zstd_store.rs that the local nightly/2026-04-09
rustfmt did not flag, breaking the Bazel Dev, asan, ubuntu-24.04,
and Bazel 8.7.0 jobs. Apply them so both rustfmts agree.

Vale flagged 'validator' as a spelling error in stores.rs and the
store-overview.mdx; it is a legitimate word for the blocking decode
task, so add it to the TraceMachina accept vocabulary.
The #[nativelink_macro::nativelink_test] attribute on the inline
batch_identity_decode_waits_for_identity_admission test expands to
tracing_test::logs_assert. The unit_test target's deps lacked
@crates//:tracing-test, so the rules_rust clippy aspect (run by
bazel test //...) failed with E0433/E0425. The integration test
suite already had it; this mirrors nativelink-util's unit_test.
…raceMachina#2667)

* Update CONTRIBUTING.MD instructions template from v0.x.y to v1.x.y since were well into v1

* Finish 0.x.y -> 1.x.y in the release asset verification examples

---------

Co-authored-by: Marcus Eagan <marcuseagan@gmail.com>
fix date

Co-authored-by: Marcus Eagan <marcuseagan@gmail.com>
…na#2675)

* fix(worker): 🐛 lease active action inputs across eviction

Protect every digest in an action's Merkle closure from filesystem-tier eviction until cleanup completes.

* fix(worker): 🐛 complete action lease teardown efficiently

Reserve queued child directories before materialization and batch lease release to avoid rescanning the LRU once per input digest. Keep the reference-counted batch path covered by regression tests.

* fix(worker): 🐛 make lease release cancellation-safe

Run filesystem-tier lease release in a detached task before awaiting it, so cancellation during action cleanup cannot strand leases in a later tier.

* fix(worker): 🐛 harden action input lease lifecycle

* fix(worker): 🐛 satisfy stable test lint

* perf(util): ⚡ skip LRU scans when all entries are leased

* perf(util): ⚡ use a dedicated evictable LRU

Keep leased entries out of the eviction index so pressure eviction only visits valid candidates and returns immediately when none exist. Preserve LRU recency on normal accesses, re-enter released keys as MRU, and keep the filtering index valid across replacements.

* fix(util): 🐛 sync candidate LRU on conditional reads

Keep remove_if recency updates consistent with the dedicated evictable index so conditional probes cannot leave eviction order stale.

* refactor(util): ♻️ centralize eviction index updates

Keep resident and evictable LRU transitions in State helpers and collect eviction candidates in one pass. This makes lease and recency synchronization explicit while preserving low-watermark behavior.

* fix(worker): 🐛 satisfy clippy self method lint

* fix(worker): 🐛 satisfy pedantic duration lint

* refactor(worker): ♻️ streamline lease bookkeeping

Reuse the lease and elapsed-time helpers across eviction and directory materialization paths without changing behavior.

* refactor(worker): ♻️ simplify lease bookkeeping

Keep the lease API distinct from the directory-cache hook, avoid duplicate root reservations, and make lease reference-count handling explicit.

* fix(worker): 🐛 finalize action cleanup before lease release

* perf(util): ⚡ evict candidates without key cloning

* refactor(util): ♻️ simplify candidate eviction loop

* refactor: ♻️ simplify eviction and lease helpers

* fix(worker): 🐛 borrow stores during lease release

* fix(util): 🐛 track leased eviction state

* fix(util): 🐛 reap expired leased entries on release

* refactor(util): ♻️ simplify lease-aware eviction

Keep lease protection and diagnostics while using the existing LRU for a single eviction scan. Remove the auxiliary candidate index and tests that only covered that optimization.

* Revert "refactor(util): ♻️ simplify lease-aware eviction"

This reverts commit b09baff.

* test(worker): ✅ restore store existence assertions

* Revert "test(worker): ✅ restore store existence assertions"

This reverts commit d4e4630.

* Revert "fix(util): 🐛 reap expired leased entries on release"

This reverts commit 3ad9b12.

* Gate active input leases behind experimental_active_input_leases (default off)

Leased inputs are excluded from size/count/TTL eviction, so a local
filesystem tier can temporarily exceed its configured max_bytes /
max_count while an action holds its input closure. Make the behavior
opt-in: a new LocalWorkerConfig field, experimental_active_input_leases
(default false), controls whether RunningActionImpl takes an
ActionInputLease. When disabled the worker never leases a digest and
eviction behaves exactly as before this change; a new regression test
asserts an active action's inputs remain ordinary eviction candidates
with the flag off.

---------

Co-authored-by: Marcus Eagan <marcuseagan@gmail.com>
Co-authored-by: Rebecca Corcillo <4566203+corcillo@users.noreply.github.com>
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.