Always account for risk of the next revision

2 files changed, 17 insertions, 0 deletions

diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentStatus.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentStatus.java
index 8d175018b1e..a82e77da130 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentStatus.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentStatus.java
@@ -257,6 +257,7 @@ public class DeploymentStatus {
         DeploymentInstanceSpec spec = application.deploymentSpec().requireInstance(instance);
         boolean ascending = next == spec.revisionTarget();
         int cumulativeRisk = 0;
+        int nextRisk = 0;
         int skippedCumulativeRisk = 0;
         Instant readySince = now;
         Change candidate = Change.empty();
@@ -272,6 +273,7 @@ public class DeploymentStatus {
 
             // This revision contains something new, so start aggregating the risk score.
             skippedCumulativeRisk += version.risk();
+            nextRisk = nextRisk > 0 ? nextRisk : version.risk();
             // If it's not yet ready to roll out, we keep looking.
             Optional<Instant> readyAt = status.dependenciesCompletedAt(Change.of(version), Optional.empty());
             if (readyAt.map(now::isBefore).orElse(true)) continue;
@@ -279,6 +281,7 @@ public class DeploymentStatus {
             // It's ready. If looking for the latest, max risk is 0, and we'll return now; otherwise, we _may_ keep on looking for more.
             cumulativeRisk += skippedCumulativeRisk;
             skippedCumulativeRisk = 0;
+            nextRisk = 0;
             if (cumulativeRisk >= spec.maxRisk())
                 return candidate.equals(Change.empty()) ? change : candidate; // If the first candidate exceeds max risk, we have to accept that.
 
@@ -289,6 +292,7 @@ public class DeploymentStatus {
         // If min risk is ready, or max idle time has passed, we return the candidate. Otherwise, no outstanding change is ready.
         return      instanceJobs(instance).values().stream().allMatch(jobs -> jobs.lastTriggered().isEmpty())
                ||   cumulativeRisk >= spec.minRisk()
+               ||   cumulativeRisk + nextRisk > spec.maxRisk()
                || ! now.isBefore(readySince.plus(Duration.ofHours(spec.maxIdleHours())))
                ? candidate : Change.empty();
     }
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTriggerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTriggerTest.java
index 876dfdadb67..a6d6b4494a3 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTriggerTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTriggerTest.java
@@ -1767,6 +1767,19 @@ public class DeploymentTriggerTest {
         tester.clock().advance(Duration.ofHours(8));
         tester.outstandingChangeDeployer().run();
         assertEquals(revision7, gamma.instance().change().application());
+
+        // revision 8 is has too low risk to roll out on its own, but will start rolling immediately when revision 9 is submitted
+        gamma.deploy();
+        alpha.submit(appPackage, 2);
+        var revision8 = alpha.lastSubmission();
+        alpha.deploy();
+        tester.outstandingChangeDeployer();
+        assertEquals(Change.empty(), gamma.instance().change());
+        assertEquals(revision7.get(), gamma.deployment(ZoneId.from("prod.us-east-3")).applicationVersion());
+
+        alpha.submit(appPackage, 5);
+        tester.outstandingChangeDeployer().run();
+        assertEquals(revision8, gamma.instance().change().application());
     }
 
     @Test