Repository navigation
Conversation
…hina#2826) should_timeout_operation ended an Executing action once Action.timeout had passed since it was assigned. The worker enforces the same timeout, but its clock starts when the command starts, after input fetch, so the scheduler always fired first. The action was requeued onto the worker still running it, which refused with AlreadyExists; the retries ran out, and when the worker's own DEADLINE_EXCEEDED result arrived it was treated as a stray and the worker was disconnected, taking every other action on it down too. The scheduler now enforces Action.timeout in every liveness case, at Action.timeout plus no_event_action_timeout, measured from the later of the assignment and the worker's last update on the action. A live worker reports first, so no liveness special-casing is needed, and the scheduler still ends an action whose worker never reports: a hung input fetch, an unkillable child, a broken timeout_handled_externally wrapper, or an orphan no instance reaps. The client's DEADLINE_EXCEEDED message now names the deadline that fired, instead of always citing no_event_action_timeout. This is the interim: the scheduler cannot observe when the command starts. TraceMachina#2827 tracks the worker reporting command start as an operation transition, so the deadline can be anchored to that event. Co-authored-by: Johnny <[email protected]>
…store, and a missing input names its digest so the client re-uploads it (TraceMachina#2855)
…the worker evicted and idle (TraceMachina#2856) * Worker: no scheduler call awaited in the run loop, a deadline on keepalives and acknowledgements that reconnects, and HTTP/2 keepalive on the scheduler channel * Worker: the GoingAway calls on shutdown carry the same deadline * Worker: the layout CI's rustfmt asks for * Keep the scheduler stream flowing: ordered processing off the reader, no deadlines on stream messages * Qualify Code in the worker API server test * Box the action future before the launch closure * Box the action chain inside the action closure * Raise the binary's recursion limit for the worker's action future * Use try_update for the filesystem store generation counter
…a#2859) Co-authored-by: config reference bot <[email protected]>
…aceMachina#2848) Co-authored-by: llms bot <[email protected]>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
* chore(deps): update rust crate serial_test to v4 * Update MODULE.bazel.lock
…lane Applies PR TraceMachina#2864's behavior (disk cache off when NativeLink Cloud remote caching is active) WITHOUT TraceMachina#2871's unrelated 'ci-mode || cache' default flip, and with one refinement: the Bazel 8.7.0 compatibility lane keeps its keyed disk cache, because the remote cache is Bazel-9-keyed and only covers 257/3740 of its actions (vs 2273/3915 on the Bazel 9 lane). Measured on upstream runners (same commit family, warm remote cache): - Redis store tester: 28m03s -> 4m52s once the ~17.9 GB disk-cache restore (12m03s) and save (8m26s) are gone. before: https://github.com/TraceMachina/nativelink/actions/runs/37028361292/job/110908644772 after: https://github.com/TraceMachina/nativelink/actions/runs/37051289151/job/110985034234 Co-Authored-By: Claude Opus 4.8 <[email protected]>
This comment has been minimized.
This comment has been minimized.
The Bazel 8 lane runs with --lockfile_mode=off and cannot use the Bazel-9-keyed remote cache effectively, so it is the slowest lane in the Bazel Native workflow (~650s cold vs ~380s for the Bazel 9 lane). It only validates that the sources still build under the older Bazel, so a pure code PR that does not touch the module graph, bazelrc, BUILD files, toolchains, or the workflow itself cannot change its answer. Gate it on those paths via dorny/paths-filter; pushes to main always run the full matrix as a post-merge backstop, so nothing lands unvalidated. Co-Authored-By: Claude Opus 4.8 <[email protected]>
generate-rbe-commands only needs the list of toolchain example lanes, but it enumerated them via rbe-toolchain-with-nativelink-test, which depends on (and rebuilds) nativelink. Add a discovery-only flake target with no nativelink dependency and point the matrix step at it: the discovery job drops from 124s to 93s and nativelink now builds once per shard instead of once extra for discovery. The key list must stay in sync with the COMMANDS table in rbe-toolchain-test.nix; a mismatch only changes which shards fan out and the lanes re-validate, so it fails safe. Co-Authored-By: Claude Opus 4.8 <[email protected]>
b7r6
force-pushed
the
ci/overhaul-honest
branch
2 times, most recently
from
October 2, 2026 20:28
1c4a30c to
98aba23
Compare
The lanes floated on `stable`, which drifts ahead of the crate's rust-version and turns new deprecation lints into hard build errors: as of 2026-10-02 both the ubuntu and windows lanes hard-error on an unmodified workflow because current stable deprecates atomic fetch_update in favor of try_update (with -Dwarnings in effect). Pin the toolchain to the committed MSRV, which is the compatibility contract the crate actually makes. Correctness fix, not a speedup: turns two red lanes green and keeps them deterministic. Co-Authored-By: Claude Opus 4.8 <[email protected]>
Judgment call, deliberately split into its own commit so it is one revert away: per-PR CI optimizes the contributor feedback loop and relies on the Bazel lanes (same sources, remote-cached) for pre-merge correctness, while the Cargo build system — including the ~15-minute Windows native-compile pole, the only lane with no Bazel/Nix path — is re-proven on every push to main, on a daily cron, and on demand via workflow_dispatch. Cargo Native is the sole cargo-only workflow: the Sanitizers lane is Bazel-based (bazel test --config=asan) and stays per-PR. Tradeoff stated plainly: a cargo-only break (Cargo.toml/lock drift, cargo-only cfg paths, Windows toolchain breakage) now reaches main before it is detected, instead of failing on the PR. If main must never redden, a merge queue (merge_group) running these lanes pre-merge is the stricter hardening and the recommended follow-up. If the maintainers prefer keeping Cargo per-PR, dropping this commit leaves the rest of the series intact. Co-Authored-By: Claude Opus 4.8 <[email protected]>
b7r6
force-pushed
the
ci/overhaul-honest
branch
from
October 2, 2026 20:30
98aba23 to
881efa9
Compare
Owner
Author
|
Closing fork-only measurement/experiment PR; superseded by the single clean PR on ci/bazel-dev-warming (#25). |
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
CI overhaul in five commits: four objective, measured wins, plus one clearly-flagged opinionated change about per-PR CI philosophy that is deliberately its own commit and one revert away. Config-only — no Rust, no store, no scheduler code changes, and no change to
ci-modedefaults or cache-write behavior (PRs stay read-only against the remote cache, so cache poisoning exposure is unchanged).Headline (Tier 1, objective): dropping the ~17.9 GB Bazel disk cache where the NativeLink remote cache already covers the lane takes the Redis store tester from 28m03s to 4m52s (-83%) on upstream runners, same tests, same remote-hit profile. This applies the behavior of @palfrey's TraceMachina#2864 (full credit — the disk-cache-off-when-remote-is-active idea and the original patch are his) without TraceMachina#2871's unrelated
ci-mode || 'cache'default flip, and with one refinement: the Bazel 8.7.0 lane keeps its keyed disk cache, because the remote cache is Bazel-9-keyed and only covers 257/3740 of its actions (vs 2273/3915 on the Bazel 9 lane).Tier 1 — objective, stands on its own:
ci: disable oversized Bazel disk cache where remote cache covers the lane— removes measured pure transfer waste (12m03s restore + 8m26s save), identical coverage.ci: gate the Bazel 8.7.0 compatibility lane on Bazel-affecting paths— code-only PRs skip the slowest Bazel lane; anything touching MODULE/BUILD/bzl/bazelrc/toolchains/workflows still runs it, and every push to main runs the full matrix as a post-merge backstop, so nothing merges unvalidated. The lane selection lives inside an always-running job (dynamic matrix), so a skipped lane can never leave a required check pending.ci(nix): split rbe-toolchain discovery from the nativelink build— Tom's suggestion ([fork-only test] toolchain diff vs affected lanes #2); discovery no longer rebuilds nativelink just to list lanes.ci(cargo): pin the native-cargo lanes to the committed MSRV (1.97.1)— correctness: floatingstable+-Dwarningsturns new deprecation lints into hard errors (today:fetch_update→try_update), reddening both lanes for reasons unrelated to the change under test.Tier 2 — opinionated, severable:
ci(opinionated): move the Cargo Native lanes off the per-PR path to main— this is a judgment call and we say so. It optimizes the per-PR loop for contributor velocity on the premise that the Bazel lanes (same sources, remote-cached) provide pre-merge correctness coverage, and accepts that a cargo-only break (Cargo.toml/lock drift, cargo-only cfg paths, Windows toolchain) now reaches main before detection, caught by the push-to-main run (plus a daily cron andworkflow_dispatch). It takes the ~16m Windows native-compile pole — the only lane with no Bazel/Nix path — off every PR push. The stricter variant that keeps main always green is a merge queue (merge_group) running these lanes pre-merge; we'd recommend that as the hardening if this direction is accepted. If you'd rather keep Cargo per-PR, drop this one commit and Tier 1 stands alone. Your repo, your call.How was this verified?
Every number below is a clickable run; cold vs warm is labeled. (The before/after pairs were measured on the single-change experiment branches; the combined branch was then re-verified green on the fork.)
generate-rbe-commandsstable+-Dwarnings: ubuntu, windows (fetch_update→try_updatedeprecation)Combined-branch verification on the fork (all five commits together):
Compute affected Bazel lanessucceeds and the resulting unit-tests matrix is exactly[Redis store tester, ubuntu-24.04]— theubuntu-24.04 / Bazel 8.7.0lane is absent; run https://github.com/b7r6/nativelink/actions/runs/37061870315).rbe-toolchain-test-listflake target was built and executed locally (nix run .#rbe-toolchain-test-list→ the ten-lane JSON array) and in the fork's Nix workflow.actionlintis clean on the modified workflows (one pre-existing warning on main is unchanged).Known honest gaps:
Risk
ci-modesemantics change; PRs remain read-only against the remote cache.rbe-toolchain-test.nix; a mismatch only changes which shards fan out and the shards re-validate, so it fails safe.rust-version(they already require a deliberate change).AI assistance
Claude (Opus 4.8) agents drafted the workflow changes, ran the fork measurement matrix, and wrote this description; a human (b7r6) directed the experiments, reviewed every line, and is answerable for all of it. Each commit also carries a Co-Authored-By trailer. The disk-cache idea and original patch are @palfrey's (TraceMachina#2864); this PR carries his behavior forward with the Bazel-8 refinement and without TraceMachina#2871's default flip.