Repository navigation
Upgrade dependencies - #87
Merged
Merged
Conversation
allada
commented
Jun 17, 2022
allada
left a comment
allada
left a comment
Contributor
Author
There was a problem hiding this comment.
Reviewed 32 of 32 files at r1, all commit messages.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on @allada)
allada
commented
Jun 17, 2022
allada
left a comment
allada
left a comment
Contributor
Author
There was a problem hiding this comment.
Reviewed 1 of 32 files at r1.
Reviewable status:
rejuvenile
added a commit
to rejuvenile/nativelink
that referenced
this pull request
Aug 19, 2026
…ality_map on epoch change) Workers send their boot_epoch_id in ConnectWorkerRequest. The server tracks the last-seen epoch per endpoint and atomically wipes that endpoint's locality_map entries on a new epoch (or on epoch=0 = legacy). This closes the lost-eviction race where a worker restart could leave the server with stale entries pointing at blobs the worker no longer has. Three commits from worktree-agent-a6873bf3 (rebased onto 71a067f): - WIP failing tests + proto field - server wipes locality_map on epoch change - clippy is_some_and review nit Includes manual pb.rs codegen for the new proto field (regen via canonical bazel toolchain queued as TraceMachina#87 follow-up). 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
Closes the BLOCK + MAJOR set surfaced by the TraceMachina#494-v3 cadre audit at HEAD 5d7d08ac. The bundle ships with v2 default-on; this commit hardens the v2 path against the half-done H1 reader cascade, the missing QueryWriteStatus phantom-success guard, the v1 deferred-publish phantom-Ok on commit failure, and the chunked_in_flight set lacking refcount semantics for concurrent same-digest sessions. BLOCK-A (QueryWriteStatus phantom-success guard): - nativelink-service/src/bytestream_server.rs inner_query_write_status now mirrors the H2 pattern from bytestream_write: downcast to FastSlowStore, consult is_chunked_in_flight, return complete=false when the digest is in chunked_in_flight_digests. Pre-fix, the has()=Some cascade returned complete=true while the v2 commit could still fail, breaking Bazel's durability contract. - Test: bytestream_server_test.rs::block_a_query_write_status_does_ not_phantom_ack_for_chunked_in_flight. Mutation-verified red-fail with the bespoke "QueryWriteStatus phantom-acked in-flight chunked digest" message. BLOCK-B (H1 reader-cascade Option F1 — read-blocks-on-commit): - nativelink-store/src/fast_slow_store.rs get_part now waits (bounded 5s polling at 50ms) when the digest is in chunked_in_flight_digests AND the chunked_read_registry pin has fallen through (v2 case: no driver pin). Closes the gap where has_with_results said Some (via the in-flight set) but get_part returned NotFound (canonical not yet on disk + no pin to serve). - Test: chunked_write_handler_v2_test.rs::block_b_h1_reader_cascade_ blocks_on_v2_inflight_then_serves_from_slow. Test simulates v2 commit landing during the wait; reader observes correct bytes. Mutation-verified red-fail with bespoke "H1 reader-cascade returned NotFound during v2 in-flight window" message. BLOCK-D (H2 production-composition test): - New test bytestream_server_test.rs::block_d_bytestream_write_h2_ does_not_phantom_ack_when_chunked_in_flight. Wraps FSS + chunked_in_flight_digests + ByteStreamServer in production composition; asserts second concurrent ByteStream::write does NOT phantom-ack. Mutation-verified red-fail with bespoke "ByteStream:: write phantom-acked second concurrent writer while chunked commit was still in-flight" message. BLOCK-E (subscribe → enable → peek → notified): - nativelink-service/src/chunked_write_handler_v2.rs::v2_await_commit_ result and chunked_write_handler.rs v1 AwaitCommit branch now call tokio::pin!(notified) + notified.as_mut().enable() BEFORE peek. The dispatch-prompt's missed-wakeup hypothesis is mitigated in tokio 1.49 by notify.rs:572's counter-capture-at-creation, but enable() is defense-in-depth against future tokio API drift AND self-documenting (eager registration is the intent). - Test: chunked_write_handler_v2_test.rs::block_e_missed_wakeup_ publish_before_first_poll_is_observed. Verifies publish-between- subscribe-and-first-poll is observable through notified.await. - Doc-comment correction: source code now documents that the classical missed-wakeup is NOT active in tokio 1.49 (counter-capture handles it), and explains why enable() is still worth applying. MAJOR-F + MAJOR-H (deferred-publish reactive instead of busy-spin): - chunked_write_handler.rs::dispatch_chunks_to_driver gains an async_result_relay parameter; the AsyncCommit branch threads it to run_async_commit_reaper. dispatch_bazel_facing_internal_chunking also gains the parameter; the BazelChunkedDispatcherImpl::dispatch Owner branch creates a oneshot pair, passes the sender into dispatch, and the deferred-publish task AWAITS the real commit_result instead of polling in_flight.contains_digest. - Eliminates: busy-spin on yield_now (N×100us ticks during multi-second commits), synthetic-Cancelled trap on spawn drop, phantom-Ok on commit failure (the prime DS hazard). - Test: chunked_write_handler_v2_test.rs::major_f_deferred_publish_ propagates_commit_err_to_sibling_v2. Uses a hash-mismatch payload to force commit failure; sibling v2 in AwaitCommit MUST observe Err (not phantom-Ok). Mutation-verified red-fail with bespoke "deferred-publish published Ok despite v1 commit failure" message. MAJOR-G (chunked_in_flight_digests refcount): - Type change: Arc<Mutex<HashSet<DigestInfo>>> → Arc<Mutex<HashMap<DigestInfo, NonZeroU32>>> in fast_slow_store.rs + chunked_write_handler.rs. InFlightChunkedGuard::new bumps refcount; Drop decrements and removes only when refcount reaches zero. - Pre-fix: two concurrent v2 sessions for the same digest both "inserted" (idempotent on HashSet), but the first to drop removed the entry while the second was still in-flight. BLOCK-B's reader wait then misses the second session and gets NotFound from the not-yet-committed slow tier. - All call-sites updated: .contains(&d) → .contains_key(&d), iter() preserves (digest, refcount) tuple for shutdown-time warnings, test manipulations use lock().insert(digest, NonZeroU32::new(1).unwrap()). - Test: chunked_inflight_cancel_safety_test.rs::major_g_refcount_ keeps_digest_visible_until_both_guards_drop. Two guards on the same digest; assert refcount sequencing through 0→1→2→1→0 with entry visibility check at each step. Mutation-verified red-fail with bespoke "refcount removed digest while second guard still in-flight" message. MAJOR-I (H2 doc-comment honesty): - bytestream_server.rs:2742 doc-comment now reflects actual behavior: the H2 fall-through to in_flight_writes only dedups OTHER ByteStream::write callers (v2-chunked sessions don't populate in_flight_writes). For v2-in-flight cases, second writer becomes the primary and races; CAS convergence (digest = content) makes on-disk byte sequence well-defined. The H2 guard is "don't lie about durability" — NOT "serialize against the chunked writer." MAJOR-J (TLA+ test assertion message divergence): - chunked_write_handler_v2_test.rs::tla_all_attached_writers_terminate_ within_70s_v1plus2v2: the v1_size + v2 commit assertion messages now match what mutation of single_stream_owner actually catches ("failed to stat chunked partial ... NotFound" — i.e. v1+v2 racing on the .partial file, NOT a 70s timeout). MAJOR-K (chunked_v2_enabled=false rollback path): - New test chunked_write_handler_v2_test.rs::major_k_chunked_v2_ disabled_returns_unimplemented_for_v2_rpcs. Constructs adapter with v2_enabled=false; asserts v2 RPCs return Unimplemented with "chunked_v2_enabled" in the message; v1 RPC still works. Out-of-scope (file follow-up trackers): - DS MAJOR-1 (latent v1 AsyncCommit code path; gated by bazel_facing_internal_chunking=false default; eliminated by MAJOR-F refactor when the v1 path is enabled). - DS MINOR-1 (chunked_in_flight TTL/sweeper backstop — MAJOR-G's refcount + RAII covers the common case). - code MAJOR-1 (probabilistic metric), code MAJOR-3 (err_tip chaining), code MINOR-1/2/3, auditor MINOR (1-off test count, TLA+ line cite). - red-team TraceMachina#5 (pilot-vs-default-flip — user already decided). - red-team TraceMachina#8 (hand-edited pb.rs — pre-existing TraceMachina#87/TraceMachina#346). - Pre-existing test failure: bazel_facing_internal_chunking_test.rs:: fast_tier_ok_dispatch_err_records_failed_slow_write — the typed-BackpressureSignal early-return at fast_slow_store.rs:1654 (aeaf078) bypasses failed_slow_writes insertion. NOT a regression from this commit; flagged for follow-up tracker. 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
Part A-only of original combined #O1+#A4 commit. Part B (deferring tree-internal BlobsAvailable via tokio::spawn in local_worker.rs) reverted per cadre red-team RECONSIDER-PREMISE: it reverses commit 8dd0a633 (2026-03-12), which added the synchronous expansion specifically because the server returned NotFound for tree-internal blobs until background upload completed (~500ms+). Chesterton's Fence. Part B also widens the SIGKILL loss window. Tracked separately as TraceMachina#87 (Part B follow-up). Part A (#A4 — parallel tree decode, running_actions_manager.rs:4823): Previously, expand_tree_file_digests walked output_folders sequentially, awaiting get_and_decode_digest per tree. For an N-folder action this walled N x per-folder latency on the publish path. Replace with FuturesUnordered; per-folder decodes are independent disk reads + protobuf decodes. Concurrency is bounded by output_folders.len() (ActionResult-level bound, typically O(1..10)). Error semantics preserved -- any decode error is still logged-and- skipped, never failing the whole expansion. Cadre approval: Part A approved independently by the O1+A4 reviewer cadre (code-reviewer, perf-optimizer, security-reviewer, testing-czar, red-team, distributed-systems-reviewer, assumption-auditor). Tests: - expand_tree_file_digests_returns_all_file_node_digests (T1c correctness): N folders, K files each -- every file_node digest must appear in the result. Guards Part A correctness across the sequential->parallel surgery. - expand_tree_file_digests_runs_decodes_concurrently (T1p parallel): asserts parallel wall-clock < sequential sum. Mutation: revert the FuturesUnordered to a sequential for loop -> red-fails with "parallel run wall-clock exceeded sequential-bound -- FuturesUnordered concurrency contract violated." Files touched: nativelink-worker/src/running_actions_manager.rs (line 4823: A4) nativelink-worker/tests/running_actions_manager_test.rs (T1c, T1p)
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
1 participant
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 change is