Improve prometheus utils api by allada · Pull Request #210 · TraceMachina/nativelink

allada · 2023-07-19T15:42:21Z

This makes things a little easier to use and collapses more of the functions into a single main entry point .publish() with the exception of .publish_stats(), because we want the user to understand when they publish slices of numbers it is treated as a stat.

This change is

This makes things a little easier to use and collapses more of the functions into a single main entry point `.publish()` with the exception of `.publish_stats()`, because we want the user to understand when they publish slices of numbers it is treated as a stat.

allada

+@chrisstaite-menlo

Reviewable status: 0 of 5 files reviewed, all discussions resolved (waiting on @chrisstaite-menlo)

chrisstaite-menlo

Reviewed 1 of 4 files at r1, 4 of 4 files at r2, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @allada)

…low tier Adds Phase 2 to graceful shutdown's `flush_slow_writes` so blobs that live in the FastSlowStore fast-tier MemoryStore but have no corresponding in-flight slow-write entry are written to the slow tier before SIGTERM returns. Pre-fix `flush_slow_writes` only drained `in_flight_slow_writes` (blobs whose background spawn was still running); MemoryStore-only blobs that arrived via slow-tier read back-population, peer mirror promotion, or a watchdog-failed write that already had its in-flight entry removed were silently lost on exit. The MemoryStore vanishes with the process, the AC entry survives in Redis, and Bazel later fails with "Lost inputs no longer available remotely." TraceMachina#206's investigation observed 9904 of 66174 worker-reported blobs missing for 6+ hours after restart, with 4 of 5 sampled small-blob digests missing from Redis (slow tier). This is the actual OOM/SIGTERM-correlated production fire that TraceMachina#210 addresses (separate from TraceMachina#203's Vec<Bytes> accumulation work). Wire-up: - `FastSlowStore::flush_fast_to_slow_at_shutdown(deadline)` enumerates the fast tier via the generic `StoreDriver::list` API, snapshots the keys, and for each one (a) skips if the slow tier already has the digest, (b) reads from the fast tier, (c) writes to the slow tier via `update_oneshot`. Per-entry errors are logged + skipped (one failing blob does not abort the loop). The deadline is enforced by checking elapsed time before each iteration; remaining keys are reported as unflushed. - `StoreManager::flush_slow_writes` runs Phase 2 AFTER the existing Phase 1 in-flight drain, splitting the budget. Phase 2 gets a 1-second floor when Phase 1 used the full timeout so it can still make progress on a near-empty fast tier. Each FastSlowStore is drained concurrently under a single global deadline. - `src/bin/nativelink.rs` outer-guard slack bumped from +2s to +8s to accommodate Phase 2's worst-case 1s floor + its own 2s outer guard. Tests in `nativelink-store/tests/shutdown_flush_memory_test.rs` (production-composition per CLAUDE.md): - `shutdown_flushes_memory_only_blobs_to_slow_tier` — under-action, primary contract (5 blobs, all must land in slow tier) - `shutdown_flush_propagates_through_production_chain` — VerifyStore + ExistenceCacheStore + SizePartitioningStore wrapping the FastSlowStore, asserting wrapper layers do not break the flush - `shutdown_flush_respects_deadline_when_slow_tier_is_slow` — over-action, asserts the deadline cap; 50 blobs × 200ms slow probe with a 200ms deadline must return promptly with unflushed > 0 - `shutdown_flush_continues_past_per_entry_errors` — per-entry tolerance, every-other-fail probe; ≥half must still land - `shutdown_flush_skips_blobs_already_in_slow_tier` — skip-existing, panic-probe never called when blob is already in slow tier Mutation step (CLAUDE.md mandatory): commenting out the function body turns 4 of 5 tests RED with their `TraceMachina#210`-tagged messages. Evidence at `.claude/reviews/210-shutdown-flush/mutation-evidence.md`. The 5th test (skip-existing) trivially passes with the mutation; the other four cover the load-bearing under/over-action contracts. Sibling regression sweep (per-bin, all green): fast_slow_store_test (47), memory_store_test (10), verify_store_test (17), mirror_durability_test (15), composability_test (8), fast_slow_local_only_reads_test (5), fast_slow_store_inner_miss_gate_siblings_test (1), waiter_explicit_termination_test (1), writer_termination_per_site_test (5), populate_pinning_test (3), pin_expire_failed_writes_test (4), production_composition_test (4), shutdown_flush_memory_test (5), existence_cache_eviction_codes_test (11), failpoint_tests (14). Mirror blobs (`mirror_blobs`) are intentionally NOT flushed — those are pinned in memory only by design until the server's BlobsInStableStorage ack arrives, and writing them to the slow tier would defeat the mirror-only invariant. Interaction with TraceMachina#203 (`InFlightEntry` shape, 8a1239a): none. The existing `flush_slow_writes` Phase 1 still drives `in_flight_empty_notify` on the `Arc<StreamingBlobInner>`-shaped entries unchanged; Phase 2 operates on the fast-tier MemoryStore independently. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

