Merge pull request #18566 from vespa-engine/jonmv/avoid-waiting-for-lock-of-transitive-dependencies-not-in-this-job

Avoid taking lock which is not a dependency in this job

2 files changed, 13 insertions, 11 deletions

diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobController.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobController.java
index 28a6d32ce54..d9d82b41e71 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobController.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobController.java
@@ -368,7 +368,7 @@ public class JobController {
         List<Lock> locks = new ArrayList<>();
         try {
             // Ensure no step is still running before we finish the run — report depends transitively on all the other steps.
-            for (Step step : report.allPrerequisites())
+            for (Step step : report.allPrerequisites(run(id).get().steps().keySet()))
                 locks.add(curator.lock(id.application(), id.type(), step));
 
             locked(id, run -> { // Store the modified run after it has been written to history, in case the latter fails.
@@ -588,7 +588,7 @@ public class JobController {
     /** Locks the given step and checks none of its prerequisites are running, then performs the given actions. */
     public void locked(ApplicationId id, JobType type, Step step, Consumer<LockedStep> action) throws TimeoutException {
         try (Lock lock = curator.lock(id, type, step)) {
-            for (Step prerequisite : step.allPrerequisites()) // Check that no prerequisite is still running.
+            for (Step prerequisite : step.allPrerequisites(last(id, type).get().steps().keySet())) // Check that no prerequisite is still running.
                 try (Lock __ = curator.lock(id, type, prerequisite)) { ; }
 
             action.accept(new LockedStep(lock, step));
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/Step.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/Step.java
index 3077dc5211a..ce34a021218 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/Step.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/Step.java
@@ -1,10 +1,12 @@
 // Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.vespa.hosted.controller.deployment;
 
+import java.util.Collection;
 import java.util.List;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
 
+import static java.util.stream.Collectors.toList;
 import static java.util.stream.Collectors.toUnmodifiableList;
 
 /**
@@ -72,24 +74,24 @@ public enum Step {
 
     private final boolean alwaysRun;
     private final List<Step> prerequisites;
-    private final List<Step> allPrerequisites;
 
     Step(boolean alwaysRun, Step... prerequisites) {
         this.alwaysRun = alwaysRun;
         this.prerequisites = List.of(prerequisites);
-        this.allPrerequisites = Stream.concat(Stream.of(prerequisites),
-                                              Stream.of(prerequisites).flatMap(pre -> pre.allPrerequisites().stream()))
-                                      .sorted()
-                                      .distinct()
-                                      .collect(toUnmodifiableList());
     }
 
     /** Returns whether this is a cleanup-step, and should always run, regardless of job outcome, when specified in a job. */
     public boolean alwaysRun() { return alwaysRun; }
 
-    /** Returns all prerequisite steps for this, recursively. */
-    public List<Step> allPrerequisites() {
-        return allPrerequisites;
+    /** Returns all prerequisite steps for this, including transient ones, in a job profile containing the given steps. */
+    public List<Step> allPrerequisites(Collection<Step> among) {
+        return prerequisites.stream()
+                            .filter(among::contains)
+                            .flatMap(pre -> Stream.concat(Stream.of(pre),
+                                                          pre.allPrerequisites(among).stream()))
+                            .sorted()
+                            .distinct()
+                            .collect(toList());
     }