queue-runner: resolve CA derivations after dependency outputs land by amaanq · Pull Request #1629 · NixOS/hydra

amaanq · 2026-03-30T22:09:16Z

Instead of resolving at dispatch time and carrying two drv identities through the gRPC layer, resolve in succeed_step once outputs are in the DB. The resolved drv becomes a fresh Step that dispatch picks up normally; the builder only ever sees one drv. A new resolvedFromBuild/resolvedFromStep foreign key BuildSteps ties the resolved step back to the dependency that triggered it.

Ericson2314

At the point where we try_resolve before, we do this:

If result is None or "same derivation":

just continue with this step, same as we always did
If result is Some(drv) and that drv is not the same as the current drv:

Step finished, status "resolved", fill in the DB pointer to the new step, and the new step now builds, this step is done.

the current "try resolve reverse dependencies" is a possible optimisation --- incrementally resolve reverse dependencies, reminiscent of union find --- but that's a more complex thing we shouldn't worry about at this time.

artemist · 2026-04-06T19:20:51Z

I think I have this working, but there are a few things I'm unclear on and would like to figure out:

Do the BuildSteps entries we create in the database properly map to the Step objects in the queue-runner, and do they have sufficient information to recover state after a restart and be shown in the UI?
How should we handle cached failures, and what is actually needed?
Are we doing any double-building, and how can we test that?

I also believe my change needs substantial refactoring, as I'm using only parts of several functions (e.g. create_step)

artemist · 2026-04-07T18:49:58Z

Rebased onto #1642

Ericson2314 · 2026-04-07T20:37:12Z

