Validate interval, tolerate cluster without this, clean up test

2 files changed, 11 insertions, 10 deletions

diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/Maintainer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/Maintainer.java
index eab7eed27f8..8b05c71fa29 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/Maintainer.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/Maintainer.java
@@ -3,12 +3,14 @@ package com.yahoo.vespa.hosted.controller.maintenance;
 
 import com.google.common.collect.ImmutableSet;
 import com.yahoo.component.AbstractComponent;
+import com.yahoo.config.provision.HostName;
 import com.yahoo.config.provision.SystemName;
 import com.yahoo.vespa.curator.Lock;
 import com.yahoo.vespa.hosted.controller.Controller;
 
 import java.time.Duration;
 import java.util.EnumSet;
+import java.util.List;
 import java.util.Set;
 import java.util.concurrent.ScheduledExecutorService;
 import java.util.concurrent.ScheduledThreadPoolExecutor;
@@ -38,6 +40,9 @@ public abstract class Maintainer extends AbstractComponent implements Runnable {
     }
 
     public Maintainer(Controller controller, Duration interval, JobControl jobControl, String name, Set<SystemName> permittedSystems) {
+        if (interval.isNegative() || interval.isZero())
+            throw new IllegalArgumentException("Interval must be positive, but was " + interval);
+
         this.controller = controller;
         this.maintenanceInterval = interval;
         this.jobControl = jobControl;
@@ -92,11 +97,12 @@ public abstract class Maintainer extends AbstractComponent implements Runnable {
     }
 
     private static long staggeredDelay(Controller controller, Duration interval) {
-        int indexInCluster = 1 + controller.curator().cluster().indexOf(controller.hostname());
-        long intervalMillis = Math.max(1, interval.toMillis());
-        long nextCycleStart = ((controller.clock().millis() / intervalMillis) + 1) * intervalMillis;
-        long staggeredStart = nextCycleStart + intervalMillis * indexInCluster / Math.max(1, controller.curator().cluster().size());
-        return staggeredStart - controller.clock().millis();
+        List<HostName> cluster = controller.curator().cluster();
+        if ( ! cluster.contains(controller.hostname()))
+            return interval.toMillis();
+
+        long timeUntilNextRun = Math.floorMod(-controller.clock().millis(), interval.toMillis() / cluster.size());
+        return timeUntilNextRun + (1 + cluster.indexOf(controller.hostname())) * interval.toMillis() / cluster.size();
     }
 
 }
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/MaintainerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/MaintainerTest.java
index 7bbecf88ae7..c10ceef6adb 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/MaintainerTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/MaintainerTest.java
@@ -32,11 +32,6 @@ public class MaintainerTest {
         assertEquals(1, executions.get());
     }
 
-    @Test
-    public void staggering() {
-        System.err.println(3 * 2 / 3);
-    }
-
     private Maintainer maintainerIn(SystemName system, AtomicInteger executions) {
         return new Maintainer(tester.controller(), Duration.ofDays(1), new JobControl(tester.controller().curator()),
                               "MockMaintainer", EnumSet.of(system)) {