… in_flight_slow_writes + S1 — wire BazelChunkedDispatcherImpl ↔ ChunkedReadRegistry Two coupled fixes that share the dispatcher impl surface: **B2:** Phase 2.7's chunked dispatch path now registers the digest in the FastSlowStore's `in_flight_slow_writes` map for the duration of the dispatch (inserted before, removed after the chunked driver's async-commit reaper completes). Restores three contracts that were broken for chunked-path blobs: - `has_with_results`: in-flight check returns Some. - `flush_slow_writes` (TraceMachina#210 graceful drain): waits for chunked dispatches to complete before returning. - `get_part` read-cascade step 1: in-flight chunked-path bytes visible. The reaper polls the chunked-driver in-flight map for completion (commit success OR failure), then removes the digest + fires the `in_flight_empty_notify` if the map drains. **S1:** `BazelChunkedDispatcherImpl` now holds an optional `Arc<ChunkedReadRegistry>` + `register`s/`deregister`s the per-blob `Arc<ChunkedDriver>` for the dispatch lifetime. Phase 2.5's read cascade can now find in-flight drivers and serve bytes from the in-memory pin during the async-commit window. Production wiring (`src/bin/nativelink.rs`): walks every CAS-backing FastSlowStore (using the existing `find_fast_slow_for_pin` chain walk), tries to downcast the slow tier to `Arc<FilesystemStore<FileEntryImpl>>`, and on success wires both the registry AND the dispatcher via the new `wire_bazel_chunked_dispatcher` helper. Non-FilesystemStore slow tiers (e.g. SizePartitioning, GrpcStore) are skipped with an info log — out of scope for v1. New helper `wire_bazel_chunked_dispatcher` in chunked_write_handler.rs creates the registry, constructs the dispatcher with both the registry AND the in-flight-tracking handles, and installs everything on the FastSlowStore in one call. Builder methods on `BazelChunkedDispatcherImpl`: - `with_registry(Arc<ChunkedReadRegistry>)` — wires S1. - `with_in_flight_tracking(map, notify)` — wires B2. Both kill-switches still default OFF; flipping requires explicit user sign-off per CLAUDE.md `feedback_async_to_sync_requires_explicit_signoff`. Updated test call-sites (`bazel_facing_internal_chunking_test.rs`): existing `dispatch_chunks_to_driver` callers now pass `None` for the new `pin_budget` and `chunked_read_registry` params (preserves their existing test semantics). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

…igests set The first B2 attempt inserted chunked-path digests into the legacy in_flight_slow_writes map with empty Vec<Bytes>. The legacy map's read path has a size-mismatch eviction guard that REMOVES entries whose `total_len != digest.size_bytes()` — empty-Vec markers were evicted on first lookup, defeating the purpose. Fix: a SEPARATE `chunked_in_flight_digests: Arc<Mutex<HashSet<DigestInfo>>>` on FastSlowStore, populated by `BazelChunkedDispatcherImpl::dispatch` and removed by its reaper. Read paths fold the new set in: - `has_with_results` (both local_only_reads + non-local branches): if the digest is in the chunked set, return Some(digest.size_bytes()). - `flush_slow_writes` (TraceMachina#210 graceful drain): waits for BOTH the legacy map AND the chunked set to drain. The chunked-driver pin (registered through ChunkedReadRegistry; see S1) remains the source of truth for the actual byte content during the async-commit window. The chunked-in-flight set is just the lifecycle/visibility marker. Updates `BazelChunkedDispatcherImpl::with_in_flight_tracking` to take the new HashSet handle instead of the legacy map handle. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

…c-commit + 2.5 pin accessor) + B2 graceful-drain New test file `nativelink-service/tests/chunked_p25_p27_e2e_test.rs` covers the cross-phase integration that the per-phase tests left untested: **S2 — E2E dispatcher↔registry wire:** `e2e_dispatcher_registers_driver_in_registry_during_dispatch`: constructs a real BazelChunkedDispatcherImpl via the new `with_registry` builder; drives `FastSlowStore::update` through the β async-commit path; concurrent observer task polls `registry.get(&digest)` during dispatch and asserts the per-blob `Arc<ChunkedDriver>` IS registered (proves the dispatcher actually calls `registry.register(...)` rather than the dead-wire scenario the testing-czar review identified). Post-dispatch, asserts the registry drains (deregister fired). Mutation-tested: removing the `reg.register` call in `dispatch_chunks_to_driver` → red-fails with the specific S1 dead-wire message. **B2 — graceful-shutdown drain:** `b2_flush_slow_writes_waits_for_chunked_dispatch_commit`: drives a slow chunked upload, calls `flush_slow_writes` mid-dispatch, asserts (a) drain returns 0 (all flushed) AND (b) elapsed > 100us (the drain ACTUALLY waited rather than no-op'ing). Without B2 wiring, flush_slow_writes returns immediately and the chunked write would be lost on shutdown. Mutation-tested: removing the `chunked_in_flight_digests.lock().insert(digest)` in `BazelChunkedDispatcherImpl::dispatch` → drain returns immediately (elapsed = 20us) → red-fails with the specific TraceMachina#210 contract message. Cargo.toml registration: new `[[test]]` entry gated on `["chunked_fast_slow", "test-utils"]`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

TraceMachina#210 SIGTERM regression) The local walker in `StoreManager::flush_slow_writes` was the fourth copy of the broken `inner_store(None)` walker pattern (TraceMachina#335 V3 consolidation went 3→1, missing this one). For the production composition `WorkerProxyStore → ExistenceCacheStore → VerifyStore → SizePartitioningStore(16384) → FastSlowStore`, the local walker bottomed out at SP's `inner_store(None) -> self` and returned None — making `flush_slow_writes` silently log "no FastSlowStore registered; skipping" on every SIGTERM and defeating the TraceMachina#210 graceful-shutdown fix from `27d79ea4` (Apr 22, 2026). Result: in-flight slow writes lost on every nativelink restart in production. Migrate to the canonical `nativelink_store::wrapper_walker::find_fast_slow_via_chain` (shared with the V3 self-retry drainer), which passes `synthetic_large_key()` to descend the upper arm correctly. All four internal callsites (Phase-1 targets-build, Phase-1 spawn block, progress ticker, Phase-2 spawn block) now share one implementation — no more 4-copy-walker drift. Adds regression test `store_manager_flush_descends_production_composition` that wraps a real `FastSlowStore` in the production `ECS → VS → SizePartitioning(16384) → FSS` chain, registers it via `StoreManager::add_store`, and asserts the Phase-2 `MemoryStore`-only flush propagates to the slow tier. Mutation step verified: reverting to the buggy local walker red-fails with the bespoke message "BLOCK-1 regression: StoreManager walker failed to descend production composition — Phase-2 flush did not propagate; slow tier is missing the blob" and the production log line "flush_slow_writes: no FastSlowStore registered; skipping" appears in the test output — exactly the production failure mode. Reviewer findings addressed: - code-reviewer + testing-czar + red-team BLOCK-1 (triple-confirmed walker bug, defeats TraceMachina#210) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

`BazelChunkedDispatcherImpl::dispatch` used to manually `set.lock().insert(digest)` then `dispatch_bazel_facing_internal_chunking .await` then `set.lock().remove(&digest)` in match arms. The await is a cancellation point: when an upstream `try_join!` (e.g. `bytestream_server.rs:1761-1775`) short-circuits on a producer Err and drops the dispatch future, the matched-arm removal NEVER runs. Result: the digest leaks into `chunked_in_flight_digests` until process restart; `FastSlowStore::has_with_results` reports `Some(size)` for leaked digests forever; `ExistenceCacheStore` caches the `Some`; FMB returns "present" forever; subsequent populates NotFound on the missing bytes. The production smoking gun was the shutdown WARN at `fast_slow_store.rs:2012-2017` ("unflushed chunked-path write at shutdown") firing 3x for `4cb49612...` 10.5h after the cancel. Fix: introduce `InFlightChunkedGuard`, an RAII guard whose `Drop` removes the digest and fires the empty-notify. Drop runs on every exit path including cancellation, so the leak is impossible by construction. The success path explicitly `disarm`s the guard (turning Drop into a no-op) and hands the (set, digest, notify) tuple to the post-dispatch reaper task, which performs removal only after the chunked-driver's in-flight tracker drains -- preserving the TraceMachina#210 graceful-drain semantics. The Err path lets the guard's natural Drop perform removal, matching the prior behavior. Tests in `nativelink-service/tests/chunked_inflight_cancel_safety_test.rs`: - `guard_drop_removes_digest`: RAII unit contract. - `guard_drop_notifies_when_set_becomes_empty`: preserves the `flush_slow_writes` lost-wakeup contract on the cancel path. - `cancel_mid_await_removes_digest`: production-shape regression test. Wraps the guard in a future awaiting a long sleep, races against a short timer in `select!`, asserts the set is empty after the dispatch future is cancelled. With the bug present (manual insert/remove), this red-fails with the bespoke message "InFlightChunkedGuard MUST remove digest on cancellation -- leak detected". With the fix, this passes. - `disarm_prevents_drop_removal`: success-path contract -- after `disarm()`, Drop is a no-op so the spawned reaper owns removal. Mutation step (CLAUDE.md TDD TraceMachina#5) verified: commenting out the body of `impl Drop for InFlightChunkedGuard` red-fails `cancel_mid_await_removes_digest` with the bespoke message above (and also red-fails `guard_drop_removes_digest` and `guard_drop_notifies_when_set_becomes_empty`). Drop body restored. Invariant being violated: every digest inserted into `chunked_in_flight_digests` MUST be removed before the dispatcher returns control to its caller (or, in the success path, before the chunked-driver's in-flight tracker drains). Mechanism that violated it: `chunked_write_handler.rs:2462` unconditional insert paired with match-arm removal at `:2492-2530`, both bypassed when the inner `.await` is cancelled. Mechanism that re-establishes it: `InFlightChunkedGuard::drop` unconditionally removes (when armed), runs on every future exit path including cancellation. Composite invariants this fix interacts with: - TraceMachina#210 graceful-drain (preserved via `disarm()` + spawned reaper) - lost-wakeup on `in_flight_empty_notify` (preserved -- Drop fires notify_waiters when the set transitions to empty) - has_with_results chunked-path Some/None invariant (no longer poisoned by leaked digests) Test that proves the fix re-establishes the invariant: `cancel_mid_await_removes_digest`. Builds verified: `cargo check --tests -p nativelink-service --features chunked_fast_slow` (lib + new test), `cargo check -p nativelink-service -p nativelink-store` (no chunked_fast_slow). 4/4 tests pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

… memory-only CAS blobs Operator directive (2026-06-23): "remove the timeout from the shutdown path; give shutdown unlimited time to flush blobs to redis and to disk. we can't have this keep happening every time the server restarts." At the 2026-06-23 17:38 buildcache restart the Phase-2 shutdown flush hit its 30 s deadline and lost 827,204 memory-only SMALL_CAS_CACHED blobs (`flushed=0 skipped_already=591792 deadline_exceeded=827204`, `elapsed_ms=29997`), causing CI "lost input too many times (TraceMachina#21)" failures across >=3 branches for ~30 min post-restart. ROOT CAUSE of flushed=0 (verified, not a counter bug): the per-blob loop in `flush_fast_to_slow_at_shutdown` was STRICTLY SERIAL — one `slow.has` RTT then one `slow.update_oneshot` RTT per blob. With 1.4M keys the 30 s budget was spent on ~591k serial Redis `has` probes (all already-present -> skipped) and the deadline (`started.elapsed() >= deadline`) fired before the loop reached a single blob that NEEDED writing. `flushed` increments correctly on the success path; the loop simply never reached a writable blob in time. So removing the deadline alone was necessary-but-insufficient — the serial structure had to be parallelized too, or an unbounded drain of 1.4M serial RTTs would near-wedge. FIX (three parts): 1. Deadline `Duration` -> `Option<Duration>`; `None` = unbounded (production path), `Some(d)` = best-effort within `d` (kept for the deadline-bound test + any future bounded caller). `store_manager` Phase 2 and the bin now pass `None`; the bin drops its outer `tokio::time::timeout` around `flush_slow_writes` (it would have re-capped the unbounded drain). Phase 1 (in-flight slow-write drain) stays bounded by `timeout` — it was NOT the incident surface (`total_pending=0` in the incident). 2. Bounded-concurrency drain (`SHUTDOWN_FLUSH_CONCURRENCY = 64`, via `buffer_unordered`) so N serial RTTs become N/K; a large fast tier drains in minutes, not tens of minutes. Skip-existing `has` pre-check is preserved (Test 5 still passes) but now runs concurrently. 3. `info!` forward-progress line every 50,000 processed blobs (`flushed/skipped/remaining/elapsed`) so an operator watching journalctl through a long drain sees motion, not a wedge. Both arms are unbounded: SMALL_CAS_CACHED (Redis slow tier — the 827k victim) AND cas_FAST_SLOW_STORE (FilesystemStore /srv/bulk slow tier). The disk arm is a plain async `update_oneshot` (FilesystemStore dedups via its in-memory index; no sync-write primitive added anywhere). INVARIANT WALK - Invariant being violated: every memory-only CAS blob in a FastSlowStore fast tier must reach its slow tier before process exit, or AC entries dangle and Bazel reports "lost input". - Mechanism that violates it: `fast_slow_store.rs` serial per-blob flush loop + 30 s deadline — burned the whole budget on serial `has` RTTs, flushed 0. - Mechanism that re-establishes it: `flush_fast_to_slow_at_shutdown(None)` runs the per-blob has/get/update at concurrency 64 with NO deadline, draining to zero (`unflushed = errored + deadline_exceeded`, both 0 under `None`+no-error). - Composite invariants this fix interacts with: mirror_blobs >=2-replica (NOT touched — mirror blobs are deliberately not flushed here); ZFS sync=disabled durability (unchanged — async writes only); admission/eviction/pin (untouched — shutting_down already fences new writes via Phase 1). - Test that proves the fix: `shutdown_flush_unbounded_drains_all_blobs` (all N land, unflushed==0) + `shutdown_flush_unbounded_drains_despite_slow_tier` (slow tier + None still drains all N; exactly N writes issued). BEHAVIOR CHANGES (every runtime change vs production composition) - FastSlowStore::flush_fast_to_slow_at_shutdown: bounded(30s)+serial -> unbounded(None)+concurrent. In prod (StoreManager Phase 2 over SMALL_CAS_CACHED + cas_FAST_SLOW_STORE): shutdown now blocks until every memory-only blob is durable instead of dropping the remainder at 30 s. - StoreManager::flush_slow_writes Phase 2: removed per-store deadline math (PHASE_2_FLOOR) and the Phase-2 outer wall-clock timeout; Phase 2 awaits all per-store drains to completion. Phase 1 unchanged. - src/bin/nativelink.rs shutdown step 4: removed the `tokio::time::timeout(flush_budget+8s, ...)` wrapper around `flush_slow_writes`; calls it directly. Phase-1 budget still passed in. - ACCEPTED RISK (per directive): a genuinely wedged slow tier now blocks SIGTERM-to-exit indefinitely. No in-process safety valve added (TRUE unbounded per directive). REQUIRES systemd `TimeoutStopSec=infinity` (currently 90 in /etc/systemd/system/nativelink.service — parent applies at deploy) or systemd SIGKILLs the unit mid-flush and re-introduces the loss. TDD: 8/8 tests green. Mutations verified: - force `Some(Duration::ZERO)` on the None path -> unbounded test red-fails "TraceMachina#210 UNBOUNDED flush must drain ALL memory-only blobs — data-loss regression: 250 of 250 blobs left unflushed with no deadline". - skip the actual `update_oneshot` write -> content check red-fails "blob ... missing from slow tier post-unbounded-flush". No fsync/sync-write primitives; no tokio-worker blocking; mirror invariant untouched. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

…rver durability bundle, directive-2) Adds graceful-shutdown Phase 3 (pull worker-only CAS blobs to the server slow tier while workers are still connected) plus a Phase-0 Bazel-REAPI quiesce, on top of directive-1's unbounded flush. New shutdown ordering: quiesce Bazel REAPI -> flush server fast tier -> pull worker-only blobs -> [directive-3 persist-locality slot] -> evict workers -> stop accepting -> drain -> shut down local workers -> exit. Eviction MOVED after the pull (you cannot pull from an evicted/disconnected worker) but stays BEFORE listener-drain (preserves d204e1b's drain-promptness rationale). Design: .claude/audits/shutdown-pull-worker-blobs-design-2026-06-23.md. == Invariant-walk == Invariant being violated: after a server restart, a CAS blob that exists ONLY on workers is not served by the server until the worker re-handshakes and re-backfills, causing a multi-minute NotFound window (Bazel "lost input TraceMachina#21"); and during shutdown the flush cannot converge because new Bazel writes keep arriving. Mechanism that violates it: src/bin/nativelink.rs evicted/disconnected all workers FIRST (old Step 1), wiping the locality map + per-worker tx before any localize attempt (worker_api_server.rs remove_endpoint); and no gate stopped new Bazel REAPI during shutdown so the flush chased fresh writes. Mechanism that re-establishes it: (Phase 0) BazelReapiQuiesce latch flipped at SIGTERM start makes new requests on the public Bazel listener return UNAVAILABLE (in-flight drain) so the in-flight write set converges; (Phase 3) ShutdownPuller::run (worker_api_server.rs) drives the existing has_with_results-gated request_missing_blob_uploads in a completion-poll loop over connected_workers_with_senders, BEFORE eviction, until every locality digest is durable in the server slow tier or has zero connected source (at-risk-skip; never wedges; no-progress watchdog bounds a stalled worker). Composite invariants this fix interacts with: (1) mirror_blobs >=2-replica + BIS-ack-as-durability-oath -- the pull ADDS the server as a replica and reuses the has_with_results-gated mark_stable (worker_api_server.rs ~2525), NOT weakened; (2) d204e1b evict-before-listener-drain -- preserved (eviction still precedes drain); (3) TraceMachina#210 unbounded memory->slow flush -- the pull runs AFTER it and depends on it for the existence check; (4) no per-RPC timeout on internal RPCs -- the pull is unbounded-time, bounded by skip-policy + no-progress watchdog, not RPC deadlines; (5) worker-facing CAS listeners (:50071/:50072) + worker_api (:50061) stay OPEN through shutdown -- the pull needs them. Test that proves the fix: shutdown_pull_lands_worker_only_blob_on_local_disk_before_future_resolves (pull future does not resolve until has_with_results == Some); shutdown_pull_skips_zero_source_blob_and_does_not_hang (zero-source -> resolves, at_risk_skipped==1, no wedge); closed_gate_rejects_new_request_with_unavailable (quiesce -> grpc 14). == What changes (architectural; .claude/rules/architectural-changes.md) == - X (today): SIGTERM evicts/disconnects workers FIRST, then drains, then flushes; worker-only blobs abandoned on disconnect; no Bazel-REAPI gate. - Y: SIGTERM quiesces Bazel REAPI, flushes, PULLS worker-only blobs (workers connected, unbounded), THEN evicts, drains, exits. - Shutdown-ordering change (worker disconnect moves after the new pull) + flow-control change (new unbounded-time, network-driven completion loop on the shutdown path). NOT an async<->sync flip: no .await-on-send added to a steady-state pump, no tokio::spawn removed from a producer/consumer; the loop is shutdown-only and uses the EXISTING fire-and-forget send + a new poll, so it adds no synchronous backpressure to any production write path. == Behavior changes (runtime, vs production composition) == - Steady state (no SIGTERM): NONE. The quiesce latch starts false (one Relaxed atomic load per request on the quiesce-marked listener only, never short-circuits). ShutdownPuller is only invoked from the SIGTERM handler. No production read/write path changes. - Config: new ServerConfig.quiesce_on_shutdown (default false). Absent => pre-change behavior. Set ONLY on the public Bazel listener (:50051). The worker-facing CAS listeners (:50071/:50072) and worker_api (:50061) do NOT set it and are unaffected. - Shutdown: ordering changes as above. The flush moves before eviction; the pull is new; eviction moves after the pull. Local-worker shutdown + exit unchanged. == Hard-constraint compliance (CLAUDE.md) == - NO fsync/sync-write: the pull writes nothing synchronously; blobs land via the worker's slow_store.update -> server gRPC CAS -> FilesystemStore/Redis async path; durability = mirror >=2-replica + ZFS sync=disabled. - Never block a tokio worker: pull is pure async (has_with_results, tokio::time::sleep poll, mpsc send); no std::thread::sleep / sync FS. - Unbounded-buffer rule: NO new blob buffer. Only a Vec<DigestInfo> + endpoints locality snapshot, annotated CAPPED AT digest_count() (~40 B/ entry, one-shot, dropped post-loop). Blob bytes stream worker->disk, never collected server-side. - BIS not weakened: the pull reuses the has_with_results-gated mark_stable; it acks-to-unpin only AFTER the server's existence check confirms durability. Tee logs: /tmp/dir2-*-{libbuild,testcompile,testrun,binbuild, mut-zerosource2,mut-skippull,mut-nocompletion,mut-quiesce}.log. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

