Repository navigation
Integration test should work for Mac OS. - #334
MarcusSorealheis merged 1 commit into
Conversation
allada
left a comment
There was a problem hiding this comment.
Reviewed 2 of 2 files at r1, all commit messages.
Reviewable status: all files reviewed, 12 unresolved discussions (waiting on @MarcusSorealheis)
run_integration_tests.sh line 19 at r1 (raw file):
if [[ $EUID -eq 0 ]]; then echo "This script shoudl not be run as root."
Misspelled.
run_integration_tests.sh line 49 at r1 (raw file):
done echo "What operating system OSTYPE: $OSTYPE"
This ENV is not set on all OSs.
run_integration_tests.sh line 59 at r1 (raw file):
TEST_PATTERNS=("*") fi
Needless new line.
run_integration_tests.sh line 73 at r1 (raw file):
sudo docker-compose rm --stop -f echo "What operating system OSTYPE: $OSTYPE"
nit: ditto
run_integration_tests.sh line 77 at r1 (raw file):
export TMPDIR=$HOME/.cache/turbo-cache/ sudo mkdir -p "$TMPDIR"
don't mkdir with sudo, it causes issues.
run_integration_tests.sh line 79 at r1 (raw file):
sudo mkdir -p "$TMPDIR" if [[ "$OSTYPE" == "linux-gnu"* ]]; then
use ${OSTYPE:-linux-gnu}
run_integration_tests.sh line 82 at r1 (raw file):
export CACHE_DIR=$(mktemp -d --tmpdir="$TMPDIR" --suffix="-turbo-cache-integration-test") elif [[ "$OSTYPE" == "darwin"* ]]; then # Create a temporary directory using mktemp with a random template, then add a suffix
nit: end in a period.
run_integration_tests.sh line 83 at r1 (raw file):
elif [[ "$OSTYPE" == "darwin"* ]]; then # Create a temporary directory using mktemp with a random template, then add a suffix export CACHE_DIR=$(mktemp -d "${TMPDIR}turbo-cache-integration-test")
nit: add slash between ${TMPDIR} and the constant.
run_integration_tests.sh line 84 at r1 (raw file):
# Create a temporary directory using mktemp with a random template, then add a suffix export CACHE_DIR=$(mktemp -d "${TMPDIR}turbo-cache-integration-test") echo "CACHE_DIR for macOS is: $CACHE_DIR" # <-- Added print statement here
nit: remove this.
run_integration_tests.sh line 85 at r1 (raw file):
export CACHE_DIR=$(mktemp -d "${TMPDIR}turbo-cache-integration-test") echo "CACHE_DIR for macOS is: $CACHE_DIR" # <-- Added print statement here # exit 1
nit: remove this.
run_integration_tests.sh line 88 at r1 (raw file):
else # Unknown OS echo "Unknown operating system. Exiting."
nit: Lets instead invert this and assume linux if it's not mac.
run_integration_tests.sh line 106 at r1 (raw file):
# Cleanup. echo "Cleaning up cache directories TURBOC_CACHE_DIR: $TURBO_CACHE_DIR"
nit: Needless spaces.
c6980b0 to
6b37ecf
Compare
MarcusSorealheis
left a comment
There was a problem hiding this comment.
Reviewable status: 1 of 2 files reviewed, 10 unresolved discussions (waiting on @allada)
run_integration_tests.sh line 73 at r1 (raw file):
Previously, allada (Nathan (Blaise) Bruer) wrote…
nit: ditto
Removed.
run_integration_tests.sh line 77 at r1 (raw file):
Previously, allada (Nathan (Blaise) Bruer) wrote…
don't mkdir with sudo, it causes issues.
Done
run_integration_tests.sh line 79 at r1 (raw file):
Previously, allada (Nathan (Blaise) Bruer) wrote…
use
${OSTYPE:-linux-gnu}
Done.
run_integration_tests.sh line 83 at r1 (raw file):
Previously, allada (Nathan (Blaise) Bruer) wrote…
nit: add slash between
${TMPDIR}and the constant.
$TEMP_DIR has a trailing slash.
run_integration_tests.sh line 88 at r1 (raw file):
Previously, allada (Nathan (Blaise) Bruer) wrote…
nit: Lets instead invert this and assume linux if it's not mac.
what about Windows? Some customers use Windows.
MarcusSorealheis
left a comment
There was a problem hiding this comment.
Reviewable status: 1 of 2 files reviewed, 10 unresolved discussions (waiting on @allada)
run_integration_tests.sh line 79 at r1 (raw file):
Previously, MarcusSorealheis (Marcus Eagan) wrote…
Done.
looks like it should be linux-gnu as I had before
MarcusSorealheis
left a comment
There was a problem hiding this comment.
Reviewable status: 1 of 2 files reviewed, 10 unresolved discussions (waiting on @allada)
run_integration_tests.sh line 79 at r1 (raw file):
Previously, MarcusSorealheis (Marcus Eagan) wrote…
looks like it should be linux-gnu as I had before
https://github.com/TraceMachina/turbo-cache/actions/runs/6598502029/job/17926538604
MarcusSorealheis
left a comment
There was a problem hiding this comment.
Reviewable status: 1 of 2 files reviewed, 10 unresolved discussions (waiting on @allada)
run_integration_tests.sh line 79 at r1 (raw file):
Previously, MarcusSorealheis (Marcus Eagan) wrote…
https://github.com/TraceMachina/turbo-cache/actions/runs/6598502029/job/17926538604
Should I write a regex for either or?
5418163 to
105f34a
Compare
|
@allada ball in your court but curious about your thoughts on |
105f34a to
68f6947
Compare
MarcusSorealheis
left a comment
There was a problem hiding this comment.
My overall opinion is that this works but I do not want to put too much effort into it given it will go away in the near term.
Reviewable status: 1 of 2 files reviewed, 9 unresolved discussions (waiting on @allada)
run_integration_tests.sh line 19 at r1 (raw file):
Previously, allada (Nathan (Blaise) Bruer) wrote…
Misspelled.
Done.
run_integration_tests.sh line 59 at r1 (raw file):
Previously, allada (Nathan (Blaise) Bruer) wrote…
Needless new line.
Done.
allada
left a comment
There was a problem hiding this comment.
Reviewed all commit messages.
Reviewable status: 1 of 2 files reviewed, 2 unresolved discussions (waiting on @MarcusSorealheis)
run_integration_tests.sh line 79 at r1 (raw file):
Previously, MarcusSorealheis (Marcus Eagan) wrote…
Should I write a regex for either or?
The command I gave you just defaults OSTYPE to linux-gnu if it is not set.
It should look like this:
if [[ "${OSTYPE:-linux-gnu}" == "linux-gnu"* ]]; thenrun_integration_tests.sh line 88 at r1 (raw file):
Previously, MarcusSorealheis (Marcus Eagan) wrote…
what about Windows? Some customers use Windows.
This test doesn't work on windows anyway.
68f6947 to
051f999
Compare
MarcusSorealheis
left a comment
There was a problem hiding this comment.
Reviewable status: 1 of 2 files reviewed, 1 unresolved discussion (waiting on @allada)
run_integration_tests.sh line 49 at r1 (raw file):
Previously, allada (Nathan (Blaise) Bruer) wrote…
This ENV is not set on all OSs.
ahh. I see what you meant. This script would fail on those platforms.
run_integration_tests.sh line 79 at r1 (raw file):
Previously, allada (Nathan (Blaise) Bruer) wrote…
The command I gave you just defaults
OSTYPEtolinux-gnuif it is not set.It should look like this:
if [[ "${OSTYPE:-linux-gnu}" == "linux-gnu"* ]]; then
that wasn't clear. I get default parameter, though. I thought you only specify the string.
MarcusSorealheis
left a comment
There was a problem hiding this comment.
Reviewable status: 1 of 2 files reviewed, 1 unresolved discussion (waiting on @allada)
run_integration_tests.sh line 79 at r1 (raw file):
Previously, MarcusSorealheis (Marcus Eagan) wrote…
that wasn't clear. I get default parameter, though. I thought you only specify the string.
I think this is fine now
allada
left a comment
There was a problem hiding this comment.
Reviewed 1 of 1 files at r7.
Reviewable status: all files reviewed (commit messages unreviewed), 2 unresolved discussions (waiting on @MarcusSorealheis)
run_integration_tests.sh line 79 at r7 (raw file):
export CACHE_DIR=$(mktemp -d "${TMPDIR}turbo-cache-integration-test") else # Unknown OS
nit: needless comment.
run_integration_tests.sh line 100 at r7 (raw file):
echo "Checking for existince of the TURBO_CACHE_DIR" if [ -d "$TURBO_CACHE_DIR" ]; then sudo find "$TURBO_CACHE_DIR" -delete # add for linux
nit: Needless comment.
051f999 to
56afc81
Compare
allada
left a comment
There was a problem hiding this comment.
Reviewable status: 1 of 2 files reviewed, 1 unresolved discussion (waiting on @MarcusSorealheis)
run_integration_tests.sh line 79 at r8 (raw file):
else echo "Unable to detect operating system. Assuming the Linux/WSL syntax will work." export CACHE_DIR=$(mktemp -d --tmpdir="$TMPDIR" --suffix="-turbo-cache-integration-test")
Since we are doing the same thing in:
if [[ "${OSTYPE:-linux-gnu}" == "linux-gnu"* ]]; then
Can we instead just check darwin and then assume linux otherwise and get rid of this else clause?
MarcusSorealheis
left a comment
There was a problem hiding this comment.
Reviewable status: 1 of 2 files reviewed, 1 unresolved discussion (waiting on @allada)
run_integration_tests.sh line 79 at r8 (raw file):
Previously, allada (Nathan (Blaise) Bruer) wrote…
Since we are doing the same thing in:
if [[ "${OSTYPE:-linux-gnu}" == "linux-gnu"* ]]; thenCan we instead just check
darwinand then assume linux otherwise and get rid of this else clause?
sure
56afc81 to
1faf0ca
Compare
allada
left a comment
There was a problem hiding this comment.
Reviewable status: 1 of 2 files reviewed, all discussions resolved (waiting on @MarcusSorealheis)
…ow writes (TraceMachina#334 fix-A/B) Two fixes for the production cascade observed today (2026-05-08, 67 GB RSS at 1h15m, 2743 errors/min, 723 unique 16KB-1MB digests). **Fix A — typed BackpressureSignal preservation.** When the fast tier (MemoryStore) early-rejects with the wire-stable ResourceExhausted+BackpressureSignal::MemoryStoreAtCapacity, it drops fast_rx, the data-stream future's send fails with channel-closed, the closure wraps that as generic Code::Internal, and the pre-existing match returned the Internal BEFORE checking fast_res — masking the typed signal. Bazel does not honor backoff hints on Internal, so retries are immediate, in_flight grows, MemoryStore at cap rejects more, cascade. Fix: after the join!, check fast_res for Code::ResourceExhausted carrying a BackpressureSignal detail; if present, surface fast_res to the caller before evaluating data_res. Strict heuristic matches Code AND detail (option a in the task brief) — bare ResourceExhausted from a future fast-tier store kind does not match. Sibling guard applied to the chunked-dispatcher path (latent today; fires if MemoryStore ever early-rejects on that path). **Fix B — aggregate slow-write byte cap.** in_flight_slow_writes pinned every payload until the background slow-write terminated; under slow-tier wedge that grew unbounded. New FastSlowSpec.slow_writes_in_flight_max_bytes (default 8 GiB) caps it via a typed BackpressureSignal::SlowWritesAtCapacity rejection (new proto variant in the same wire-stable enum). Counter increments after insert under the in_flight_slow_writes mutex, decrements before remove under the same mutex; check is best-effort snapshot before the spawn. Zero = uncapped (historic behavior bit-identical for opt-out callers). Tests in fast_slow_store_334_backpressure_preservation_test.rs cover under-action + over-action + interaction paths per CLAUDE.md asymmetric contract coverage. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…-precondition race Without `tokio::task::yield_now()` in the test producer, send_fut completes synchronously before update_fut polls — data_stream_fut wins the send-vs-fast-rx-drop race, fast_res Err arrives via the regular `fast_res?` path that already preserves the typed signal, and the test PASSES even when Fix A is disabled (mutation step gives a false green). The yield reliably suspends the producer so the inner FastSlowStore poll order matches production: data_stream_fut hits `reader.recv().await` (suspends, no data yet), fast_store_fut polls and runs MemoryStore's ExactSize early-reject (returns Err, drops fast_rx), data_stream_fut wakes when send_fut's first send arrives, calls (*fast_guard).send().await, fails with channel-closed, returns the wrapped Code::Internal error. Now mutation step Fix A red-fails with the canonical production error message. Also remove the stale MUTATION STEP comment from fast_slow_store.rs that survived a mid-development edit cycle. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…a#334 fix-AB fixup) Reviewer findings on the Fix A+B bundle: 1. **perf BLOCKER + security MEDIUM**: `get_part`'s defensive size-mismatch guard at `:4856-4861` removed from `in_flight_slow_writes` without decrementing `in_flight_slow_writes_bytes`. Each activation permanently shrinks the effective cap; over a long-lived process the cap drifts to 0 and rejects every admission with `SlowWritesAtCapacity`. Mirror the canonical `:4218-4222` decrement pattern. 2. **dsr MAJOR + red-team blind spot**: cap-rejection at `update:4023` and `update_oneshot:4330` previously left the digest invisible to `failed_slow_writes` drain, so the server-side `failed_writes_drain` → `UploadMissingBlobs` recovery loop could not pick it up. If the upstream client retry budget exhausted, bytes aged out of MemoryStore (PIN_TIMEOUT = 120s) with no recovery. Now mirrors the chunked dispatcher's symmetric error site at `:1229-1232`: insert into `failed_slow_writes` + re-pin BEFORE returning the typed error. 3. **perf MAJOR + code-reviewer H2**: doc-comments at `:529-538` and `:4053-4060` (post-Fix-B) misdescribed the lock/counter ordering. The actual semantics are eventually-consistent: insert under lock, fetch_add OUTSIDE lock, remove under lock, fetch_sub under lock. Rewrite to accurately describe the asymmetry and the `tokio::spawn`-ordering invariant that prevents underflow. 4. **code-reviewer H3**: `slow_writes_in_flight_max_bytes` was an `AtomicU64` but is set once at construction with no setter. Convert to plain `u64`.
…TraceMachina#334 fix-AB fixup) Reviewer findings: 1. **code-reviewer H1 + dsr M2 + testing-czar**: new variant `BackpressureSignal::SlowWritesAtCapacity` was not added to the existing classifier-iteration tests (`looks_like_dead_channel_respects_backpressure_signal`, `new_fixup_variants_roundtrip_and_classify`). A future change to either helper could silently regress for the new variant. Add to both iteration lists. 2. **red-team blind spot**: `SLOW_WRITES_AT_CAPACITY_RETRY_MS = 100` had no rationale comment for the 4× difference vs `MEMORY_STORE_BACKPRESSURE_RETRY_MS = 25`. MemoryStore eviction resolves on single-digit-ms timescale; slow-tier ack on hundreds-of-ms. Document. 3. **red-team TraceMachina#3 + dsr M4**: 8 GiB server cap default needed an explicit per-host rationale and worst-case overshoot bound. Document both. 4. **code-reviewer C1 + dsr M4**: worker `slow_writes_in_flight_max_bytes: 0` lacked any explanatory comment — looked like a copy-paste oversight. Per parent decision: keep `: 0` on worker (worker's slow tier is GrpcStore→server, bounded by upstream h2 windowing + server-side cap). The 2026-05-08 OOM was on buildcache, not on workers. Add explicit comment justifying the choice at both worker FSS construction sites.
… fixup) Reviewer findings (code-reviewer C2): the new `slow_writes_in_flight_max_bytes: 0` field was inserted at 12-space indent across ~37 test files where surrounding fields use 8 spaces. Run rustfmt on every file in the branch's diff to fix; the bulk of the line churn is rustfmt's reformatting of other pre-existing non-conformant style in the same files, which CI's `cargo fmt` gate would have rejected anyway.
…raceMachina#334 fix-AB fixup) Reviewer findings: 1. **testing-czar BLOCK-1 + dsr (Fix A over-action)**: existing test family covered Fix A under-action only. Added `fix_a_does_not_over_match_bare_resource_exhausted` — a new `BareResourceExhaustedFastStore` returns `Code::ResourceExhausted` with NO `BackpressureSignal` detail; the Fix A heuristic (`Code::ResourceExhausted && error_has_backpressure_signal(e)`) MUST NOT match. Mutation-step: dropping the `error_has_backpressure_signal` clause from the predicate red-fails the test with the bespoke "Fix A predicate over-matched" message. 2. **testing-czar BLOCK-2 (Fix B failure-path counter decrement)**: covers the failure arm at `fast_slow_store.rs:4196-4221`. Added a `set_fail_after_release` flag to `GatedSlowStore` so the spawned slow-write returns `Err(Internal)` after the gate releases. Asserts `in_flight_slow_write_bytes()` drains to zero on failure (not just success). Mutation-step: commenting out the `fetch_sub` in the failure-arm closure red-fails with the bespoke "counter leaked on failure path — admission cap will permanently reject after first slow-write failure" message. 3. **testing-czar BLOCK-3 (`update_oneshot` Fix B sibling)**: parallel shape to `fix_b_slow_writes_in_flight_byte_cap_emits_typed_signal` but exercises `Store::update_oneshot(...)` directly. Asserts the cap-check at `:4359` fires + counter mutates correctly at `:4338-4339`/`:4459-4471`. Catches a regression in the oneshot path that the streaming `update` test would miss. 4. **testing-czar BLOCK-4 (production composition wrap)**: changed `fix_a_small_blob_preserves_backpressure_signal_through_fast_slow_store` to wrap `FastSlowStore` in `VerifyStore` and `ExistenceCacheStore` (matching the production `cas_STORE` chain). The bug being fixed is cross-component signal preservation; testing only at the FSS unit boundary would miss a regression where a wrapper layer demotes the typed signal to generic Internal across its own join boundary. 5. **testing-czar MAJOR-1 (mutation-step bespoke message reachable)**: reordered the asserts in the Fix A under-action test so the bespoke `assert!` ("typed BackpressureSignal must be preserved when MemoryStore early-rejects") fires BEFORE the helper's generic `assert_eq!(err.code, ResourceExhausted, ...)` panic. Without this, the documented mutation-step red-fail message would never appear in CI output. All 7 tests in the file pass + the 2 classifier tests (`looks_like_dead_channel_respects_backpressure_signal`, `new_fixup_variants_roundtrip_and_classify`) pass with the new `SlowWritesAtCapacity` variant.
…yStore pin overrides (TraceMachina#334 Fix C) Server-side cas_FAST_SLOW_STORE writes go through MemoryStore (48 GB fast tier). Pre-fix MemoryStore::pin_delegation = Leaf made pin_digests a documented no-op via the trait default's Leaf arm; the post-write pin held to bridge the BlobsInStableStorage ack window silently evaporated, so an LRU eviction during the ack window broke the >=2-replica durability invariant. Changes: - Add unpin_digests trait method (mirrors pin_digests routing via the PinDelegation enum so existing wrapper chain composes for free). - Override pin_digests / pin_digests_with_results / unpin_digests on MemoryStore (delegate to MokaEvictingMap pin_keys / unpin_key, mirroring FilesystemStore). - Override unpin_digests on FilesystemStore as the trait-method form of the existing public unpin_digest (singular) so the fan-out from FastSlowStore reaches the slow tier too. - Replace MemoryStore::pin_delegation doc-comment ("no-op") with a Fix-C-aware comment explaining why Leaf + overrides is the correct semantics ("no inner-store delegation needed", not "no pinning"). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…a#334 Fix C) After broadcast_blobs_in_stable_storage_chunked tells workers a CAS digest is durably mirrored, release the server-side fast-tier pin acquired by FastSlowStore::update at write time. Without this unpin every write accumulates a permanent pin entry in MemoryStore's 25%-of-cap pin budget; once the budget fills (~12 GB at the 48 GB production cap) MokaEvictingMap::pin_keys warn-and-skips every subsequent pin and silently re-opens the durability gap Fix C closes. Capture per-store CAS drains alongside the existing flat broadcast list so unpin can be scoped to the originating store (avoids racing with a pin acquired by a fresh write of the same digest on a different store). AC stores intentionally not unpinned: AC pin lives in ac_pin_registry, swept separately above by remove_digests_for_endpoint_batch. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…x C pin/unpin
Two tests covering both directions of the pin contract on
MemoryStore (asymmetric coverage per CLAUDE.md):
- memory_store_pin_survives_eviction_pressure (under-action):
asserts pin_digests on the cas_FAST_SLOW_STORE chain
(ExistenceCacheStore → VerifyStore → FastSlowStore { fast: Memory,
slow: Filesystem }) actually keeps blob A alive across sibling-write
eviction pressure that drives MemoryStore past cap. Pre-fix: trait
default's Leaf arm no-op'd the pin and A was evicted.
- memory_store_unpin_releases_for_eviction (over-action):
after unpin_digests on the same chain, the pin must be released so
A becomes the LRU victim. Pre-fix: no unpin_digests trait method
existed so the BIS broadcast loop had no path to release pins; an
accumulating pin set would fill the 25%-of-cap pin budget and
silently re-open the durability gap Fix C closes.
Both tests use 5 s tokio::time::timeout deadlock detectors and
bespoke .expect() messages naming the contract violation. Mutation
verified: removing pin_digests body red-fails the under-action with
"MemoryStore-pin no-op shipped" message; removing unpin_digests
body red-fails the over-action with "pin leaks past BIS-ack" message.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…raceMachina#334 Fix C sibling) RefStore overrides pin_digests to forward to the resolved target store; symmetric override for unpin_digests so the BIS broadcast loop can release pins through any RefStore-mediated chain. Without this the trait default's Leaf arm would be a no-op for RefStore and the unpin would never reach the actual pinning leaf. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…dsr MAJOR-1)
Per dsr's MAJOR-1 review of Fix C: the original BIS-broadcast unpin
loop only released CAS fast-tier pins. The doc-comment claimed "AC
backend is FilesystemStore-only in production" which is factually
wrong against the production config — `AC_BACKEND_CACHED` is
`FastSlowStore { fast: MemoryStore(4 GB), slow: ref(REDIS_AC_STORE) }`.
Every AC write pins on the AC MemoryStore at fast_slow_store.rs:4103,
but Fix C never unpinned, leaking the AC pin budget symmetrically to
the CAS leak Fix C closed. After ~1 GB of AC writes (4 GB cap × 25%
pin cap) `pin_keys: pin cap exceeded` would flood and AC pins would
silently fail, re-opening the same durability gap on AC.
Symmetric fix: build per-AC-store drains alongside the existing
per-CAS-store drains, and call `unpin_digests` on each originating
AC store post-broadcast. Routes through the same `pin_delegation`
chain (AcProxyStore → CompletenessCheckingStore → FastSlowStore)
that `pin_digests` traversed at write time.
Doc-comment updated to reflect the correct AC composition.
…x C AC unpin
Symmetric to the existing memory_store_pin_durability_test for CAS:
exercises the AC chain (AcProxyStore → CompletenessCheckingStore →
FastSlowStore { fast: MemoryStore, slow: MemoryStore }) for both
under-action (pin survives eviction pressure) and over-action (unpin
releases for eviction) contracts.
Mutation step verified: setting AcProxyStore::pin_delegation to Leaf
red-fails ac_unpin_releases_for_eviction with the bespoke
"AC pin leaks past BIS-ack — entry A survived sibling-pressure
eviction even after unpin_digests through the AC chain..." message.
This proves the test guards the new AC unpin loop's chain-routing
contract end-to-end.
The under-action test (ac_pin_survives_eviction_pressure) covers
the symmetric AC-side of the existing CAS pin override; this test
will additionally guard against any future regression in
AcProxyStore / CompletenessCheckingStore pin_delegation forwarding.
Fix C eviction extension) With MemoryStore::emit_backpressure_enabled=true (production default for cas_FAST_SLOW_STORE.fast), check_backpressure_gate refuses inserts BEFORE moka can do its natural admission-driven LRU eviction. Once the cache fills to cap, NOTHING evicts (no insert pressure, no TTL, no explicit removes), turning the cache into write-once-evict-never until process restart. This new helper restores liveness by giving the gate a way to free unpinned room before emitting backpressure. Pinned entries (the load- bearing ones during the BIS ack window) live OUTSIDE moka's cache in the `pinned` DashMap, so they're protected automatically — this helper can only touch evictable entries. Order caveat: moka exposes only arbitrary-order iter() (no public LRU walker — access_order_q_node is pub(crate)). Aspirational "LRU" in the method name; arbitrary order is acceptable because pin protection is independent of iteration order, and unpinned entries are by definition evictable. Documented in the helper rustdoc.
…ix C eviction extension Three tests covering the asymmetric contract for the gate's new active-eviction path before emitting the typed BackpressureSignal: 1. Under-action — eviction frees room for unpinned entries: cap full of 4 unpinned entries, gate evicts one to admit a new write. Mutation step verified: commenting out evict_unpinned_lru_bytes in check_backpressure_gate red-fails this test with the bespoke "Fix C eviction extension regressed — gate refused admission with evictable unpinned entries available" message. 2. Over-action — pinned entries are protected: cap saturated by a pinned entry, gate finds nothing evictable, emits typed signal, pinned entry survives. Mutation step verified: making evict_unpinned_lru_bytes also drain the `pinned` DashMap red-fails this test on either the pin-survival assert or the expect_err assert with the bespoke "must NEVER touch keys present in `pinned`" message. 3. Mixed — partial eviction frees enough room: cap = 1 pinned + 3 unpinned, gate evicts ≥1 unpinned to admit a new write while the pinned entry survives. Same mutation as test 2 catches durability regressions; same mutation as test 1 catches under-eviction. Each assertion runs under a tokio::time::timeout(5s) deadlock detector (eviction loop is sub-millisecond healthy) so a lock-contention regression manifests as a red test rather than a hung CI runner. Cap = 4 KiB, entry = 1 KiB: the smallest cap where pin_cap (25% × cap) fits a real pinned entry AND a triggerable over-cap incoming write.
…raceMachina#334 bundle fixup TraceMachina#1) Branch 2 (Fix C + AC + eviction extension) authored two test files — `memory_store_pin_durability_test.rs` and `ac_pin_durability_test.rs` — without the new `slow_writes_in_flight_max_bytes` field on `FastSlowSpec`. After the cherry-pick merge of Fix A+B (which adds the field with `#[serde(deny_unknown_fields)]`), these tests fail to compile with E0063: missing field. Both literals add `slow_writes_in_flight_max_bytes: 0,` (= uncapped, the historic behavior bit-identical for opt-out callers). The eviction- extension test file `memory_store_backpressure_eviction_test.rs` was checked workspace-wide for FastSlowSpec literals; it has none, so no change required there. Also surfaced and fixed: pre-existing `chunked_fast_slow` gate hole on `writer_termination_245_drop_fallback_test`. The test calls `set_chunked_size_threshold_for_test` (gated `#[cfg(all(feature = "chunked_fast_slow", any(test, feature = "test-utils")))]`) and `set_bazel_chunked_dispatcher` (gated on `chunked_fast_slow`). Without the matching `[[test]] required-features = ["chunked_fast_slow", "test-utils"]` declaration, the test silently passes when the feature is off and fails to compile when the feature is on. Mirrors the existing entry for `fast_slow_str_key_skips_chunked_dispatch_test`. Compile verified with both `cargo check -p nativelink-store --tests` and `--features chunked_fast_slow`. Workspace-level `cargo check --workspace --tests --features chunked_fast_slow` clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…afety mechanism (TraceMachina#334 bundle fixup TraceMachina#2) 6 reviewers (testing-czar + dsr + code-reviewer + perf + simplifier + red-team) converged on the BIS-loop unpin doc-comment at `src/bin/nativelink.rs:1037-1042`. The original comment claimed "per- store accounting (drains_per_store) keeps the unpin scoped to the BIS- acked write so we don't race a fresh pin on the same digest." This is incorrect: `MokaEvictingMap::unpin_key` (`moka_evicting_map.rs:903-911`) is wholesale `pinned.remove(key)` with NO per-write epoch tracking. A concurrent re-pin between drain and unpin DOES lose its pin. The race IS benign — but for a different reason than the original comment claimed: - BIS-ack implies the SLOW TIER (FilesystemStore for CAS, Redis for AC) already has the bytes. The fast-tier pin is no longer load- bearing past this point. - A concurrent re-pin between drain and unpin loses its pin, but the bytes are recoverable via slow-tier. - For CAS specifically, content-addressing means a re-uploaded same-key blob has identical bytes; the slow tier already has them. Rewrites both doc-comments (server BIS loop + MemoryStore::unpin_digests) to name the actual mechanism. Adds an explicit warning that future callers needing different per-write semantics will need epoch tracking in `MokaEvictingMap` — do NOT assume safety in arbitrary contexts. No code changes; doc-only. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…es (TraceMachina#334 bundle fixup TraceMachina#3) Code-reviewer MA-3: production deploys with `chunked_fast_slow` enabled, but `.github/workflows/native-cargo.yaml` never runs the chunked_fast_slow-gated test bins. Three tests in this bundle (and ten others pre-existing) silently skip in CI today. Adds a CI step `Test chunked_fast_slow feature` analogous to the existing failpoints carve-out. Combines `chunked_fast_slow` AND `test-utils` so tests requiring both (e.g. `fast_slow_str_key_skips_chunked_dispatch_test`, `writer_termination_245_drop_fallback_test`, `chunked_filesystem_serialization_test`) all run in one pass. Also adds `[[test]] required-features = ["chunked_fast_slow"]` for `memory_store_backpressure_eviction_test` so cargo silently skips it from default `cargo test --all` (matches the existing pattern for `memory_store_backpressure_emission_test`). The other two new test files (`memory_store_pin_durability_test`, `ac_pin_durability_test`) do not depend on chunked_fast_slow APIs and run fine without the feature, so no required-features declaration for them. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Description
Minor issues related to MacOS for the integration tests.
Fixes # #316
Not technically an issue. It's blocking an in-progress PR.
not work as expected)
How Has This Been Tested?
Please also list any relevant details for your test configuration
Checklist
bazel test //...passes locallygit amendsee some docsThis change is