diff --git a/subprojects/hydra-queue-runner/src/state/build.rs b/subprojects/hydra-queue-runner/src/state/build.rs index f0a688a61..e6fac08a5 100644 --- a/subprojects/hydra-queue-runner/src/state/build.rs +++ b/subprojects/hydra-queue-runner/src/state/build.rs @@ -356,7 +356,7 @@ impl BuildTimings { } } -#[derive(Debug)] +#[derive(Debug, Clone)] pub struct BuildOutput { pub failed: bool, pub timings: BuildTimings, diff --git a/subprojects/hydra-queue-runner/src/state/mod.rs b/subprojects/hydra-queue-runner/src/state/mod.rs index ad8819725..2753f75db 100644 --- a/subprojects/hydra-queue-runner/src/state/mod.rs +++ b/subprojects/hydra-queue-runner/src/state/mod.rs @@ -189,6 +189,14 @@ pub struct State { /// restart. pub resolved_drv_map: parking_lot::RwLock>, + /// Cache mapping a CA derivation path to the `BuildOutput` it produced. + /// CA floating outputs have no statically-known path, so an already-built + /// CA derivation can only be detected via the in-memory `Steps` map, which + /// drops entries once the owning build finishes. A later build resolving to + /// the same derivation would then rebuild it (and lose its output). This + /// cache lets such builds be marked cached instead. + pub resolved_output_cache: parking_lot::RwLock>, + pub fod_checker: Option>, pub started_at: jiff::Timestamp, @@ -292,6 +300,7 @@ impl State { db, machines: Machines::new(), resolved_drv_map: parking_lot::RwLock::new(HashMap::new()), + resolved_output_cache: parking_lot::RwLock::new(HashMap::new()), log_dir, builds: Builds::new(), jobsets: Jobsets::new(), @@ -655,6 +664,26 @@ impl State { // Otherwise, `succeed_step` would prematurely mark the build as // finished when an intermediate resolved step completes. let is_toplevel = build.toplevel.load().as_deref() == Some(&*step_info.step); + + // If this resolved derivation was already built, mark the + // build cached instead of rebuilding. Only safe at toplevel: an + // intermediate resolved step does not finish the build itself. + if is_toplevel { + let cached_output = self + .resolved_output_cache + .read() + .get(&resolved_path) + .cloned(); + if let Some(output) = cached_output { + tracing::info!( + "resolved derivation {resolved_path} already built, marking build {} as cached", + build.id + ); + self.mark_build_cached(build.clone(), output).await?; + return Ok(RealiseStepResult::Resolved); + } + } + let referring_build = if is_toplevel { Some(build.clone()) } else { @@ -2594,8 +2623,47 @@ impl State { } #[tracing::instrument(skip(self, build), fields(build_id=build.id), err)] + /// Mark a build succeeded and cached using a known build output, without + /// re-deriving it from the store. + async fn mark_build_cached( + &self, + build: Arc, + output: BuildOutput, + ) -> Result<(), StateError> { + { + let mut db = self.db.get().await?; + let mut tx = db.begin_transaction().await?; + let now = jiff::Timestamp::now().as_second(); + tx.mark_succeeded_build( + get_mark_build_sccuess_data(&build, &output), + true, + i32::try_from(now)?, + i32::try_from(now)?, + self.pool.store_dir(), + ) + .await?; + self.metrics.nr_builds_done.inc(); + tx.notify_build_finished(build.id, &[]).await?; + tx.commit().await?; + } + build.set_finished_in_db(true); + self.builds.remove_by_id(build.id); + Ok(()) + } + async fn handle_cached_build(&self, build: Arc) -> Result<(), StateError> { - let res = self.get_build_output_cached(&build.drv_path).await?; + // CA floating outputs have no statically-known path, so + // `get_build_output_cached` cannot recover them from the derivation + // alone and would yield an empty output. Prefer a cached output. + let cached = self + .resolved_output_cache + .read() + .get(&build.drv_path) + .cloned(); + let res = match cached { + Some(output) => output, + None => self.get_build_output_cached(&build.drv_path).await?, + }; { let mut db = self.db.get().await?; diff --git a/subprojects/hydra-tests/content-addressed/early-cutoff.t b/subprojects/hydra-tests/content-addressed/early-cutoff.t index d7fb119cb..0e9f5819c 100644 --- a/subprojects/hydra-tests/content-addressed/early-cutoff.t +++ b/subprojects/hydra-tests/content-addressed/early-cutoff.t @@ -13,55 +13,65 @@ use Test2::V0; my $db = $ctx{context}->db(); -my $project = $db->resultset('Projects')->create({name => "tests", displayname => "", owner => "root"}); +# Loop to reproduce the flaky "One downstream build should be cached" race. +my $iterations = $ENV{HYDRA_EARLY_CUTOFF_ITERATIONS} // 10; -my $jobset = createBaseJobset($db, "content-addressed-early-cutoff", "content-addressed-early-cutoff.nix", $ctx{jobsdir}); +for my $iter (1 .. $iterations) { + my $project = $db->resultset('Projects')->create({ + name => "tests-$iter", + displayname => "", + owner => "root", + }); -ok(evalSucceeds($ctx{context}, $jobset), "Evaluating early cutoff jobs should exit with return code 0"); -is(nrQueuedBuildsForJobset($jobset), 4, "Should queue 4 early cutoff builds"); + my $jobset = createBaseJobset( + $db, + "content-addressed-early-cutoff-$iter", + "content-addressed-early-cutoff.nix", + $ctx{jobsdir}, + ); -my @builds = queuedBuildsForJobset($jobset); -ok(runBuilds($ctx{context}, @builds), "Building all early cutoff jobs should exit with code 0"); + ok(evalSucceeds($ctx{context}, $jobset), "[$iter] Evaluating early cutoff jobs should exit with return code 0"); + is(nrQueuedBuildsForJobset($jobset), 4, "[$iter] Should queue 4 early cutoff builds"); -for my $build (@builds) { - my $newbuild = $db->resultset('Builds')->find($build->id); - is($newbuild->finished, 1, "Build '".$build->job."' should be finished."); - is($newbuild->buildstatus, 0, "Build '".$build->job."' should have buildstatus 0."); -} + my @builds = queuedBuildsForJobset($jobset); + ok(runBuilds($ctx{context}, @builds), "[$iter] Building all early cutoff jobs should exit with code 0"); + + for my $build (@builds) { + my $newbuild = $db->resultset('Builds')->find($build->id); + is($newbuild->finished, 1, "[$iter] Build '".$build->job."' should be finished."); + is($newbuild->buildstatus, 0, "[$iter] Build '".$build->job."' should have buildstatus 0."); + } + + # earlyCutoffUpstream1 and earlyCutoffUpstream2 differ but produce the same + # content-addressed output. Building earlyCutoffDownstream1 should let + # earlyCutoffDownstream2 be cached, since its resolved input is identical. + my $upstream1 = $db->resultset('Builds')->find({ jobset_id => $jobset->id, job => "earlyCutoffUpstream1" }); + my $upstream2 = $db->resultset('Builds')->find({ jobset_id => $jobset->id, job => "earlyCutoffUpstream2" }); + + my $upstream1_out = $upstream1->buildoutputs->find({ name => "out" }); + my $upstream2_out = $upstream2->buildoutputs->find({ name => "out" }); + is($upstream1_out->path, $upstream2_out->path, + "[$iter] Both upstream builds should resolve to the same content-addressed output path"); -# Early cutoff: earlyCutoffUpstream1 and earlyCutoffUpstream2 have -# different derivations but produce the same content-addressed output. -# After building earlyCutoffDownstream1, earlyCutoffDownstream2 should -# be cached because its resolved input is identical. -my $upstream1 = $db->resultset('Builds')->find({ - jobset_id => $jobset->id, - job => "earlyCutoffUpstream1", -}); -my $upstream2 = $db->resultset('Builds')->find({ - jobset_id => $jobset->id, - job => "earlyCutoffUpstream2", -}); - -my $upstream1_out = $upstream1->buildoutputs->find({ name => "out" }); -my $upstream2_out = $upstream2->buildoutputs->find({ name => "out" }); -is($upstream1_out->path, $upstream2_out->path, - "Both upstream builds should resolve to the same content-addressed output path"); - -my $downstream1 = $db->resultset('Builds')->find({ - jobset_id => $jobset->id, - job => "earlyCutoffDownstream1", -}); -my $downstream2 = $db->resultset('Builds')->find({ - jobset_id => $jobset->id, - job => "earlyCutoffDownstream2", -}); - -my $downstream1_out = $downstream1->buildoutputs->find({ name => "out" }); -my $downstream2_out = $downstream2->buildoutputs->find({ name => "out" }); -is($downstream1_out->path, $downstream2_out->path, - "Both downstream builds should create the same content-addressed output path"); - -ok($downstream1->iscachedbuild || $downstream2->iscachedbuild, - "One downstream build should be cached"); + my $downstream1 = $db->resultset('Builds')->find({ jobset_id => $jobset->id, job => "earlyCutoffDownstream1" }); + my $downstream2 = $db->resultset('Builds')->find({ jobset_id => $jobset->id, job => "earlyCutoffDownstream2" }); + + my $downstream1_out = $downstream1->buildoutputs->find({ name => "out" }); + my $downstream2_out = $downstream2->buildoutputs->find({ name => "out" }); + is($downstream1_out->path, $downstream2_out->path, + "[$iter] Both downstream builds should create the same content-addressed output path"); + + my $passed = ok($downstream1->iscachedbuild || $downstream2->iscachedbuild, + "[$iter] One downstream build should be cached"); + + unless ($passed) { + for my $b ($upstream1, $upstream2, $downstream1, $downstream2) { + my $out = $b->buildoutputs->find({ name => "out" }); + diag(sprintf("build id=%d job=%s out=%s iscached=%s start=%s stop=%s", + $b->id, $b->job, ($out ? $out->path : ""), + $b->iscachedbuild, $b->starttime // "?", $b->stoptime // "?")); + } + } +} done_testing;