Merge master into nix-next#1717
Open
github-actions[bot] wants to merge 254 commits into
Open
Conversation
hydra-tests: Extract `ProcessGroup` and shared test helpers
Apply [semantic line breaks](https://sembr.org/) (one sentence per line) across all markdown files in the manual and repository. This makes diffs easier to review since changing one sentence no longer reflows an entire paragraph. Also add `CONTRIBUTING.md` documenting the convention, and clean up various pandoc artifacts along the way: escaped quotes (`\'`), escaped underscores (`\_`), broken multi-line list items, and non-breaking spaces.
Reformat markdown to use semantic line breaks
Forgot to do this when bumping harmonia. The commented stuff will return when we merge `nix-next`.
`buildproducts.path` can contain a sub-path below a store output (e.g. `/nix/store/…-nix-manual-2.31.4/share/doc/nix/manual`), but `OwnedBuildProduct::parse_paths` was calling `StoreDir::parse` which only accepts bare store paths. Any path with a `/` after the store path name failed with `invalid store path / symbol`, breaking the cached-build fast path for those builds. Move `RelativeStorePath` from the queue-runner into a new `store-path-utils` crate so the DB crate can use it directly. Change `OwnedBuildProduct`'s default type parameter from `StorePath` to `RelativeStorePath` and have `parse_path()` call `RelativeStorePath::from_path`, which correctly splits the full path into a `StorePath` base and a relative suffix.
When `create_step` encounters an existing step (`is_new=false`), it now re-checks whether the step's outputs have appeared in the local nix store since the step was first created. Without this check, builds whose outputs became available between poll cycles (e.g. via concurrent builds, substitution, or external upload) would get stuck in an infinite re-load loop: the DB says `finished=0`, the step already exists in memory so `create_build` inserts the build but never routes it to `handle_cached_build`. The fix adds two checks before returning `Valid(step)`: 1. If the step is already marked finished, return `None` immediately. 2. Query missing outputs from the local store; if all are present, mark the step finished, insert into `finished_drvs`, and return `None` so `create_build` routes to `handle_cached_build`.
Nix writes `Compression: bzip2` in narinfo files (see `src/libstore/nar-info.cc`), but hydra's parser only accepted `bz2` (the file extension, not the compression name). This caused errors like `invalid value for Compression: bzip2` when reading old narinfo entries from `cache.nixos.org`, making the queue runner treat those store paths as missing and rebuild them. Fix `FromStr` to accept `bzip2`, and fix `as_str` to write `bzip2` (matching nix). The file extension method `ext()` correctly uses `.nar.bz2` and is unchanged. Examples of affected narinfo files: - https://cache.nixos.org/2crhfrb8pqy61d32jaajpw33jjmny6xz.narinfo - https://cache.nixos.org/9fggf2bx5p29da89jrbmdh2fcd68m1mx.narinfo - https://cache.nixos.org/0xs59grdbgz7z2ci2ff4yfwb6jha1qaa.narinfo
The architecture doc was sitting outside `src/`, presumably because it was mostly notes and not yet cleaned up enough to be part of the user-facing docs. We will do that clean up next, but before we do that, move the file and expose it, so the subsequent diff is readable and not confused by the move.
- Organize components by deployment topology: master machine vs builder machines vs destination store, reflecting which components share a Nix store and which don't - Add `hydra-builder`, `hydra-notify`, `hydra-eval-jobset` — components that weren't documented before - Update `hydra-queue-runner` language from C++ to Rust - Add source layout and build system overview - Add database schema section with pointers to the SQL and migration docs - Update FAQ: both Nix and Hydra now have experimental dynamic derivations - Fix typos (`hydra-build-producs`, `obsolute`)
Add updated architecture section to the manual
Pick up latest changes from nix-community/harmonia.
Both `hydra-builder` and `hydra-queue-runner` had near-identical `build.rs` files that compiled the same `.proto` file and generated the same version constant. This consolidates that into a single `hydra-proto` crate with feature flags: - `client` — generates tonic client stubs (used by `hydra-builder`) - `server` — generates tonic server traits (used by `hydra-queue-runner`) - `db` — `From` impls bridging proto enums to `db::models` types Proto ↔ native type conversions are also consolidated here: - `ProtoStorePath` moved from `shared::proto`, now depends on `harmonia-store-core` directly rather than `nix-utils` - `Pressure`/`PressureState` — both components now use the generated proto types directly, eliminating duplicate structs and field-by-field conversion boilerplate - `PresignedUploadOpts`, `ConfigUpdate` — queue-runner now uses the proto types directly instead of maintaining trivial local wrappers - `BuildResultState` — restructured as a nested enum wrapping `hydra_proto::BuildResultState` with extra `Aborted`/`Cancelled` variants for queue-runner-internal states - `RelativeStorePath` — new proto message type making the store path vs sub-path split explicit on the wire; `From`/`TryFrom` conversions in `hydra-proto` bridge to `store_path_utils::RelativeStorePath` - `BuildProduct.path` changed from opaque `string` to structured `RelativeStorePath` message Also: - Groups all local workspace crates under `[workspace.dependencies]` with `workspace = true` references instead of relative paths - Updates the architecture doc with descriptions of each shared crate
Extract hydra-proto crate for shared `gRPC` code generation
Introduces `store.proto` (`nix.store.v1` package) containing types that are generic to any Nix store, not specific to Hydra: - `StorePath`, `StorePaths`, `RelativeStorePath` - `ValidPathInfo` — store path with NAR hash/size, references, deriver, CA - `NarInfo` — wraps `ValidPathInfo` with cache-specific fields (url, compression, file hash/size) `streaming.proto` (`runner.v1`) imports these and uses them for `BuildMessage.drv`, `BuildProduct.path`, `PresignedUploadComplete`, etc. `PresignedUploadComplete` is restructured to contain a `NarInfo` message instead of flattening all fields, making the layering explicit.
Split `.proto` into generic Nix store types and Hydra-specific types
The `binary-cache` crate had its own `NarInfo` struct that duplicated
what harmonia now provides as `harmonia_store_nar_info::NarInfo`. This
replaces it with the upstream type, which also brings in
`harmonia-store-path-info` for `ValidPathInfo`/`UnkeyedValidPathInfo`.
The local struct was flat; the harmonia type is nested:
`NarInfo { path, info: UnkeyedNarInfo { info: UnkeyedValidPathInfo, url, compression, ... } }`.
All construction and field access sites are updated accordingly.
`From<harmonia_store_nar_info::NarInfo>` and `TryFrom<proto::NarInfo>`
conversions are added in the proto crate so both components can convert
between native and wire representations without manual field-by-field
marshalling. Round-trip unit tests verify the conversion is lossless.
Also adds `store_dir` field to proto `ValidPathInfo` to match the
harmonia type. The queue-runner now asserts the store dir matches
rather than silently overriding it.
Replace local `NarInfo` with `harmonia-store-nar-info`
Consumers now import `StorePath`, `StoreDir`, `OutputName`, etc. directly from `harmonia-store-core` rather than through `nix-utils` re-exports. This makes the actual dependency on harmonia explicit at each use site. `nix-utils` retains its own local functionality (`parse_store_path`, `BaseStore`, `LocalStore`, `realise_drv`, etc.) but no longer acts as a forwarding layer for harmonia types.
Remove harmonia re-exports from `nix-utils`
Replace stringly-typed fields in `store.proto` with proper message types: - `Hash` — algorithm enum + raw digest bytes - `Signature` — key name + signature string - `ContentAddress` — method enum + hash `UnkeyedValidPathInfo.nar_hash` is now `Hash` (not string), `signatures` is `repeated Signature` (not `repeated string`), `ca` is `optional ContentAddress` (not `optional string`). `UnkeyedNarInfo.file_hash`/`file_size` renamed to `download_hash`/`download_size` with proper `Hash` type. All `From`/`TryFrom` impls use `&` for harmonia→proto direction (no need to consume the source) and by-value for proto→harmonia (proto types are consumed). Round-trip tests cover every layer.
Use structured proto types for `Hash`, `Signature`, and `ContentAddress`
Move each test from the monolithic `nixos-tests.nix` into its own file under `nixos-tests/`, with shared VM configuration extracted to `common.nix`. No functional changes to existing tests.
Split NixOS tests into separate files
Tests the full Hydra pipeline against a real S3-compatible store (garage): evaluate a jobset via the web API, build it, and verify that outputs are uploaded to the S3 bucket. Currently checks NAR listings (`.ls` files): the test builds a derivation that produces regular files, an executable, a symlink, and a subdirectory, then does an exact JSON comparison of the uploaded listing against the expected structure. Uses `services.garage` NixOS module on a dedicated `s3` VM, and `POST /api/push` to trigger evaluation explicitly rather than relying on `checkinterval` polling.
Add NixOS end-to-end S3 integration test
Replace the `list_nar_deep` FFI call (which used Nix's C++ `getFSAccessor` and `listNarDeep`) with harmonia's pure-Rust `parse_nar_listing`. The new `nar_listing()` helper streams NAR bytes from `nar_from_path` through `harmonia-file-nar`'s parser, producing a `FileTree<NarFileInfo>` that serializes to the same JSON format. This removes the last use of `list_nar_deep` from `binary-cache`, so the FFI function and its `BaseStore` trait method are deleted along with the C++ implementation and the `list_nar` example. Also adapts `parse_drv` to take `&[u8]` instead of `&str`, matching upstream harmonia's `parse_derivation_aterm` signature change. The previous commit's NixOS test ensures we get this right.
Use harmonia for NAR listings instead of Nix C++ FFI
The presence cache used the default rollback journal with synchronous=FULL, so every write fsynced, which on ZFS is a ZIL commit and dominated ingestion time. It is only a cache, so open it in WAL mode with synchronous=NORMAL and trade the per-write sync for durability.
Ideally we could switch soon to a store that queue-runner owns itself that is not shared with the system, as we are now bottlenecked again on daemon interactions. But that is a bigger change. For now, restore some of the performance lost when switching store reads from SQLite to the nix-daemon: create_step opens a single daemon connection and uses it for all of a step's output-validity checks rather than handshaking per output. The connection lives on the stack for that step and closes on return, so nothing is held open between ingestion passes.
Build with swc:
`swc compile ./ansi_up.ts --config-json '{"module": {"type": "umd"}, "minify": true, "jsc": {"parser": {"syntax": "typescript"}}}' --out-file ansi_up.js`
hydra-builder pins each build's inputs with AddTempRoot on a per-build daemon connection. Nothing exercised that, so a regression to the pinning would only show up as the production-only "input missing" flake from #1806. Add a test that runs nix-store --gc on the builder store while a build is inside its builder script and asserts the input survives and the build succeeds. The build handshakes with the test through two sentinel files (sandbox is off), so the GC point is deterministic. The input is referenced only via args so it stays out of /proc/*/environ, which the GC scans for runtime roots and would otherwise mask a broken pin.
request_build builds the derivation on its own daemon connection. The daemon roots a build's outputs via the connection that ran it, so dropping that connection releases them. Outputs were only re-pinned onto the persistent roots connection after request_build returned, so they were rooted by nothing in between. A GC in that window deletes the outputs, and the following query_path_info and upload fail with "path is not valid". This does not depend on the daemon honoring temp roots added mid-collection: the outputs are simply unrooted for the duration of the gap. Pin the outputs onto the roots connection before dropping the build connection.
Services log to journald, not a terminal, so the ANSI escape codes were noise and the coloring was wasted work.
The dispatcher calls get_required_features per runnable step each cycle, so cloning the feature Vec churned the allocator. Expose the StepDrvInfo Arc and let callers borrow the slice.
create_build and create_step threaded a growing list of shared Arcs. Gather the pass-wide state into one InjectCtx and split the GC-abort, dependency injection, missing-output and substitution logic into helpers. No behaviour change.
create_step opened a daemon connection per step for output-validity reads, and the recursion fans out, so an ingestion burst opened thousands. The daemon forks a worker per connection on a single accept thread, hammering one nix-daemon. Carry a per-pass DaemonConnPool in the ingestion context that caps live connections and reuses them, so a pass costs a fixed number of handshakes instead of one per step. The pool drops when the pass returns. A connection left desynced by a failed query is dropped rather than returned.
Profiling showed propagate_priorities at ~37% of CPU in the queue monitor loop, ~25% of all samples in StorePath hashing. The walk dedups visited steps with HashSet<Arc<Step>>, hashing each Step by its drv_path (20-byte hash plus name string) on every dependency edge. The Steps registry only yields one Arc<Step> per derivation, so pointer identity matches content identity. Key the set by Arc pointer instead, reducing each membership check to a usize hash. ByPtr holds the Arc rather than a raw pointer to pin the address for the walk; a freed step's address could otherwise be reused by a concurrently created step and cause a false "already visited" hit.
add_root() created a gcroot with a bare symlink() and was never called. A bare symlink can land after a running collector has scanned the gcroots directory, so the rooted output races that collection and can be deleted. Route rooting through the daemon's addPermRoot, which pins the path with a temp root (synchronising with a running GC over its socket), writes the symlink atomically and registers the indirect root, under the GC lock. Call it on build success, but only without presigned uploads: with them the builder pushes NARs straight to the remote cache and downstream builders fetch inputs from there, so outputs need not live locally.
Ordering by step numer is local to a build closure and jumps around a lot.
complete_build ran the finalize in a detached task and acked the builder immediately. If that task failed, the error was only logged: remove_job never ran, so the machine slot stayed allocated until the next restart. On a long-running queue runner these dead slots accumulate on idle builders. Run the finalize inline and return a non-OK status on failure. The builder retries complete_build, so a failed finalize reschedules instead of leaking the slot.
acquire() used sqlx's 30s default and surfaced a momentary pool spike as a hard error, failing callers on the critical path (build finalization). Shorten the per-attempt timeout and retry a bounded number of times so a spike is waited out; only sustained exhaustion fails. Other errors return immediately.
create_step recurses with per-level buffered concurrency, so a deep build graph fans out into many more in-flight prefetches than one level, each acquiring a DB connection. A large graph drains the pool and starves build finalization, which then times out on acquire. Cap prefetches with a semaphore sized to a quarter of the pool. The permit is released before create_step recurses, so it cannot deadlock.
The queue runner pins non-presigned build outputs with a GC root until a dependent build consumes them. This test simulates the output going missing, so nix-store --delete hits that live root and fails. Delete with --ignore-liveness, matching the test's intent of forcefully removing it.
The evaluator updates builds rows in a large transaction while the queue runner marks builds finished, so Postgres occasionally aborts one side of the lock cycle. The evaluator already retries; the queue runner did not, so a completion that lost the deadlock was rolled back and the build stayed unfinished. Add retry_serialization_failures in the db crate (retries on 40001/40P01) and wrap the queue runner's build-finishing transactions in it.
The evaluator held Builds row locks for the entire evaluation (observed ~13 minutes on nixpkgs) because it repointed builds at their new derivation inline in the large evaluation transaction. That lock window deadlocked the queue runner finishing builds of the same jobset, and a contended jobset could exhaust the retry budget and lose the evaluation. Defer the repoint: record which builds need it and apply the updates after the evaluation transaction commits, in a short id-ordered transaction with its own deadlock retry. The evaluation transaction no longer writes existing Builds rows.
succeed_step and fail_step update several build rows per transaction but iterated them in arbitrary order (Vec in weak-ref order, HashSet). Two concurrent completions with overlapping build sets could lock the same rows in opposite orders and deadlock. Sort both by build id so completion transactions lock rows in a consistent order.
get_jobset_build_steps wrapped stopTime in to_timestamp() before
comparing against the time window. The planner cannot use
indexbuildstepsonstoptime as a range bound on a transformed value, so it
scans the whole index. On nixpkgs jobsets this took over a minute and
blocked queue-runner startup.
Compare stopTime directly against an integer epoch so the index becomes
a range scan.
Before:
Parallel Index Scan ... indexbuildstepsonstoptime
Filter: to_timestamp(stoptime) > now() - '240:00:00'
Rows Removed by Filter: 57685544
Execution Time: 74536 ms
After:
Parallel Index Scan ... indexbuildstepsonstoptime
Index Cond: stoptime > EXTRACT(epoch FROM now())::bigint - 864000
Execution Time: 2204 ms
Also drop the accidental "* 10" on the scheduling window: those extra 9
days are pruned back to SCHEDULING_WINDOW anyway.
"starting dispatch", "create_step: ... already finished ... skipping", and "step ... is now runnable" fire for every dispatch loop and every step scanned, flooding the journal on large jobsets. They are routine scan progress, not notable events, so log them at debug.
Pulls in the harmonia-store-remote change that logs the daemon handshake at debug instead of info, which the queue runner triggered on every RPC.
The daemon's error_msg appends the last N build-log lines and a "For full logs" footer. Hydra already stores and links the full log, so this only bloated the errormsg shown in the web UI. Build the errormsg from the FailureStatus instead.
This makes nixos-unstable with the many big test closures progress very slowly.
The client options sent "max-log-size", which is not a Nix setting name (the setting is max-build-log-size, alias build-max-log-size). The daemon ignores unknown settings, so the log size limit was never enforced.
latestbuilds/nrbuilds warn "Use of uninitialized value" whenever the optional project/jobset/job/system parameters are omitted. Check definedness before comparing against the empty string.
Cover meta.maxSilent, meta.timeout, maxLogSize and maxOutputSize through the full evaluator -> queue-runner -> builder -> daemon path, and verify that a job's meta.maxSilent overrides the queue-runner's maxSilentTime default instead of being capped by it.
hydra-eval-jobset baked 7200/36000 into every build without meta.maxSilent/meta.timeout, so the queue-runner's maxSilentTime and buildTimeout settings never applied to top-level jobs. Store NULL instead and fall back to the queue-runner's configured defaults.
Hydra.nixos.org: fixes from nixos build infra
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
10 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Automated PR to keep
nix-nextin sync withmaster.