Repository navigation
add a few depdencies. - #253
Closed
MarcusSorealheis wants to merge 1 commit into
Closed
MarcusSorealheis wants to merge 1 commit into
MarcusSorealheis wants to merge 1 commit into
Conversation
allada
reviewed
Sep 8, 2023
allada
left a comment
allada
left a comment
Contributor
There was a problem hiding this comment.
I was planning on pushing some changes to the dockerfile that account for this problem...
see:
https://reviewable.io/reviews/allada/turbo-cache/254
Reviewable status: 0 of 1 files reviewed, all discussions resolved
Collaborator
|
By default these should not be part of the build image as that does not create hermetic builds. It is sometimes necessary for some environments, but it should not be the standard. |
Member
Author
|
Yea, I agree completely. We were discussing the project, stepping through some issues. There are other hermeticity considerations as well. This PR is definitely close-able. |
rejuvenile
added a commit
to rejuvenile/nativelink
that referenced
this pull request
Aug 19, 2026
…mitigation) Today's TraceMachina#253 production OOM (19:45:39 PDT, anon-rss 83.6 GB / 80 GB cap) reproduces TraceMachina#197 (2026-04-27) exactly: log-spam-driven allocator pressure with mimalloc PAGE_FULL_RETAIN=8 pinning freed pages. Pre-OOM rate was ~1073 lines/sec sustained, dominated by diagnostic info! calls added during recent TraceMachina#171, TraceMachina#212, TraceMachina#218 instrumentation passes. Demote those calls to debug! so they remain available via RUST_LOG=debug but stop producing per-chunk allocator pressure in release builds. Demotions (info! -> debug!, structured fields and messages preserved): nativelink-service/src/bytestream_server.rs 1290 inner_read awaiting consume_fut (per-chunk, hottest) 1303 inner_read consume returned empty (EOF) (per-stream-end) 1318 inner_read consume returned chunk (per-chunk) nativelink-store/src/worker_proxy_store.rs 1884 WorkerProxyStore::get_part_sequential awaiting inner.get_part 1906 WorkerProxyStore::get_part_sequential inner.get_part returned nativelink-store/src/fast_slow_store.rs 1852 populate run_producer entry 3234 FastSlowStore::update background slow write spawned nativelink-scheduler/src/api_worker_scheduler.rs 2916 broadcast_blobs_in_stable_storage_chunked dispatching These are diagnostic logs added to investigate specific bugs (TraceMachina#171 inner-miss flow, populate join3 ordering, BIS chunked dispatch); the underlying bugs are now fixed but the info-level emission was never demoted, re-running the same OOM cascade after every deploy. The behavior path is unchanged - only the tracing level moves. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
rejuvenile
added a commit
to rejuvenile/nativelink
that referenced
this pull request
Aug 19, 2026
…broadcast, stable_notify, per-stream-end, slow-write-complete Phase 3 of the TraceMachina#255 OOM investigation (audit at .claude/audits/255-oom-investigation.md). Combined with the TraceMachina#253 predecessor demotions (commit 42e83f9), these eliminate the dominant remaining per-request log volume from the post-restart top list. Production rate evidence (5-min burst counts post-TraceMachina#253 restart): 11193 src/bin/nativelink.rs:677 stable_notify_fire (per CAS write) 6866 src/bin/nativelink.rs:703 BIS broadcasting drained digests 6866 src/bin/nativelink.rs:721 BIS chunked broadcast returned 4712 bytestream_server.rs:407 logging_read_stream poll yielded 3288 fast_slow_store.rs:3359 background slow write complete These are not error-coded; the audit (Phase 4) classifies them as pure per-request traces whose info-level retention only diluted the genuine warn/error signal during phantom-blob clusters and contributed to the allocator pressure that drives the OOM clock. The TraceMachina#253 commit already proved the demotion approach with the bytestream/worker_proxy hot-path sites; this is the remaining ~80% of the post-TraceMachina#253 volume. Pure macro rename (info! -> debug!); structured fields, message text, and target are unchanged so debug-level enablement reproduces the prior observability verbatim. Adds `debug` to the `tracing` import in src/bin/nativelink.rs.
rejuvenile
added a commit
to rejuvenile/nativelink
that referenced
this pull request
Aug 19, 2026
…a#262) During the 2026-05-05 RSS-leak investigation a Bazel fetch-storm drove RSS to a 27 GB plateau. Six info! log sites dominated the burst-time output (cumulatively ~2300 events / 30 s) and compounded with mimalloc page retention. Each is leftover diagnostic from TraceMachina#212/TraceMachina#218/TraceMachina#252 chunked-streaming work. Same shape as TraceMachina#255 / TraceMachina#186 / TraceMachina#253. Demoted sites: - bytestream_server.rs:385 "ByteStream::read: CAS read completed" 525 events / 30 s. Just bumped to info! today via e59b776 (LoggingReadStream consolidation). The 17 lines/sec sustained rate is too high for info! even though it carries throughput/duration — the CLAUDE.md rule "info! for transfer completions with throughput" was written for whole-RPC summaries, not per-stream-chunk reads. - bytestream_server.rs:1007 "ByteStream::inner_read entry" 525 events / 30 s. Per-call entry log; debug! is the right level. - bytestream_server.rs:1369 "inner_read get_part_fut resolved (still awaiting consume)" 520 events / 30 s. Per-event branch trace. Sibling at :1332 (`after consume_err`) left at info! — fires only on the rarer error path. - bytestream_server.rs:2010 "ByteStream::write: skipped, blob already exists" 377 events / 30 s. The WorkerProxyStore::has() write-skip optimization log; TraceMachina#133 already filed to demote it. - streaming_blob.rs:325 "streaming blob writer sent eof, notify_waiters firing" 293 events / 30 s. Per-blob lifecycle event; the wakeup propagates to readers without needing a separate log. - existence_cache_store.rs:147,154,156 eviction-callback trio (~258 events / 30 s each). Per-eviction mechanical follow-ups. Wedge-digest gates at :150 left at info! — they're explicitly filtered to a single digest under investigation. - fast_slow_store.rs:1939 "populate sending streaming_writer.send_error after head failure" 241 events / 30 s. Per-failed-RPC mechanical follow-up. The summary line at :1927 ("populate head_result Err") left at info! — one line per failure is the right cadence. streaming_blob.rs's `info` import removed (now unused). All four files already had `debug` in scope. Behavior-only change: no flow-control, concurrency, or storage semantics modified. Build verified --release --features quic,pprof,chunked_fast_slow. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
rejuvenile
added a commit
to rejuvenile/nativelink
that referenced
this pull request
Aug 19, 2026
Aggregates seven reviewer findings on the worker_proxy_store / SF
wire-up into one fix-up commit:
- code-reviewer MAJOR-1 (deploy-blocker): wire SingleflightMap into
WorkerProxyStore::publish via a `singleflight` group block, so the
metric annotations on SingleflightMapInner reach Prometheus.
- code-reviewer MAJOR-2 + dsr MAJOR-4 (deploy-blocker): demote
hot-path `info!("singleflight: dedup hit")` to `trace!`; promote
`info!("singleflight: bypass cap_exceeded")` to `warn!` (genuine
performance anomaly per CLAUDE.md tracing rules). Avoids TraceMachina#253/TraceMachina#255
log-rate OOM class.
- perf-optimizer MAJOR-1 + sibling-class to TraceMachina#171: add
`!writer.is_pipe_broken()` guard at the SF waiter NotFound match
arm. Without the guard, a future defensive WriteHalfGuard wrapper
above WPS would silently trip "Tried to send while stream is
closed" on every fall-back. New test
`wps_waiter_fallback_skips_when_writer_pipe_broken_guard` exercises
the contract; mutation REDs with bespoke message.
- testing-czar MAJOR-1: replace canonical Notify::notify_waiters
lost-wakeup race in test fake with `tokio::sync::watch::channel`.
Subscribe-before-poll; one watch flip releases all parked + future
callers. Removes 1000-iter sleep loops at three call sites.
- red-team MAJOR (deploy-blocker): correct doc-comment lie in
get_part_and_cache_inner that described choice β; restated to
describe choice α (all callers `drop(cache_handle)`).
- red-team finding: add
`total_waiter_fallback_to_direct: AtomicU64` counter on
SingleflightMapInner with `#[metric]` annotation. Operator can now
distinguish "SF engaging cleanly" from "SF degrading via
race-window fall-back". Existing fall-back test extended to
assert ≥ 1 increment; mutation REDs with bespoke message.
Plus MINOR follow-ups:
- testing-czar MINOR-1: tighten observed-fetch assertion to ≤ 2
(was N/4 == 4); the cache typically completes WHILE the leader
forwards.
- testing-czar MINOR-3: add `wps_oversized_blob_bypasses_sf` test;
mutation (remove size bound from should_cache) REDs with bespoke
message.
- code-reviewer MINOR-2: closure-capture comment corrected — `&Store`
is Copy, not borrowed.
- perf-optimizer MINOR-3: downgrade `was_leader_or_bypass` Ordering
from SeqCst to Release/Acquire (was overspec'd; only happens-before
needed).
- red-team finding: add Iteration history section to
docs/130-singleflight-peer-fetch-design.md documenting V1/V2/V3
prototypes and the choice-β rejection.
Test results: 21 pass (8 cdn_tee + 13 SF wire-up), 1 ignored
(stub kept for grep-ability), 0 failures. The slow-cache decoupling
gate `cdn_tee_slow_cache_abandons_does_not_block_bazel` still
passes — choice α preserved.
All three new/strengthened tests have mutation-step verified
bespoke RED messages.
Deferred to TraceMachina#182: code-reviewer MINOR-7 (test-only `pub` accessors
under cfg gate) — `nativelink-store` has no `test-helpers` feature
yet, and integration tests can't see `cfg(test)`.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
rejuvenile
added a commit
to rejuvenile/nativelink
that referenced
this pull request
Aug 19, 2026
…ug! (TraceMachina#250) Salvaged from the OOM-killed small-fix-batch agent. NL_REDIRECT is high-volume in production (every Bazel CAS Read for a blob the server hands off to a worker peer). Previously contributed to log-rate-driven OOMs (TraceMachina#186 false-alarm bursts, TraceMachina#197 phantom-blob bursts, TraceMachina#253/TraceMachina#255 backfill bursts). Redirect is normal protocol behavior, not a state transition or anomaly — debug! is the right level per CLAUDE.md tracing-levels rule. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
rejuvenile
added a commit
to rejuvenile/nativelink
that referenced
this pull request
Aug 19, 2026
…TraceMachina#449 measurement-first) Decompose the conflated `back_edge_ms` warn (chunked_driver.rs:992; production p99=1216 ms, max=20130 ms per re-mined journal at 16,774 samples) into the three sub-stages the existing warn identifies as candidate culprits: per-blob async-mutex acquire, spawn_blocking dispatch (submit → closure first-poll), and the pwrite(2) syscall loop. Same 50 ms threshold as the driver-side warn so operator alert volume baseline is unchanged; correlate the two warns by `digest` + `offset`. Per the TraceMachina#449 design review (red-team RECONSIDER-PREMISE; code-simplifier APPROVE-WITH-SIMPLIFICATION; both converged on this smaller shape), the 30-LoC inline split delivers the diagnostic decomposition needed to attribute production tail latency to a specific sub-stage BEFORE committing to the 500-LoC full instrumentation (new modules, 24-variant SpawnSite enum, macro injection across ~25 spawn sites). Same approach informs the TraceMachina#448 chmod-as-publish decision: assumption-auditor falsified the original "rename dominates" framing (back_edge_ms max = 20130 ms exceeds the longest rename = 6646 ms; pwrite/mutex independently produces the long tail), so both designs are gated on this measurement landing first. Silence-is-diagnostic: `total_inner_us` excludes four spans the driver's `back_edge_ms` includes — `open_or_create_partial.await`, `try_clone()`, JoinHandle re-poll between closure return and `.await` resuming, and `drop(file_guard)`. When the driver warn fires WITHOUT the decomposition warn, the cost lives in one of those four; the absence IS the operator-actionable signal pointing at FS overhead, dup syscall contention, scheduler resume latency, or mutex teardown. Cost: 3 × `Instant::now()` (~30 ns) + 3 × `as_micros()` (~10 ns) per chunk. At 2000 chunks/sec peak = 0.006 % CPU. Composite warn rate at the slow tail is ~40 lines/sec, 27× under the 1073 lines/sec OOM ceiling from incident TraceMachina#253. Zero heap allocations on the success path (closure returns the timings via the existing Result tuple, no Arc<AtomicU64> per chunk). Reviewers (focused 4-cadre): - code-reviewer: APPROVE (2 NITs only) - perf-optimizer: APPROVE (cost claims verified; concurrency correctness clean; sibling audit on `entry.file.lock()` shows one call site) - testing-czar: NEEDS-MORE-COVERAGE non-blocking (follow-up: inline test with two same-digest writers + delay asserting decomposition warn fields; mutation = comment out the `if total_inner_us > 50_000` guard) - assumption-auditor: SHIP with comment tightening applied (production p99 re-verified within sampling noise: 1211 ms measured vs 1216 ms claimed; all line citations verified; CLAUDE.md hard rules satisfied) All 61 chunked module tests + 5 chunked_filesystem_serialization integration tests pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
rejuvenile
added a commit
to rejuvenile/nativelink
that referenced
this pull request
Aug 19, 2026
Adds info! emissions at each writer-side registration / RPC entry so journal scans can attribute a chunked-write trace (e.g. the 21:14:28 UTC 2026-05-16 event for b0eb2a47...95641600) to the specific writer path that produced it. Pure observability change; no behavior modified. Sites instrumented (`writer_path` field on every emit): - `bazel_facing_v1_dispatcher` — `chunked_write_handler.rs:3933` BazelChunkedDispatcher::dispatch Owner registers in FSS-level chunked_in_flight_digests - `server_v2_session` — `chunked_write_handler_v2.rs:253` v2 session registers in FSS-level chunked_in_flight_digests - `server_v1_rpc` — `chunked_write_handler.rs:875` WriteChunked RPC entry (worker→server v1) - `server_v2_rpc` — `chunked_write_handler_v2.rs:156` WriteChunkedV2 RPC entry (worker→server v2) - `server_v1_handler_local` — `chunked_write_handler.rs:1133` write_chunked_inner inserts into HANDLER-LOCAL in_flight (NOT chunked_in_flight_digests) - `shared_dispatch_chunks_to_driver` — `chunked_write_handler.rs:2856` shared dispatch_chunks_to_driver handler-local insert - `worker_chunked_client` — `chunked_client.rs:439` write_chunked_stream entry (worker-side; once per blob) - `worker_dispatch_v1` — `chunked_client.rs:201` WorkerApiWriteChunkedDispatcher::dispatch (once per attempt) - `worker_dispatch_v2` — `chunked_client.rs:276` WorkerApiWriteChunkedV2Dispatcher::dispatch (once per attempt) InFlightChunkedGuard gains `new_with_caller(...,caller: &'static str)`; plain `new(...)` retains the pre-existing signature for tests (defaults caller to `"unspecified"`). Drop emits a paired removal log with `outcome` (drop / disarm / reaper_decrement) + `duration_ms`. KEY FINDING (to be confirmed by tests + journal scan): `write_chunked_inner` (the v1 worker→server WriteChunked SERVER handler) does NOT register in the FSS-level `chunked_in_flight_digests` set. It registers ONLY in the handler-local `in_flight` map. Therefore Option C reader-waits (which key off `chunked_in_flight_digests`) are a STRUCTURAL NO-OP for the v1 worker→server WriteChunked wire shape. Confirmed by absence of `chunked_in_flight_digests` references inside `write_chunked_inner` at `:875-1400`. Log-volume budget: per blob, ~6-10 lines across all sites (entry + registration + handler-local + removal). At production cadence (~1142 chunked admissions/h = ~0.3/sec) total volume is <10 lines/sec, well below the 1073-lines/sec OOM threshold (TraceMachina#253). Tests still to add (next WIP commit). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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
3 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.
From last night's conversation. Seems like these two would be important.
This change is