Repository navigation
Give flaky memory store test more wiggle room - #448
Merged
Merged
Conversation
This is known to cause issues, especially for sanitized tests. Additional testing for MacOS showed that this needs to be at least 25%, so make it 30% to be on the safe side.
aaronmondal
commented
Dec 6, 2023
aaronmondal
left a comment
aaronmondal
left a comment
Contributor
Author
There was a problem hiding this comment.
Reviewable status: 0 of 1 files reviewed, all discussions resolved (waiting on @allada)
allada
approved these changes
Dec 6, 2023
allada
left a comment
allada
left a comment
Contributor
There was a problem hiding this comment.
Reviewed 1 of 1 files at r1, all commit messages.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on @aaronmondal)
MarcusSorealheis
pushed a commit
to MarcusSorealheis/nativelink
that referenced
this pull request
Nov 3, 2025
This is known to cause issues, especially for sanitized tests. Additional testing on MacOS showed that this needs to be at least 25%, so make it 30% to be on the safe side.
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>
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.
This is known to cause issues, especially for sanitized tests. Additional testing for MacOS showed that this needs to be at least 25%, so make it 30% to be on the safe side.
This change is