about summary refs log tree commit diff
diff options
context:
space:
mode:
authorsterni <sternenseemann@systemli.org>2024-03-10T17·05+0100
committerclbot <clbot@tvl.fyi>2024-04-16T16·16+0000
commit8fed8982b75d658b620447fc02bccab3bfa0cde6 (patch)
tree0c996c54660346c3ebd2476eaa14dbdce1db315e
parentdce1a7480c283e5298f5f8ecdb1df95f164c7576 (diff)
feat(nix/buildkite): reflect deps between derivations in pipelines r/7940
Most of the steps in our buildkite pipeline build derivations without
doing anything else. A lot of those derivations depend on each other.
Consequently, buildkite will schedule builds of derivations whose
dependencies are still in the process of being built. The result is many
buildkite agents doing nothing but blocking on other derivations being
built. We can easily prevent this by using the dependency information we
can get from the derivation (files) of the targets we want to build and
translating them into buildkite step dependencies.

The hard part of this has already been done for a while:
//nix/dependency-analyzer finds the dependencies between a list of
“known” derivations (even if they only depend on each other through
intermediate derivations) without depending on a specific derivation
builder convention, but rather relying on `.drv` files. It still has a
few rough edges, but has been working reliably for our purposes.

Since our steps are identified by derivation hashes, we can just
directly use the available dependency data. Luckily, buildkite seems to
just takes a step as if it was completed if it is skipped, so we don't
even have to check whether dependencies have been skipped or not.

On whitby it seems that the dependency analysis costs about a minute
additionally (which is how long it takes to run
//nix/dependency-analyzer in isolation just about).

Supersedes cl/5063, cl/5060, cl/5064 and cl/5065.

Change-Id: I91d2eb2b43d60811cac0d26fa94467298f622970
Reviewed-on: https://cl.tvl.fyi/c/depot/+/11116
Autosubmit: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
Reviewed-by: ezemtsov <eugene.zemtsov@gmail.com>
-rw-r--r--nix/buildkite/default.nix26
1 files changed, 23 insertions, 3 deletions
diff --git a/nix/buildkite/default.nix b/nix/buildkite/default.nix
index cb40fb9623..17f08ce1d5 100644
--- a/nix/buildkite/default.nix
+++ b/nix/buildkite/default.nix
@@ -27,6 +27,8 @@ let
 
   inherit (pkgs) lib runCommand writeText;
   inherit (depot.nix.readTree) mkLabel;
+
+  inherit (depot.nix) dependency-analyzer;
 in
 rec {
   # Create a unique key for the buildkite pipeline based on the given derivation
@@ -90,8 +92,21 @@ rec {
     target.__readTree
     ++ lib.optionals (target ? __subtarget) [ target.__subtarget ];
 
+  # Given a derivation (identified by drvPath) that is part of the list of
+  # targets passed to mkPipeline, determine all derivations that it depends on
+  # and are also part of the pipeline. Finally, return the keys of the steps
+  # that build them. This is used to populate `depends_on` in `mkStep`.
+  #
+  # See //nix/dependency-analyzer for documentation on the structure of `targetDepMap`.
+  getTargetPipelineDeps = targetDepMap: drvPath:
+    # Sanity check: We should only call this function on targets explicitly
+    # passed to mkPipeline. Thus it should have been passed as a “known” drv to
+    # dependency-analyzer.
+    assert targetDepMap.${drvPath}.known;
+    builtins.map keyForDrv targetDepMap.${drvPath}.knownDeps;
+
   # Create a pipeline step from a single target.
-  mkStep = { headBranch, parentTargetMap, target, cancelOnBuildFailing }:
+  mkStep = { headBranch, parentTargetMap, targetDepMap, target, cancelOnBuildFailing }:
     let
       label = mkLabel target;
       drvPath = unsafeDiscardStringContext target.drvPath;
@@ -110,7 +125,9 @@ rec {
       # Add a dependency on the initial static pipeline step which
       # always runs. This allows build steps uploaded in batches to
       # start running before all batches have been uploaded.
-      depends_on = [ ":init:" ] ++ lib.optionals (target ? meta.ci.buildkiteExtraDeps) target.meta.ci.buildkiteExtraDeps;
+      depends_on = [ ":init:" ]
+      ++ getTargetPipelineDeps targetDepMap drvPath
+      ++ lib.optionals (target ? meta.ci.buildkiteExtraDeps) target.meta.ci.buildkiteExtraDeps;
     } // lib.optionalAttrs (target ? meta.timeout) {
       timeout_in_minutes = target.meta.timeout / 60;
       # Additional arguments to set on the step.
@@ -213,12 +230,15 @@ rec {
       # logic/optimisation depends on knowing whether is executing.
       buildEnabled = elem "build" enabledPhases;
 
+      # Dependency relations between the `drvTargets`. See also //nix/dependency-analyzer.
+      targetDepMap = dependency-analyzer (dependency-analyzer.drvsToPaths drvTargets);
+
       # Convert a target into all of its steps, separated by build
       # phase (as phases end up in different chunks).
       targetToSteps = target:
         let
           mkStepArgs = {
-            inherit headBranch parentTargetMap target cancelOnBuildFailing;
+            inherit headBranch parentTargetMap targetDepMap target cancelOnBuildFailing;
           };
           step = mkStep mkStepArgs;