Merge pull request #11066 from vespa-engine/jvenstad/really-do-set-number-of-upgrades-per-minute

Really do n number of upgrades per minute

4 files changed, 35 insertions, 2 deletions

diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/Upgrader.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/Upgrader.java
index 73b135aa912..c89d74864cb 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/Upgrader.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/Upgrader.java
@@ -116,7 +116,18 @@ public class Upgrader extends Maintainer {
 
     /** Returns the number of applications to upgrade in this run */
     private int numberOfApplicationsToUpgrade() {
-        return Math.max(1, (int) (maintenanceInterval().getSeconds() * (upgradesPerMinute() / 60)));
+        return numberOfApplicationsToUpgrade(maintenanceInterval().dividedBy(Math.max(1, controller().curator().cluster().size())).toMillis(),
+                                             controller().clock().millis(),
+                                             upgradesPerMinute());
+    }
+
+    /** Returns the number of applications to upgrade in the interval containing now */
+    static int numberOfApplicationsToUpgrade(long intervalMillis, long nowMillis, double upgradesPerMinute) {
+        long intervalStart = Math.round(nowMillis / (double) intervalMillis) * intervalMillis;
+        double upgradesPerMilli = upgradesPerMinute / 60_000;
+        long upgradesAtStart = (long) (intervalStart * upgradesPerMilli);
+        long upgradesAtEnd = (long) ((intervalStart + intervalMillis) * upgradesPerMilli);
+        return (int) (upgradesAtEnd - upgradesAtStart);
     }
 
     /** Returns number of upgrades per minute */
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTester.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTester.java
index 61b393efbff..f9639835cf0 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTester.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTester.java
@@ -75,6 +75,7 @@ public class DeploymentTester {
 
         JobControl jobControl = new JobControl(tester.curator());
         this.upgrader = new Upgrader(tester.controller(), maintenanceInterval, jobControl, tester.curator());
+        this.upgrader.setUpgradesPerMinute(1); // Anything that makes it at least one for any maintenance period is fine.
         this.outstandingChangeDeployer = new OutstandingChangeDeployer(tester.controller(), maintenanceInterval, jobControl);
         this.readyJobTrigger = new ReadyJobsTrigger(tester.controller(), maintenanceInterval, jobControl);
         this.nameServiceDispatcher = new NameServiceDispatcher(tester.controller(), Duration.ofHours(12),
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/UpgraderTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/UpgraderTest.java
index 1fd51a453fd..d0c67d67842 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/UpgraderTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/UpgraderTest.java
@@ -1377,4 +1377,25 @@ public class UpgraderTest {
         assertTrue("Upgrade complete", applications.get().change().isEmpty());
     }
 
+    @Test
+    public void testUpgradesPerMinute() {
+        assertEquals(0, Upgrader.numberOfApplicationsToUpgrade(10, 0, 0));
+
+        for (long now = 0; now < 60_000; now++)
+            assertEquals(7, Upgrader.numberOfApplicationsToUpgrade(60_000, now, 7));
+
+        // Upgrade an app after 8s, 16s, ..., 120s.
+        assertEquals(3, Upgrader.numberOfApplicationsToUpgrade(30_000,       0, 7.5));
+        assertEquals(4, Upgrader.numberOfApplicationsToUpgrade(30_000,  30_000, 7.5));
+        assertEquals(4, Upgrader.numberOfApplicationsToUpgrade(30_000,  60_000, 7.5));
+        assertEquals(4, Upgrader.numberOfApplicationsToUpgrade(30_000,  90_000, 7.5));
+        assertEquals(3, Upgrader.numberOfApplicationsToUpgrade(30_000, 120_000, 7.5));
+
+        // Run upgrades for 20 minutes.
+        int upgrades = 0;
+        for (int i = 0, now = 0; i < 30; i++, now += 40_000)
+            upgrades += Upgrader.numberOfApplicationsToUpgrade(40_000, now, 8.7);
+        assertEquals(174, upgrades);
+    }
+
 }
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/ContainerControllerTester.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/ContainerControllerTester.java
index 8a1bbcd09d5..912d285fb52 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/ContainerControllerTester.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/ContainerControllerTester.java
@@ -54,8 +54,8 @@ public class ContainerControllerTester {
     public ContainerControllerTester(JDisc container, String responseFilePath) {
         containerTester = new ContainerTester(container, responseFilePath);
         CuratorDb curatorDb = controller().curator();
-        curatorDb.writeUpgradesPerMinute(100);
         upgrader = new Upgrader(controller(), Duration.ofDays(1), new JobControl(curatorDb), curatorDb);
+        upgrader.setUpgradesPerMinute(100); // Anything to make it more than one per maintenance interval.
     }
 
     public Controller controller() { return containerTester.controller(); }