…_stable + chunked-safe shutdown flush Server-only, strictly-safer Stage 1 of the v3 durability-ack design (.claude/audits/durability-ack-architecture-design-2026-06-24-v3.md §3.0 keystone + Change A). No proto/wire change, no pin-policy change. Three in-scope changes: 1. `has_durably` — a forced-`DurableDelegation` durable-presence query mirroring the `MarkStableDelegation` pattern. New `DurableDelegation` enum + no-default-body forced `durable_delegation()` method (a missing wrapper arm is a COMPILE ERROR — §6.8 seam) + `has_durably()` with an enum-dispatch default whose `Leaf` arm reports ABSENT (NOT a `has_with_results`-collapse). Threaded through the CAS chain: WPS(Inner, no locality short-circuit) → Verify(Inner) → ECS(Inner, bypasses the existence cache) → SizePartitioning(Leaf+override, routes by size) → FSS(Leaf+override → `slow_store.has_with_results` ONLY). RefStore resolve-and-forwards. 2. Durable-gate `mark_stable` in `worker_api_server::request_missing_blob_uploads`: the BIS→unpin OATH is now gated on `has_durably` (durable subset), not `has_with_results`. The upload-request decision keeps `has_with_results`. 3. Shutdown-flush (Change A) in `FastSlowStore::flush_fast_to_slow_at_shutdown`: KEEP the whole-tier `fast_store.list()` byte source (TraceMachina#210 superset, the only source reaching chunked bytes), DROP the per-key `slow.has()` pre-check (the RCA `flushed=0 deadline_exceeded=827204` burner) and the `deadline` param (R2), FILTER to the in-memory NOT-YET-DURABLE at-risk digest set (in_flight ∪ chunked ∪ failed). Applies to both FSS instances via the single `store_manager` call site. Filter-predicate choice (Change A.1): the at-risk DIGEST SET, not a fast-tier pin-state query — already queryable (3 `self.` fields, no new moka API) and the SAME set assumption-auditor proved COMPLETE for the acked-not-durable class. Default-flush-on-doubt: a content-addressed re-write is an idempotent no-op. --- Invariant walk --- Invariant being violated: the worker's pin on a CAS digest is the ≥2-replica backstop while the server's only copy is in volatile RAM; the server must NOT tell the worker to unpin until it holds a DURABLE copy. Mechanism that violates it: `worker_api_server.rs` request_missing_blob_uploads gated `mark_stable` (→ BIS → worker unpin) on `has_with_results`, which the FSS satisfies from the fast tier / in-flight maps / RAM-only mirror — so BIS could fire on RAM-only presence, and a server restart then loses the only durable copy of a mirror_blob. Mechanism that re-establishes it after the fix: the gate now calls `cas_store.has_durably` (worker_api_server.rs request_missing_blob_uploads), which routes through the chain to `FastSlowStore::has_durably` → `slow_store.has_with_results` (fast_slow_store.rs) — durable-tier-only — so `mark_stable` fires only once the slow write has landed. Composite invariants this fix interacts with: (a) the ≥2-replica invariant (BIS-ack ⇒ durable server copy now holds by construction); (b) the TraceMachina#210 graceful-shutdown drain (Change A flushes the at-risk superset, no acked-not-durable blob lost); (c) the async-slow-write boundary (UNCHANGED — no .await added on a send, no tokio::spawn removed, no TraceMachina#203-class flip); (d) the at-risk DIGEST SET completeness proof (the flush filter reuses it). Test that proves the fix re-establishes the invariant: mark_stable_does_not_fire_on_ram_only_presence_test (keystone, durable_gate_mark_stable_test.rs). --- Behavior changes (enumerated against production composition) --- - worker_api_server `mark_stable` gate: has_with_results → has_durably for the unpin subset. Production composition (WPS→Verify→ECS→SizePart→FSS): BIS now fires LATER (once the FSS slow tier holds the blob), NEVER earlier. Strictly-safer; the upload-request path is unchanged. - FastSlowStore::flush_fast_to_slow_at_shutdown: signature drops `deadline: Option<Duration>`; drops the per-key `slow.has()` probe; flushes only the at-risk subset (was: every fast-tier resident, has()-skipping durable ones). Production composition (the single store_manager.rs call site, both FSS instances): drains the chunked acked-not-durable class losslessly (its bytes are in the fast tier; its digest in chunked_in_flight_digests), skips already-durable residents without a slow probe. nativelink.rs post-pull flush comment updated to match. - `DurableDelegation` arm added to all 22 production StoreDriver impls + the benchmark mock + all test mocks. Runtime behavior of `has_durably`: - FastSlowStore: routes to slow tier (durable query). Verified in the CAS-chain compositions (keystone + seam tests). - WorkerProxyStore: Inner → inner.has_durably, NO locality augmentation (a locality hit is peer RAM, never durable). Verified: WPS is the outermost prod CAS wrapper. - ExistenceCacheStore / OntapS3ExistenceCacheStore: Inner → bypasses the existence cache (a cache hit is not durability proof). - SizePartitioningStore: Leaf + per-digest size-routed override. - RefStore: Leaf + resolve-and-forward override. - VerifyStore / CompressionStore / AcProxyStore / CompletenessCheckingStore: Inner (transparent forward). - MemoryStore / NoopStore / FilesystemStore / RedisStore / GrpcStore / S3 / GCS / Azure / Mongo / OntapS3: Leaf → report ABSENT. These are leaf tiers; in the prod CAS chain the FSS short-circuits durable queries to `slow_store.has_with_results` before reaching a leaf's `has_durably`, so ABSENT is never consulted in prod and is the strictly-safer default elsewhere. - DedupStore / ShardStore: Leaf → ABSENT. NOT in the prod durability-gated CAS chain; a future on-chain composition must add a real override (documented at each arm). - No other runtime behavior change. No proto/wire/config/pin-policy change. The async-slow-write boundary is PRESERVED. Mutation-verified (fail→pass→mutate, each restored): keystone gate (revert to has_with_results → "BIS fired on RAM-only presence"); seam collapse (FSS has_durably → has_with_results → "has_durably collapsed..."); seam compile-fail (omit a durable_delegation arm → E0046); flush filter drop (if-false → "already-durable resident re-written..."); flush slow.has re-add (→ "RCA budget-burner is back"). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

…on, no auto-loss Server SIGTERM hung forever burning 111% CPU "still serving the backfill storm": the unbounded shutdown drain (TraceMachina#210) never reached a fixed point because already-open worker connections kept ticking BlobsAvailable (~100ms), each tick soliciting fresh UploadMissingBlobs + running has_with_results/ has_durably scans, re-feeding the at-risk set the drain was trying to converge (observed total=11887 missing=11887 mid-shutdown). The only exit was a lossy manual SIGKILL. Invariant being violated: the shutdown drain must reach its fixed point (every at-risk in-memory CAS blob lands on its size-correct durable tier) instead of being re-fed forever by worker-solicited intake. Mechanism that violates it: src/bin/nativelink.rs accept_stop_tx (the only worker-intake stop) fires AFTER the unbounded flush/pull, and worker_api_server.rs handle_blobs_available keeps soliciting UploadMissingBlobs + the cooldown-bypassing pinned-mirror pull on every tick during the drain. Mechanism that re-establishes it: a new ShutdownQuiesce latch (nativelink-service/src/worker_quiesce.rs), flipped at SIGTERM Phase 0b in src/bin/nativelink.rs BEFORE the unbounded flush, read at the entry of WorkerConnection::request_missing_blob_uploads (new quiesce: Option param). The TWO handler-invoked feeds (backfill+mark_stable spawn; pinned-mirror-pull spawn) pass Some(latch) and early-return -> intake -> 0 -> the unbounded fast->slow drain reaches its fixed point. The server-initiated ShutdownPuller::run passes None and stays open (it IS the drain, not the storm). Composite invariants this fix interacts with: - no-fsync (BLOCK): no I/O primitive added; flush uses existing async write path. Held. - slow-writes-async (business req): unchanged; latch only stops NEW worker- solicited writes, no async->sync flip. Held. - >=2-replica / BIS ack: suppressing mark_stable during shutdown does not strand a worker pin; server is exiting and post-restart BIS re-acks (same as the evict/GoingAway path today). Held. - no per-RPC timeout (operator directive): no per-RPC timeout added; liveness is the latch + the existing skip-policy/no-progress watchdog. Held. Tests that prove the fix: - quiesce_suppresses_handler_backfill_solicitation (handler solicits nothing + shutdown_suppressed_backfill_solicitations_total advances when quiesced). - shutdown_pull_still_solicits_when_quiesced (ShutdownPuller still lands a worker-only blob while the latch is set). - flush_does_not_abandon_at_risk_blob_when_phase1_budget_exceeded (Phase-2 drains the post-budget residue; no data loss). - stall_detector_fires_on_wedged_slow_tier_without_killing (per-size-class gauges + stall detector fire on a wedged tier WITHOUT killing/abandoning). - worker_quiesce unit tests (latch open-by-default + clone-shares-Arc). Correction 3 (no automatic data loss): removed the OUTER tokio::time::timeout(timeout, drain) in store_manager.rs that returned Vec::new() + logged "some slow writes will be lost" on elapse. The per-store inner flush_slow_writes(flush_budget) WAIT is retained (benign: bounds the wait not the data; residue is a strict subset of the Phase-2 at-risk set, drained unbounded by Phase 2). Making the inner wait infinite was tried and reverted (it wedged the existing post_pull_flush test, which relies on the inner budget bounding a never-completing synthetic in-flight write). Correction 4 (instrumentation, observability ONLY — never auto-acts): per-phase enter logs, a per-size-class progress poller (event="drain-progress": small_redis/large_tank remaining blobs+bytes + drain-rate deltas), a stall/livelock detector (event="drain-stalled" error! when remaining>0 with zero net drain over a 20s window), and an operator-facing gauge group shutdown/ (shutdown_remaining_blobs/bytes{class}, shutdown_phase, shutdown_stalled) so an operator can make an INFORMED force-kill decision. NO code auto-kills. Behavior changes (enumerated against production composition): - WorkerApiServer (prod, has worker_api entry): during SIGTERM Phase 0b..exit, handler-invoked backfill + pinned-mirror-pull solicitation is SUPPRESSED (early-return). Steady-state (not shutting down) is UNCHANGED — latch starts open, no per-tick cost added beyond one Relaxed atomic load. ShutdownPuller unchanged. - StoreManager::flush_slow_writes (prod, called twice in SIGTERM): Phase-1 no longer abandons via the outer timeout (data was already saved by Phase 2, so no end-user behavior change beyond truthful residual reporting + no false "all drained" log); adds per-size-class progress/stall logs + gauge writes during the drain (zero effect outside shutdown — gauges read 0). - worker variant (worker process, no worker_api entry): Phase 0b is a no-op (no WorkerApiServer registered); flush instrumentation runs identically. - TimeoutStopSec stays infinity; nothing auto-kills. No fsync, no per-RPC timeout, no new unbounded network buffer. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

allada force-pushed the better-metrics-api branch from d27b7fb to 49f2d60 Compare July 19, 2023 15:54

allada assigned chrisstaite-menlo Jul 19, 2023

allada commented Jul 19, 2023

View reviewed changes

chrisstaite-menlo approved these changes Jul 19, 2023

View reviewed changes

allada merged commit 87bd0e6 into master Jul 19, 2023

allada deleted the better-metrics-api branch July 19, 2023 16:16

Navigation Menu

Repository navigation

Improve prometheus utils api - #210

Improve prometheus utils api#210
allada merged 1 commit into
masterfrom
better-metrics-api

allada commented Jul 19, 2023 •

edited

Loading

Uh oh!

allada left a comment

Uh oh!

chrisstaite-menlo left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

allada commented Jul 19, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

allada left a comment

Choose a reason for hiding this comment

Uh oh!

chrisstaite-menlo left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

allada commented Jul 19, 2023 •

edited

Loading