Repository navigation
Switch json5 to serde_json5 - #420
Merged
Merged
Conversation
aaronmondal
force-pushed
the
serde_json5
branch
from
November 27, 2023 15:07
cb5862d to
b8e99d7
Compare
allada
approved these changes
Nov 27, 2023
allada
left a comment
allada
left a comment
Contributor
There was a problem hiding this comment.
Reviewed 8 of 8 files at r1, all commit messages.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on @aaronmondal)
The latter is a more recently updated fork whose file layout is easier to process for automatic dependency resolvers.
aaronmondal
force-pushed
the
serde_json5
branch
from
November 27, 2023 15:29
b8e99d7 to
c30277b
Compare
TripleKai
pushed a commit
to TripleKai/turbo-cache
that referenced
this pull request
Nov 29, 2023
The latter is a more recently updated fork whose file layout is easier to process for automatic dependency resolvers.
MarcusSorealheis
pushed a commit
to MarcusSorealheis/nativelink
that referenced
this pull request
Nov 3, 2025
The latter is a more recently updated fork whose file layout is easier to process for automatic dependency resolvers.
rejuvenile
added a commit
to rejuvenile/nativelink
that referenced
this pull request
Aug 19, 2026
…tlier probe (TraceMachina#413 Option A) Per audit `.claude/audits/413-large-blob-cascade-design-20260511.md`: 17-min post-deploy window 21:55:38–22:13 PDT 2026-05-11 captured 11 cascade events / 9 distinct digests (1.2% of 900 chunked commits) at the prior cap=64. Size distribution: - 64-128 MiB: 2 events - 128-256 MiB: 7 events - 256-540 MiB: 2 events Cap=256 (× 1 MiB CHUNK_SIZE = 256 MiB per-blob, exactly equal to `MAX_CHUNKED_BLOB_SIZE`) structurally eliminates 9 of 11 events (everything ≤256 MiB). The 2 residual 256-540 MB events (RustcLink output blobs / macOS bundles) are tracked under TraceMachina#413 Option D (server-internal driver fan-out, design doc `.claude/audits/303-driver-fanout-design-20260512.md`) and will be revisited once the new `back_edge_ms` probe data validates the inferred ~25-50 MB/s per-blob drain ceiling. **Bottleneck localized via TraceMachina#413 audit:** `chunk_send_ms = 0` for ALL 14 logged steps in cascade trace — the producer never blocks. The cascade is **chunker → driver back-edge**, where per-blob driver pwrite (serialized by per-entry async mutex at `chunked_filesystem.rs:467`) drains slower than the chunker fills the per-blob mpsc. cap=256 widens the buffer; the underlying drain rate (TraceMachina#394) is unchanged. **Aggregate RSS bound unchanged.** Per-blob × concurrent-blobs is NOT the regulator. The §13.2 12-16 GiB worst-case server RSS bound is derived from the GLOBAL `ChunkBudget` (4 GiB / 4096 permits at `chunk_budget.rs:52`) and per-blob `PinBudget`. Bumping cap=256 shifts fan-out shape (max 16 saturated blobs at this cap; was 64 at cap=64; was 256 at cap=16) but not the aggregate ceiling. **New `back_edge_ms` outlier probe** at `chunked_driver.rs:987-999`: emits `warn!` when per-chunk back-edge drain exceeds 50 ms (40 ms = 1 MiB / 25 MB/s healthy ceiling + 25 % cushion). Survives in release builds (`Cargo.toml:101 release_max_level_info` retains `warn!`/`error!`; only `debug!`/`trace!` are compiled out — a `debug!`-level probe would have been dead code in production). Fires only on the success path; the timeout arm (`record_pwrite_timeout_and_maybe_warn`) and write-error arm emit their own `warn!`s without `back_edge_ms`. Acceptable for cap- tuning: failures already log loudly; outlier-on-success was the gap. Field name `back_edge_ms` (NOT `pwrite_ms`) reflects the actual scope: the wall-clock includes per-blob async-mutex acquire BEFORE spawn_blocking, so a same-digest mutex contention spike shows up as "async-mutex hold-time" in the >50 ms arm — operator-actionable, correctly attributed. **Updated:** - `PER_BLOB_MPSC_CAP` constant 64 → 256 + rewritten doc-comment (audit cite, back-edge bound rationale, MAX_CHUNKED_BLOB_SIZE natural ceiling, observability gap acknowledgment) - ChunkPin doc-comment math (64 MiB → 256 MiB) - Unit test renamed `per_blob_mpsc_cap_is_sixtyfour` → `per_blob_mpsc_cap_is_two_fifty_six`, updated body + pin doc - NEW `global_chunk_budget_remains_4_gib_bound_at_cap_256` composite- invariant test (CLAUDE.md "Admission/Eviction/Pin Composability" mandatory): asserts `PER_BLOB_MPSC_CAP × 16 == TOTAL_CHUNK_PERMITS` + saturates 16 × cap = 4096 permits + 17th `try_acquire_chunk()` returns `None`. Mutation-verified RED-fail with bespoke "composite invariant violated" message. - `chunked_client.rs` module-doc reference (256; bumped from 64 on 2026-05-12; was 16 before 2026-05-11; see TraceMachina#413 audit) - `chunked_write_handler.rs:591` stale `(64)` literal dropped from comment (pattern has gone stale 3 cap-bumps in a row; symbolic `PER_BLOB_MPSC_CAP` only) - Integration test `handler_per_blob_mpsc_full_returns_resource_exhausted_with_backpressure_signal` — `const N: usize = 32` → `const N: usize = PER_BLOB_MPSC_CAP + 1` (symbolic, with `wrapping_add` for byte-fill overflow safety) + imports + history-of-regression doc (16 → 64 → 256). Was already toothless at cap=64; cap=256 doubly so. Restored to actually exercise the rejection branch. Composite invariant (per CLAUDE.md): gate-active (per-blob mpsc admission cap) ⇒ (eviction-recovers-fast via chunk-completion permit-drop OR pin-prevents-double-admission via PinBudget). Cap bump shifts which corner fires first; the global ChunkBudget remains the dominant gate at 4 GiB. Reviewer cadre (4 + 2 follow-up): - perf-optimizer: APPROVE (after fix-up: 2 BLOCKERs closed cleanly, mutation-verified — debug→warn release-survival + integration test symbolic N + composite-invariant test with bespoke red-fail) - distributed-systems-reviewer: APPROVE-WITH-FINDINGS (no composite invariant violated; cap edge with MAX_CHUNKED_BLOB_SIZE validated; bitmap completeness exact-equal at 256 chunks; TraceMachina#345 mirror cap interaction confirmed unrelated for chunked path) - code-reviewer: APPROVE-WITH-FINDINGS (file-local `?digest` convention preserved over CLAUDE.md style guide; sibling-pwrite- probe audit clean — no other per-chunk pwrite sites) - assumption-auditor: SHIP (12/12 claims PASS post-fix-up; mutation re-verified the composite test red-fails with the bespoke message; 9-of-11 cascade-coverage doc citation corrected from "8 of 11") Build: `cargo check -p nativelink-store -p nativelink-service` clean. Test: 59 chunked + 20 chunked_write_handler tests pass; new `global_chunk_budget_remains_4_gib_bound_at_cap_256` passes; renamed constant-pin test passes; integration test now actually exercises the rejection branch at N=257. Follow-ups filed: - TraceMachina#413 Option D (driver fan-out, design ready, awaiting pwrite_ms data — see `.claude/audits/303-driver-fanout-design-20260512.md`) - TraceMachina#420 (chunked test /tmp tempdir leak — separate from this diff) 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
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.
The latter is a more recently updated fork whose file layout that is easier to process for automatic dependency resolvers.
This change is