Repository navigation
Conversation
Completes the no-per-PR-OCI-builds intent: image-off-pr moved image.yaml off PRs and TraceMachina#2877 moved the nix integration job off PRs, but main.yaml's integration-tests (docker-compose image build + run_integration_tests.sh) fell between them and still ran on every PR. Defer to nightly + merge-to-main, matching TraceMachina#2880's pattern. Fork-local until folded into the upstream OCI/nightly set.
…nstruction The matching-engine task is spawned inside `Arc::new_cyclic`, during which `weak_inner.upgrade()` returns `None` because the strong count is still zero. The task treats a failed upgrade as scheduler shutdown, so if `task_change_notify` already held a permit (e.g. a Redis pubsub event arriving before `SimpleScheduler::new`), its first poll completed instantly, consumed the permit, saw `None`, and returned permanently — a scheduler that never matches anything. Gate the task's loop on a oneshot fired unconditionally right after `Arc::new_cyclic` returns, so the task can neither consume a pre-stored notify permit nor observe the partially constructed scheduler. Includes a deterministic in-crate repro (adversarial review fleet, branch repro/me-constructor-race, d44e3cb) that pre-stores a notify permit and widens the in-closure window via a test-only timing seam; it fails without the fix and passes with it. Co-Authored-By: Claude Fable 5 <[email protected]>
… writes
Client keep-alive stamps in the memory awaited-action db (the
ActionEvent::ClientKeepAlive path in handle_action_events, and the
initial stamp in try_subscribe) mutate the AwaitedAction in place via
send_if_modified WITHOUT bumping AwaitedActionVersion. Any concurrent
read-modify-write cycle (SimpleSchedulerStateManager borrow() ->
update_awaited_action) that snapshotted the action at version N before
the stamp still passes the version CAS, and send_replace writes the
whole stale snapshot back -- a classic lost update that silently moves
last_client_keepalive_timestamp backwards. The Code::Aborted retry
defense never fires because the version matches. Consequence: the
client-timeout sweep force-fails operations whose clients are alive and
polling ("no more clients listening" DeadlineExceeded), including
operations a client subscribed to moments earlier.
Fix shape: treat the keep-alive stamp as data outside the versioned
snapshot rather than routing it through the versioned update path.
update_awaited_action now merges the stamp forward (monotone max of old
and new) before send_replace, so an accepted stale write can never
revert it. Chosen over version-bumping keep-alives because (1) a bump
would spuriously abort legitimate in-flight worker updates and wake
every changed() subscriber ~every CLIENT_KEEPALIVE_DURATION per client,
(2) it needs no new per-operation state, and (3) it leaves the
AwaitedAction wire/record format and the store/redis backend untouched.
Two deterministic repro tests included; both fail without the one-line
merge and pass with it (verified both directions, 5x each).
Provenance: adversarial review fleet.
Co-Authored-By: Claude Fable 5 <[email protected]>
…under a live id ApiWorkerSchedulerImpl::add_worker used LruCache::put, which silently replaced a live entry when a worker re-registered under an existing id. The replaced Worker was dropped together with its running_action_infos: nothing was requeued, the old channel never heard Disconnect, and the gauges were only partially adjusted. Downstream, the replaced worker's operations stayed Executing assigned to the id; when the new incarnation was later evicted its empty list requeued nothing, and the eventual timeout requeued the operation to another worker while the original process might still be running it -- double execution. Now ApiWorkerScheduler::add_worker treats a live entry under the same id exactly like a disconnect before admitting the new incarnation: it goes through immediate_evict_worker, which drains running_action_infos via UpdateOperationType::UpdateWithDisconnect, sends Disconnect on the old channel, and balances the connect/disconnect gauges. A requeue failure is logged and does not block the reconnecting worker. Deterministically reproduced by the DST fuzzer schedule in PR TraceMachina#2831; the new regression test (worker re-registers mid-execution, asserts Disconnect on the old channel, re-dispatch of the same operation to the new channel, and completion) fails on unfixed main. Co-Authored-By: Claude Fable 5 <[email protected]>
…n remove_timedout_workers remove_timedout_workers had a TOCTOU window: it snapshots each worker's last_update_timestamp under the inner lock, releases the lock to consult the worker registry (an await boundary every sweep crosses), then re-acquires the lock and calls immediate_evict_worker using the stale snapshot. A keepalive processed in that window refreshes last_update_timestamp under the very same mutex, yet the sweep never re-consults it, so a demonstrably-alive worker is evicted and all of its running actions are requeued. Fix: after re-acquiring the inner lock, re-read the worker's last_update_timestamp and skip the eviction if it has moved past the timeout threshold. Because keepalives update the timestamp under the same mutex, the staleness decision and the eviction are now atomic with respect to keepalive processing. No new locking is added to any dispatch path. Kill-overdue evictions deliberately do not re-check: keepalives from a worker that refuses to acknowledge a kill are exactly the wedged-worker symptom that path exists to handle. Tests: two deterministic repros. One parks the sweep at the pre-existing await boundary via a cfg(test)-only Notify rendezvous and delivers a full worker_keep_alive_received; the other forces the interleaving with genuine lock contention (holding the registry write lock, then the inner lock) via a cfg(test)-only WorkerRegistry::write_lock_for_test accessor. Both fail without the fix and pass with it. The write_lock_for_test seam is kept: it exercises the window through real lock contention rather than the injected gate, so the two tests do not share a failure mode. Provenance: adversarial review fleet. Co-Authored-By: Claude Fable 5 <[email protected]>
…keepalive_ts, retiring a live client's action as abandoned Co-Authored-By: Claude Fable 5 <[email protected]>
…l timestamp In OperationSubscriber::changed (USE_SEPARATE_CLIENT_KEEPALIVE_KEY branch), a failed ck_* store write still advanced the local last_known_keepalive_ts and then parked for a full CLIENT_KEEPALIVE_DURATION. Peer replicas judge client liveness solely from the store, so a couple of consecutive transient store failures (Redis blip) left the store's keepalive stamp aging past client_action_timeout and a peer's sweep retired the live client's action as abandoned. Now the local timestamp only advances on a successful write; on failure the sleep becomes a bounded CLIENT_KEEPALIVE_RETRY_DELAY (1s) so the write is retried promptly without spinning when the store is down hard. Repro test fails before the fix (attempts = 1 across 3x keepalive periods) and passes after. Co-Authored-By: Claude Fable 5 <[email protected]>
…ner.lock (TL1)
Registry is the second liveness source remove_timedout_workers consults ("alive
if EITHER source"), but refresh_worker stamped it only AFTER the inner.lock block
— which holds the lock across an await (unacked-dispatch sweep). A punctual
keepalive whose inner.lock is contended, or whose task is starved on a saturated
runtime, refreshed NEITHER liveness source before the eviction sweep read them,
falsely evicting a live worker. Transport/runtime-starvation liveness class,
seeded by Aman TraceMachina#2856 (h2-read starvation); TL1 is the sibling where a read
keepalive must still LAND through the lock.
Fix: stamp update_worker_heartbeat on receipt (lightweight RwLock insert, no
await-under-lock/store/sweep); make it monotone so an out-of-order keepalive
cannot regress last_seen. Regression heartbeat_is_monotone_never_regresses is
fails-without by construction; decoupling half exercised by the soak harness.
NOT YET built/run on metal.
Co-Authored-By: Claude Opus 4.8 <[email protected]>
Astra finding N2 (P1): a same-worker retry overwrites a live reservation. `Worker::run_action` unconditionally re-ran `reduce_platform_properties` and overwrote the op-keyed `running_action_infos` entry. When an op whose earlier attempt was still resident (e.g. an execution-deadline requeue that left the ledger entry in place) was dispatched again to the same worker, the platform budget was deducted a second time while `complete_action` only refunds the surviving entry — leaking the first reservation's capacity permanently. Fix: reject-if-exists under the dispatch lock. `run_action` now refuses a dispatch for an op the worker already holds (Code::AlreadyExists) before any budget deduction or insert. Regression test: tests/worker_retry_reservation_test.rs. It dispatches an op (reserving 4 of 10 mem), then re-dispatches the same op to the same worker and asserts the budget is still 6. WITHOUT THE FIX the second dispatch deducts again and the budget reads 2 (test fails: found 2, expected 6); with the fix the redundant dispatch is refused and the budget stays 6. Co-Authored-By: Claude Opus 4.8 <[email protected]>
…n, not timestamp kill_revoked_operations snapshots (worker, operation) pairs under the scheduler mutex, then confirms revocation against the state manager lock-free. If the operation is declined/requeued and re-dispatched to the same worker inside that window, the final locked re-guard used to pass on the fresh entry and the kill landed on the new, legitimate dispatch. An earlier version of this fix carried the entry's dispatched_at timestamp as the dispatch's identity, but a timestamp is not an identity: dispatched_at has second resolution (duration_since(UNIX_EPOCH).as_secs()), so a requeue-and-redispatch inside one wall-clock second carries the same dispatched_at and the stale kill still lands. Review caught the collision; the regression test now pins both dispatches to the same second and fails on the timestamp scheme. Each dispatch now carries a dispatch_generation: a scheduler-wide u64 incremented under the scheduler mutex at send time, stored on the running_action_infos entry. The sweep snapshots the generation and worker_notify_kill_operation only sends the kill if the entry under the lock still carries it; a re-dispatch always gets a strictly greater generation, so a stale kill can never match. The counter is scheduler-wide rather than per-worker: a per-worker counter would restart when a worker re-registers under the same id, letting a stale snapshot from the old incarnation match a fresh dispatch on the new one. dispatched_at stays as a timestamp for the acknowledgement timeout math. Also registers tests/kill_revoked_redispatch_race_test.rs in the Bazel rust_test_suite, which had not picked it up. Provenance: adversarial review fleet; incompleteness found by a follow-up review of TraceMachina#2838. Co-Authored-By: Claude Fable 5 <[email protected]>
Matching computed typed resource requirements once and parked that snapshot; assignment reloaded the current action but only guarded against double-assignment. It never compared the reloaded action's requirements against the ones the matcher used to pick the worker. So a matcher could park a 4 GiB snapshot of X; a peer runs X, hits a memory-limit kill, requeues X escalated to 8 GiB; the stale 4 GiB snapshot then dispatched X to a 4 GiB-only worker. The DB said 8 GiB but StartExecute and the reservation carried 4 GiB — the escalation was silently bypassed. Fix: assign_operation now carries the placement-relevant requirement snapshot the worker was chosen for. The accept, inside the same CAS that flips the action to Executing, rejects (Aborted) if the action's current platform_properties no longer match that snapshot. A stale proposal becomes a lost race; the action stays queued and is rematched against its current requirements. Dispatch still sends the parked payload, so rejecting before send keeps DB and worker in agreement. Regression test assignment_rejects_a_stale_placement_after_escalation reproduces the two-matcher interleaving: it FAILS without the guard (the 4 GiB placement is silently accepted to Executing) and PASSES with it. Co-Authored-By: Claude Opus 4.8 <[email protected]>
`try_subscribe` stopped at the first exact unique-qualifier match, even when that match was a completed record. The unique-qualifier index retains completed records alongside any live successor, and the backend does not order results by state, so a retained completed record could be returned ahead of a joinable live one — forcing an unnecessary recreate and a new operation instead of joining the live successor. Scan every exact match and prefer a joinable (neither finished nor abandoned) candidate, keeping the first exact match only as a fallback. This is the selection half of finding N7. The atomic qualifier->live-op claim (two concurrent same-qualifier submissions can still both create an operation) needs a store primitive that does not exist today (a compare-and-set on the shared qualifier key); that is tracked separately. Co-Authored-By: Claude Opus 4.8 <[email protected]>
…s listening, instead of being counted forever A queued index entry whose backing record cannot be decoded (for example serialization drift across a rolling upgrade, or a corrupted write) had no TTL and no deletion path. The sweep and the listings skip such an entry and count it (`record_awaited_action_orphan`), but nothing ever removes it, so it is re-listed, re-skipped and re-counted by every pass forever: the queue depth metric stays accurate while the queue itself never drains. The redis listing surfaces the failure as a stream error with no key attached, so the sweep could not delete the record even if it wanted to. The FT.AGGREGATE now also loads RediSearch's `__key`, and when a listed row fails to decode the store removes it under three guards, checked atomically in Lua: - only the queued-state listing opts in (`SchedulerIndexProvider::lost_record_liveness_guard_key`); - the record's client keepalive key (written with a TTL of the client action timeout) must not exist, so a record any client has touched within the timeout is never removed; - the record must still hold the version the listing saw, so a record rewritten concurrently is never removed. The row is still reported as lost, so the orphan counter records it one last time before it is gone. Co-Authored-By: Claude Opus 4.8 <[email protected]>
… entry The inflight-dedup drop-guard in CacheLookupScheduler::add_action was bound as `let _scope_guard = scope_guard;`, so it fired on *every* exit of the spawn — including normal completion, after the spawn had already removed its own entry and resolved the coalesced waiters. A concurrent request for the same cacheable key, arriving in the window between the leader's own `inflight_cache_checks.remove(...)` and the spawn's exit (dropping the guard), inserts a fresh entry. The stale guard then removes *that* entry, orphaning the second request: its oneshot sender is dropped, surfacing a spurious "tx hung up" error to the client and forcing a needless retry. Fix: defuse the guard via `ScopeGuard::into_inner` the moment the cache check returns. From that point every arm removes our own entry with no intervening `.await` (no drop point), so defusing is leak-free and the guard can no longer clobber a concurrent same-key re-insertion. The guard still provides true drop-safety for the only window that matters: a spawn death during the cache-store check, before we claim the entry. Regression test leader_drop_guard_does_not_orphan_a_later_same_key_request parks a leader in its forward to the inner scheduler (an await after it has removed its entry), sends a second same-key request into that window, then lets the leader finish. Without the fix the second request fails with "tx hung up"; with it, the second request is forwarded and answered. Co-Authored-By: Claude Opus 4.8 <[email protected]> Co-Authored-By: Claude Opus 5.5 <[email protected]>
…aphore Evicted entries' backing files are unlinked by a fire-and-forget background_spawn! in EncodedFilePath::drop, with no bound. Under an eviction storm (cache wipe, overwrite flood) that grows without limit: queued task frames consume memory and disk/inode reclamation lags behind what the async runtime can schedule. Open FDs are already bounded by a semaphore; deletes were not. Add max_concurrent_deletes. drop is sync so it cannot await a permit; it try_acquire_owned's and, when saturated, deletes synchronously inline -- which is the backpressure (the evicting thread pays the unlink cost directly) and never leaks a file. An inline_deletes metric exposes how often backpressure engages. Bounded by default (1024), unlike max_concurrent_writes: leaving an unbounded delete backlog off by default would move a latent memory/inode blowup from a high-context code review to a late-night config change nobody remembers to make. The default is generous enough that normal eviction never reaches it (so the inline path never fires in ordinary operation); it only engages under a genuine storm. 0 remains the explicit escape hatch for unlimited. The serde field default governs JSON5 configs (production); derive(Default) still yields 0, so existing Rust-constructed tests are unchanged.
… as success DedupStore::get_part trusted the per-chunk sizes recorded in the index and streamed each content chunk without checking it actually held as many bytes as its index entry declared. A short content chunk (a torn/partial read or an index/content size disagreement) would either panic the worker on the bytes_to_skip assert, or silently under-deliver and send EOF, serving a truncated read to the caller as success. Add a per-chunk size-consistency check: a fetched content chunk whose length does not equal the size its index entry declares now yields a NotFound error. The check is per-chunk (not a post-loop total), so a legitimate bounded request that overshoots the end of the blob still returns the available bytes. Regression tests: get_part_short_chunk_is_error_test (fails without the fix: "got Ok with 10 bytes") and get_part_over_length_request_still_ok_test (guards against over-correction). Co-Authored-By: Claude Opus 4.8 <[email protected]>
The test file was only picked up by Cargo's tests/ auto-discovery; the Bazel integration suite lists its sources explicitly, so CI never ran it. Register it, and drop the dev-dependency on mock_instant, which is already a regular dependency of the crate. Co-Authored-By: Claude Opus 5.5 <[email protected]>
The test file was only picked up by Cargo's tests/ auto-discovery; the Bazel integration suite lists its sources explicitly, so CI never ran it. Co-Authored-By: Claude Opus 5.5 <[email protected]>
Rejecting a wrong-size chunk with NotFound stopped the truncated read, but left the bad chunk in place. `has()` sums the sizes the content store reports, so the blob stayed "present", and `update()` skips any chunk the content store already has, so the client's re-upload kept the bad copy. The blob could never be read again. On a size mismatch, `get_part` now makes a best-effort `remove` of the chunk from the content store, as `VerifyStore` does for a copy that fails verification. The blob then reads as missing and the next upload writes the chunk again. The error stays NotFound, not DataLoss, so a `FastSlowStore` above can still fall back to its slow tier. Tests now cover the error code, the removal, a read that starts inside a short chunk (the old assert panic), a chunk longer than declared, and an end-to-end truncate / fail / re-upload / read cycle. The removal, has() and repair assertions fail without this change. Co-Authored-By: Claude Opus 5.5 <[email protected]>
…part
S3/ONTAP/GCS get_part built bytes={offset}-{offset+length}, but HTTP byte
ranges are inclusive on both ends, so a bounded read ending before the object
end over-read one byte (a torn read on content-addressed partial reads). Use
offset+length-1, guard zero-length reads, and fix the GCS mock (which sliced
exclusively and masked the bug). r2_store wraps s3_store, so S3/R2/ONTAP + GCS
are all covered.
Co-Authored-By: Claude Opus 4.8 <[email protected]>
The part-size math targeted MIN_MULTIPART_SIZE (5 MiB), so a large blob was split into thousands of tiny parts — a 12 GiB blob became ~2300 concurrent 5 MiB PUTs. That churns HTTP connection pools (aws-smithy "connection never set" storms); on a good link the retries win slowly, on a lossy one they exhaust and the slow-store write fails, which write-through then cascades into dropping the successful fast-store write too. Size parts at a configurable multipart_part_size (default 64 MiB), growing only as needed to stay under the 10,000-part ceiling. 12 GiB -> ~192 parts. The same knob is wired into the ONTAP S3 store, whose previous formula divided the object size by the 5 MiB floor — a part count — and clamped that as a byte size, so every blob used 5 MiB parts. Larger parts mean more buffered bytes, so this also makes the buffering bounds real, in both stores. The existing select loop drained the bounded part channel into an unbounded FuturesUnordered, so the documented per-upload bound was never enforced; the recv arm is now gated on in-flight uploads, bounding one upload's buffer at roughly (2 * multipart_max_concurrent_uploads + 1) * part_size. Across uploads, a store-wide admission semaphore (max_concurrent_multipart_uploads, default 4, whole-upload permits so it cannot deadlock the part loop) bounds the aggregate near 5 GiB at the defaults. The single-request threshold intentionally stays at 5 MiB: the single-PUT path replays from a max_retry_buffer_per_request-sized buffer (default 5 MiB), so raising the threshold would break retries for mid-size uploads. Mid-size blobs pay two extra small requests. The existing multipart mock tests pin multipart_part_size to the old 5 MiB so their part boundaries stay small (which also exercises the knob); a new test locks the default to a single part for a blob between 5 MiB and 64 MiB. Co-Authored-By: Claude Fable 5 <[email protected]>
…ne), not Err
ExperimentalMongoStore::update_data used find_one_and_update(...).upsert(true). On a version-CAS conflict (a concurrent writer already bumped the version) the {_id, version} filter matches nothing, so upsert tries to INSERT a doc with the existing _id and MongoDB returns a DuplicateKeyError (E11000), surfaced as Err(Internal). The version-CAS contract (and the Redis backend, which returns Ok(None)) require a conflict to be retryable; instead concurrent updates on a Mongo-backed record spuriously failed. Fix: .upsert(current_version == 0) so only a genuine first-create upserts; a non-matching filter on an existing record now returns Ok(None) (retryable). Adds a deterministic fails-without regression and corrects an existing (ignored) scheduler test that had pinned the buggy Err behavior.
Co-Authored-By: Claude Opus 4.8 <[email protected]>
Three follow-ups to the change that stopped backend errors being served as misses. - A failure in the CAS existence check (the output files, stdout, stderr) was still rewrapped as `Internal`, which retry policies treat as permanent. It now keeps its own code, so a transient CAS fault is retryable like an AC one. `incomplete_entries_counter` now counts only genuine misses, not propagated errors. - Tests cover the two misses that must stay `NotFound` and were untested: an absent action-result record and an evicted Tree blob. Both fail if every lookup error is propagated. The existing get-path miss now checks for `NotFound` exactly, and a new test checks a CAS failure keeps its code (it fails against the old `Internal` wrapping). - The scheduler regression test is bounded, so a regression fails in 5 s instead of hanging, and its fake store uses its configured code. Comments no longer claim undecodable records propagate: `ac_utils` maps decode failures to `NotFound`, so they stay misses. Co-Authored-By: Claude Opus 5.5 <[email protected]>
…ff the lock Follow-ups from review of the subscription-lifecycle fix. reconnect: subscribing the new connection to patterns recorded mid-reconnect happened while holding the connection write lock, so every get_connection (every Redis operation) waited on those round trips. Subscribing now happens without the lock; the lock is taken only to check nothing is still missing and to publish, repeating if a pattern arrived in between. A pattern recorded after the final check still waits for the lock in get_connection and then subscribes on the new connection, so the original race stays closed. Eviction subscription retry: - The task holds a weak reference, so a dropped store ends it instead of being kept alive and retried forever. - It logs the first failure as an error, then only at powers of two, so a permanent failure (an ACL forbidding PSUBSCRIBE) doesn't log every 5 s. - On a connection-type failure it re-resolves the connection, as the store's other retry paths do, instead of retrying a dead handle. - Reading notify-keyspace-events stays advisory, now logged at warn with its own message so it can't be mistaken for the subscription failing. Tests: - The existing init test only covered the advisory CONFIG GET and passed without any retry. A new test fails the first PSUBSCRIBE and checks the subscription is still established; it fails if the retry loop gives up. - A new test checks a dropped store ends the retry task; it fails if the task holds a strong reference. - A new unit test checks get_connection is not blocked while reconnect catches up; it fails against the previous locking. - The reconnect race test uses #[nativelink_test] instead of a hand-built runtime, so the clippy and lint exemptions are gone, and the test names now describe the fixed behaviour. - The fake Redis gains make_fake_redis_with_first_responses, which answers a command differently the first time only. Co-Authored-By: Claude Opus 5.5 <[email protected]>
… residual) PR TraceMachina#2841 added length + existence guards to the chunked Redis read path. Astra's C10 audit flagged that length checking cannot establish stable value identity: if a key's value is replaced by a *different* value of the same total length between chunk reads, the reader stitches bytes from two values and serves a torn blob as success. STRLEN is identical the whole way through, so no length-based guard can catch it. This confirms the hole is reachable and ships a reproduction, rather than a shallow length patch that would not close it. `get_part` reads a value larger than `read_chunk_size` with one `GETRANGE` per chunk, each a separate round-trip; values are mutable (`update` writes a temp key then `RENAME`s it onto the final key, discarding whatever was there). Content addressing makes this a non-issue for CAS, but the AC / scheduler / arbitrary-key uses of this same driver mutate values legitimately, so an equal-length replacement interleaved between two chunk reads tears the result. The new test drives exactly that interleaving through the mock connection and asserts the current code returns head-of-A spliced onto tail-of-B as Ok. Why no point fix here: closing equal-length replacement requires a snapshot-consistent read across more than one round-trip. On plain Redis strings that is only achievable by (a) reading the whole range in one atomic command (EVAL / MULTI), which materializes the entire value in a single reply and is unbounded for large blobs, or (b) attaching a version/identity to the value that `update` writes and `get_part` checks across chunks -- a change to the write path and the key/value schema. Both change the store read/write contract broadly, so the complete fix is RFC-track rather than a local patch. The single-chunk read path is already atomic and unaffected. Co-Authored-By: Claude Opus 4.8 <[email protected]>
The production wedge: a ShardStore ring over grpc_store backends hangs EVERY CAS RPC when one peer is dead or restarting, with no error and no timeout — the operational workaround was restarting the frontend. Where the hang lived: ConnectionManager retries a dead endpoint's connect forever in the background (by design — never give up on an endpoint), but connection() requests queue unbounded in waiting_connections while it does, so no error ever reaches the retry/error machinery. rpc_timeout_s was only applied on the ByteStream.Write path; reads and unary RPCs had no deadline at all. The fix, two layers: - connection_manager: track per-endpoint connect health. When the last connect attempt to every endpoint has failed and no channel is available, fail queued and incoming connection() requests fast with Unavailable instead of queuing them forever. A background reconnect success clears the flag and service resumes. - grpc_store: apply the existing rpc_timeout_s knob to every RPC — unary calls, and read-stream establishment through the first response chunk — not just writes. A timeout maps to DeadlineExceeded, which is retryable. Tests (repro first; both failed against the old code by hanging past a generous outer timeout): - connection_manager_test::dead_endpoint_fails_fast_instead_of_queuing_forever - shard_ring_dead_peer_test: an in-process two-shard lab ring (one live fake CAS peer, one dead port) proving reads and multi-key existence checks settle with Unavailable/DeadlineExceeded while the live shard keeps serving. Co-Authored-By: Claude Fable 5 <[email protected]>
`nativelink <config>` previously validated a config only by booting it (binding listeners, connecting to backends), so there was no way to check a config in CI without a live environment. Add `nativelink --check <config>`: it parses the config (already running check_store_conflict) and resolves every store/scheduler name reference — the typo'd-reference class that otherwise only surfaces at boot — then exits without constructing the store manager, binding a socket, connecting to any backend, or creating store directories. Success prints a one-line summary to stdout (exit 0); unresolved references and duplicate store/scheduler names are collected and reported to stderr, each with its config path (exit 1). CasConfig::validate_references walks workers and server services, and recurses ref_store references for the transitive closure of stores that are actually wired to a consumer. It is conservative to avoid false positives: optional/defaulted refs are checked only when present, and store-internal refs of unwired stores are skipped — so all 17 shipped example configs pass --check (proven offline via strace: zero network connects), while a dangling reference is named and fails. Co-Authored-By: Claude Fable 5 <[email protected]>
…es the digest An existence check that found a slow-store write in flight parked on that specific writer's completion mutex. A ByteStream upload that is interrupted (cancel, half-close, client deadline) is parked for resumption, keeping its FastSlowStore::update() future -- and thus the locked mutex -- alive. A later registration for the same key overwrote the map entry, so completing the digest under a fresh upload id never woke the original waiters: they starved until their client deadline even though the blob was present and new lookups succeeded. Replace the per-writer mutex with a per-key slot shared by all concurrent writers of that key, broadcasting Pending/Done over a watch channel. Any writer completing the digest wakes every waiter; the entry is removed (and waiters released with "not written") only when the last writer deregisters, which also fixes the stale-remove where a second writer's guard evicted a slot the first writer still occupied. Regression: stalled_upload_waiter_released_by_fresh_upload_of_same_digest -- fails on v1.7.3 (waiter hangs past a 2s deadline), passes with the fix; also asserts a genuinely-absent digest still resolves promptly. Co-Authored-By: Claude Opus 4.8 <[email protected]>
The N2 regression test was added under nativelink-scheduler/tests/ but never listed in the rust_test_suite, so Cargo ran it and Bazel (and CI) did not. List it alongside the other integration tests. Co-Authored-By: Claude Opus 5.5 <[email protected]>
Regenerated web/apps/docs/content/docs/reference/nativelink-config/main.mdx from the nativelink-config crate (build-schema + schema-to-mdx), as AGENTS.md requires when a config field or its doc comment changes.
…feats-version-cas' into HEAD
…HEAD # Conflicts: # nativelink-scheduler/BUILD.bazel
# Conflicts: # nativelink-scheduler/tests/simple_scheduler_state_manager_test.rs
…t-queued-records' into HEAD # Conflicts: # nativelink-store/src/redis_store.rs
…esh-upload' into HEAD # Conflicts: # nativelink-store/tests/fast_slow_store_test.rs
# Conflicts: # web/apps/docs/content/docs/reference/nativelink-config/main.mdx
…er' into HEAD # Conflicts: # .github/styles/config/vocabularies/TraceMachina/accept.txt # web/apps/docs/content/docs/reference/nativelink-config/main.mdx
…ig' into HEAD # Conflicts: # web/apps/docs/content/docs/reference/nativelink-config/main.mdx
Co-Authored-By: Claude Fable 5.1 <[email protected]>
- fast_slow_store_test: the merge of the N14 inflight-waiter test had landed inside the filesystem-slow-tier test's body; close that test. - simple_scheduler_state_manager_test: rebuilt from the first parent plus the N3 placement-version hunks, so both the concurrent-listing test and the stale-placement test are whole. - worker_retry_reservation_test: RunAction carries a dispatch generation since the N3 merge; pass 0. Co-Authored-By: Claude Fable 5.1 <[email protected]>
…e lock inside the stream Since 4edf822 (TraceMachina#3001) the filter pass reads listed rows 64 at a time with `buffered`, which keeps asking the listing for the next row while it hands rows downstream. The memory store's listing took the store's lock inside the stream (a `ChunkedStream` whose chunk fetch awaits `inner.lock()`), so under load the next chunk fetch was left parked inside the stream, waiting for the lock, while the matcher wrote for a row it had been handed, on the same task, and waited for the same lock. Once the parked wait had starved past async-lock's fairness threshold the lock was reserved for it, nothing polled it, and the scheduler stopped for good: every state update blocked, actions the workers had finished and uploaded "timed out with no update from their worker", and nothing dispatched again. On railgun a `bazel build //...` of this repository through a three-worker fleet wedged within 15 s on TraceMachina#3001 and on main, every time; v1.7.7, TraceMachina#2954 and v1.8.0 run it clean. The listing is now read whole under one hold of the lock, taken in the `async fn` before the stream exists, and the stream only hands out what was read. The chunk fetch already read the whole range in one go, so the snapshot is what the stream yielded before; the lock just moves out of it. Rows are still handed on one at a time as they are read, which the matcher's mid-pass placement relies on, and `buffered` keeps its concurrency for the Redis store, whose reads are round trips. The test pins what the fix relies on: a row added after the listing was requested is not in it. The lazy listing read it at the first poll. Co-Authored-By: Claude Fable 5.1 <[email protected]>
…nto HEAD # Conflicts: # nativelink-scheduler/tests/simple_scheduler_state_manager_test.rs
`warn_if_eviction_events_are_disabled` holds a `ClientWithPermit<C>` across an await, which the nightly clippy lane (`future_not_send`) rejects while `C` is only `Sync` there. Every connection type used is `Send`, and the `StoreDriver` impl already requires it. Co-Authored-By: Claude Fable 5.1 <[email protected]>
This was referenced Oct 22, 2026
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
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.
What and why
The single soak branch: TraceMachina/nativelink
mainatb01417643e(v1.8.0 + TraceMachina#2997–TraceMachina#3007, including TraceMachina#3006's filesystemhas()content-length fix) with every live fix branch merged on top (30 merges,--no-ff, one per branch, so each is a revertable unit), then the config reference regenerated. Not for merging intomain; it exists to be built, soaked on railgun and compared against upstream HEAD.Merged: #5 #36 #37 #38 #39 #40 #41 #42 #43 #44 #45 #46 #50 #51 #52 #53 #54 #56 #57 #58 #59 #62 #68 #72 plus the PR-less
fix/completeness-followups,fix/redis-subscription-followups,upstream/delete-backpressure(TraceMachina#2910),b7r6/shard-ring-dead-peer-failover,b7r6/cas-batch-read-timeout-config,pr/buf-channel-inflight,pr/sha2-asm.Left out on purpose: #49/TraceMachina#2842 (global pause starves invalidation; needs per-key pausing first), #64 (fuzzer; its own smoke tests fail), the
ci/*,diag/*anddocs/*branches.Conflicts resolved by hand:
nativelink-scheduler/BUILD.bazel(two test registrations),simple_scheduler_state_manager_test.rs(two appended tests),redis_store.rs(keptDOC_KEY_FIELD_NAME, droppedINDEX_TTL_S, which TraceMachina#3000 removed upstream),fast_slow_store_test.rs(two appended tests), and the generatedmain.mdxthree times (regenerated once at the end). One test adapted: the filesystem-slow-tier populate test no longer asserts the block-roundedhas()size, since TraceMachina#3006 changed it.How was this verified?
bazel test //...locally: 130/133 before the test-file repairs; the three repaired targets pass; the full rerun on this exact commit is in flight.nativelink --check(from nativelink: add --check for offline config validation #62) accepts upstream-dialect renders of railgun's cas, scheduler and worker configs.bazel build //...of this repo remote-executed through it.Risk
None to
main; draft, not for merge. The soak itself runs beside the live 1.7.4 fleet on separate ports and state.AI assistance
Assembled with Claude Code; b7r6 chose the set and reviewed the conflict resolutions.
🤖 Generated with Claude Code