+            } else {
+                None
+            }
+        };
+
+        if let Some(resolved_path) = resolved {


I think we can combine this with the previous branch? Or at least we could compute a "do we want to resolve" bool, and then have the actual resolving (which much succeed) and this stuff in the same branch.

This is doable, though it will take out a lock on the step's derivation for longer than strictly necessary.

Ericson2314 · 2026-04-07T20:41:12Z

+            resolved_result.set_start_time_now();
+            resolved_result.set_stop_time_now();
+            resolved_result.log_file.clone_from(&job.result.log_file);
+            finish_build_step(


Let's try to do everything in one transaction here. finish_build_step current creates its own transaction, so let's make take the transaction as an argument instead.

Ericson2314 · 2026-04-07T20:45:06Z

+
+            tx.set_resolved_to(build_id, step_nr, nr).await?;
+            tx.commit().await?;
+


TODO think about restart resiliency. I believe everything else after this point in this branch doesn't use the database. So that's good, we aren't logically splitting a transaction in two (assuming the previous stuff is done).

When is left to think about is ---- if the queue runner was restarted at this point, would the general "rebuild state machine from build database state" logic get us into the same step as what this code right here does?

We could make a variation of our CA tests where where the queue runner dies here and then restarts to test this

Unfortunately it looks like hydra-queue-runner is making duplicate BuildStep rows in the database on restart. I'm unsure if this also occurs for non-CA derivations, but it should be fixed. Restart currently doesn't fail, but it leaves the DB in a not very clean state.

Oh, that may be an issue with CA builds, since in create_step we assume that all CAFloating derivations are unfinished, instead of checking if the outputs exist.

It is still true that the queue runner does not attempt to load steps from the database on restart, it recalculates them.

Ericson2314 · 2026-04-07T20:47:25Z

+                    Arc::new(parking_lot::RwLock::new(HashSet::new())),
+                    Arc::new(parking_lot::RwLock::new(HashSet::new())),
+                    Arc::new(parking_lot::RwLock::new(HashSet::new())),


We should have a big fat comment why it is correct to not give this function the mutable state it expects

Comment made

Ericson2314 · 2026-04-07T20:52:40Z

+            // If we're the root of a build then we need to become the new root,
+            // lest the Step get garbage collected.


Comment should be clear this is about Arc keeping things alive, not store keeping ,drv files alive.

We should also be clear that in the non-root case, the idea is that we registered some reverse des above that will keep us alive. Maybe we should assert that we did in fact go through that loop.

We will need to figure out how to keep the .drv file alive for all resolved derivations, and the GC roots that nix-eval-jobs made will not do the job since these are new drvs.

Comment should be done

Ericson2314 · 2026-04-07T21:03:26Z

    foreign key   (build) references Builds(id) on delete cascade,
-    foreign key   (propagatedFrom) references Builds(id) on delete cascade
+    foreign key   (propagatedFrom) references Builds(id) on delete cascade,
+    foreign key   (build, resolvedToStep) references BuildSteps(build, stepnr) on delete cascade


So long term, when the database distinguishes derivations from build attempts, this should actually point to a derivation, not another build attempt.

A build attempt samples the state of the database at that point to resolve, and creates a new (resolved) derivation. That derivation can be attempted to be built multiple times (We don't really care to privilage the first attempt with a foreign key). However, subsequent attempts to build the original derivation --- if we rebuild other upstream derivations and they are content-addressed and non-deterministic --- will result in a different resolved derivations, and that too can be attempted to build multiple times.

Memoizing realization in this way does give us something close a derived build trace, but the fact that our build trace is thus keyed off attempts not not derivations gives us some ability to represent (and not choke on) incoherence.

Ericson2314 · 2026-04-14T01:08:16Z

Unfortunately, once #1667 is merged in, this PR will hang.

Instead of resolving at StepInfo construction and carrying two drv identities through the gRPC layer, resolve in realise_drv_on_valid_machine once all deps are built. If resolution yields a different drv, the original step is marked Resolved and a new DB step is created for the resolved drv with a resolvedTo FK linking them. The builder only ever sees one drv. We create a new Step for that resolution and bunt it back to the scheduler. This grants us more flexibility in execution and the method can be used in the future for dynamic derivations, which won't map 1:1 with the original derivations. We also now only create database steps when we are sure they are necessary, reducing the number of duplicate `BuildStep` rows. In order to make tests more consistent, CA derivations will fail if they cannot be fully resolved. Otherwise, there could be inconsistent successes depending on which builder a step was performed on. As part of this, add local outputs to resolution table With the current queue-runner design, all dependency outputs of a CAFloating derivation must be recorded in the hydra database. This is true for things built or substituted by hydra, but until now not by things found on the local nix store. This may occur for outputs that are part of the system configuration. Therefore, add all local outputs that are not already in the database to the resolution table upon creating a step. This makes it possible to build derivations from `contentAddressedByDefault` nixpkgs. Co-Authored-By: Artemis Tosini <artemis.tosini@obsidian.systems> Co-Authored-By: John Ericson <John.Ericson@Obsidian.Systems> Co-Authored-By: Amaan Qureshi <git@amaanq.com>

Ericson2314 reviewed Mar 30, 2026

View reviewed changes

Comment thread subprojects/hydra/sql/hydra.sql Outdated

Ericson2314 requested changes Mar 30, 2026

View reviewed changes

amaanq force-pushed the ca-resolve-two-phase branch 2 times, most recently from ae692ac to 05eef39 Compare March 31, 2026 03:10

Ericson2314 reviewed Mar 31, 2026

View reviewed changes

Comment thread subprojects/crates/db/src/models.rs

Ericson2314 reviewed Mar 31, 2026

View reviewed changes

Comment thread subprojects/hydra/sql/hydra.sql Outdated

Ericson2314 reviewed Mar 31, 2026

View reviewed changes

Comment thread subprojects/hydra-queue-runner/src/state/mod.rs Outdated

Ericson2314 reviewed Mar 31, 2026

View reviewed changes

Comment thread subprojects/hydra-queue-runner/src/state/mod.rs Outdated

Ericson2314 force-pushed the ca-resolve-two-phase branch from 05eef39 to de9aed8 Compare March 31, 2026 18:45

artemist force-pushed the ca-resolve-two-phase branch 6 times, most recently from 0768499 to 80430bf Compare April 6, 2026 18:37

artemist force-pushed the ca-resolve-two-phase branch 2 times, most recently from 3c7b586 to 7eb2319 Compare April 7, 2026 18:49

artemist force-pushed the ca-resolve-two-phase branch from 7eb2319 to 62abe6f Compare April 7, 2026 20:03

Ericson2314 reviewed Apr 7, 2026

View reviewed changes

Comment thread subprojects/hydra-queue-runner/src/state/mod.rs

Ericson2314 reviewed Apr 7, 2026

View reviewed changes

Comment thread subprojects/hydra-queue-runner/src/state/mod.rs Outdated

Ericson2314 reviewed Apr 7, 2026

View reviewed changes

Comment thread subprojects/hydra-queue-runner/src/state/mod.rs Outdated

Ericson2314 reviewed Apr 7, 2026

View reviewed changes

Ericson2314 mentioned this pull request Apr 7, 2026

Don't pass resolved_drv to the builder #1620

Closed

artemist force-pushed the ca-resolve-two-phase branch 5 times, most recently from 4be2bbb to 0344518 Compare April 13, 2026 14:31

Ericson2314 force-pushed the ca-resolve-two-phase branch 2 times, most recently from 1f18bcf to f040eaa Compare April 14, 2026 12:15

artemist mentioned this pull request Apr 14, 2026

crates/db: Use harmonia types for store paths #1670

Merged

artemist force-pushed the ca-resolve-two-phase branch 4 times, most recently from 676817a to 90af869 Compare April 14, 2026 20:20

artemist force-pushed the ca-resolve-two-phase branch from 90af869 to 3e6dae1 Compare April 16, 2026 14:52


		tx.set_resolved_to(build_id, step_nr, nr).await?;
		tx.commit().await?;

		// If we're the root of a build then we need to become the new root,
		// lest the Step get garbage collected.

Uh oh!

Conversation

amaanq commented Mar 30, 2026

Uh oh!

Uh oh!

Ericson2314 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

artemist commented Apr 6, 2026

Uh oh!

artemist commented Apr 7, 2026

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Ericson2314 Apr 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Ericson2314 commented Apr 14, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Ericson2314 left a comment •

edited

Loading

Ericson2314 Apr 7, 2026 •

edited

Loading