Emit QoS metric for all maintainers

6 files changed, 191 insertions, 76 deletions

diff --git a/vespajlib/src/main/java/com/yahoo/concurrent/maintenance/JobMetrics.java b/vespajlib/src/main/java/com/yahoo/concurrent/maintenance/JobMetrics.java
new file mode 100644
index 00000000000..4c05d46d782
--- /dev/null
+++ b/vespajlib/src/main/java/com/yahoo/concurrent/maintenance/JobMetrics.java
@@ -0,0 +1,41 @@
+// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+package com.yahoo.concurrent.maintenance;
+
+import java.time.Clock;
+import java.time.Instant;
+import java.util.Map;
+import java.util.Objects;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.function.BiConsumer;
+
+/**
+ * Tracks and forwards maintenance job metrics.
+ *
+ * @author mpolden
+ */
+public class JobMetrics {
+
+    private final Clock clock;
+    private final BiConsumer<String, Instant> metricConsumer;
+
+    private final Map<String, Instant> successfulRuns = new ConcurrentHashMap<>();
+
+    public JobMetrics(Clock clock, BiConsumer<String, Instant> metricConsumer) {
+        this.clock = Objects.requireNonNull(clock);
+        this.metricConsumer = metricConsumer;
+    }
+
+    /** Record successful run of given job */
+    public void recordSuccessOf(String job) {
+        successfulRuns.put(job, clock.instant());
+    }
+
+    /** Forward metrics for given job to metric consumer */
+    public void forward(String job) {
+        Instant lastSuccess = successfulRuns.get(job);
+        if (lastSuccess != null) {
+            metricConsumer.accept(job, lastSuccess);
+        }
+    }
+
+}
diff --git a/vespajlib/src/main/java/com/yahoo/concurrent/maintenance/Maintainer.java b/vespajlib/src/main/java/com/yahoo/concurrent/maintenance/Maintainer.java
index 9c40e5ec54f..0385c27536d 100644
--- a/vespajlib/src/main/java/com/yahoo/concurrent/maintenance/Maintainer.java
+++ b/vespajlib/src/main/java/com/yahoo/concurrent/maintenance/Maintainer.java
@@ -26,17 +26,19 @@ public abstract class Maintainer implements Runnable, AutoCloseable {
 
     private final String name;
     private final JobControl jobControl;
+    private final JobMetrics jobMetrics;
     private final Duration interval;
     private final ScheduledExecutorService service;
 
-    public Maintainer(String name, Duration interval, Instant startedAt, JobControl jobControl, List<String> clusterHostnames) {
-        this(name, interval, staggeredDelay(interval, startedAt, HostName.getLocalhost(), clusterHostnames), jobControl);
+    public Maintainer(String name, Duration interval, Instant startedAt, JobControl jobControl, JobMetrics jobMetrics, List<String> clusterHostnames) {
+        this(name, interval, staggeredDelay(interval, startedAt, HostName.getLocalhost(), clusterHostnames), jobControl, jobMetrics);
     }
 
-    public Maintainer(String name, Duration interval, Duration initialDelay, JobControl jobControl) {
+    public Maintainer(String name, Duration interval, Duration initialDelay, JobControl jobControl, JobMetrics jobMetrics) {
         this.name = name;
         this.interval = requireInterval(interval);
         this.jobControl = Objects.requireNonNull(jobControl);
+        this.jobMetrics = Objects.requireNonNull(jobMetrics);
         service = new ScheduledThreadPoolExecutor(1, r -> new Thread(r, name() + "-worker"));
         service.scheduleAtFixedRate(this, initialDelay.toMillis(), interval.toMillis(), TimeUnit.MILLISECONDS);
         jobControl.started(name(), this);
@@ -72,8 +74,8 @@ public abstract class Maintainer implements Runnable, AutoCloseable {
     @Override
     public final String toString() { return name(); }
 
-    /** Called once each time this maintenance job should run */
-    protected abstract void maintain();
+    /** Called once each time this maintenance job should run. Returns whether the maintenance run was succesful */
+    protected abstract boolean maintain();
 
     /** Returns the interval at which this job is set to run */
     protected Duration interval() { return interval; }
@@ -82,7 +84,12 @@ public abstract class Maintainer implements Runnable, AutoCloseable {
     @SuppressWarnings("unused")
     public final void lockAndMaintain() {
         try (var lock = jobControl.lockJob(name())) {
-            maintain();
+            try {
+                if (maintain()) jobMetrics.recordSuccessOf(name());
+            } finally {
+                // Always forward metrics
+                jobMetrics.forward(name());
+            }
         }
     }
 
diff --git a/vespajlib/src/test/java/com/yahoo/concurrent/maintenance/JobControlStateMock.java b/vespajlib/src/test/java/com/yahoo/concurrent/maintenance/JobControlStateMock.java
new file mode 100644
index 00000000000..28c701a67db
--- /dev/null
+++ b/vespajlib/src/test/java/com/yahoo/concurrent/maintenance/JobControlStateMock.java
@@ -0,0 +1,35 @@
+// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+package com.yahoo.concurrent.maintenance;
+
+import com.yahoo.transaction.Mutex;
+
+import java.util.Collections;
+import java.util.HashSet;
+import java.util.Set;
+
+/**
+ * @author mpolden
+ */
+class JobControlStateMock implements JobControlState {
+
+    private final Set<String> inactiveJobs = new HashSet<>();
+
+    @Override
+    public Set<String> readInactiveJobs() {
+        return Collections.unmodifiableSet(inactiveJobs);
+    }
+
+    @Override
+    public Mutex lockMaintenanceJob(String job) {
+        return () -> {};
+    }
+
+    public void setActive(String job, boolean active) {
+        if (active) {
+            inactiveJobs.remove(job);
+        } else {
+            inactiveJobs.add(job);
+        }
+    }
+
+}
diff --git a/vespajlib/src/test/java/com/yahoo/concurrent/maintenance/JobControlTest.java b/vespajlib/src/test/java/com/yahoo/concurrent/maintenance/JobControlTest.java
index 0640ab2835a..a0ca9b529c5 100644
--- a/vespajlib/src/test/java/com/yahoo/concurrent/maintenance/JobControlTest.java
+++ b/vespajlib/src/test/java/com/yahoo/concurrent/maintenance/JobControlTest.java
@@ -1,15 +1,8 @@
 // Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.concurrent.maintenance;
 
-import com.yahoo.transaction.Mutex;
 import org.junit.Test;
 
-import java.time.Duration;
-import java.time.Instant;
-import java.util.HashSet;
-import java.util.List;
-import java.util.Set;
-
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
@@ -21,18 +14,13 @@ public class JobControlTest {
 
     @Test
     public void testJobControl() {
-        MockJobControlState state = new MockJobControlState();
+        JobControlStateMock state = new JobControlStateMock();
         JobControl jobControl = new JobControl(state);
 
-        MockMaintainer maintainer1 = new MockMaintainer();
-        MockMaintainer maintainer2 = new MockMaintainer();
-        assertTrue(jobControl.jobs().isEmpty());
-
         String job1 = "Job1";
         String job2 = "Job2";
-
-        jobControl.started(job1, maintainer1);
-        jobControl.started(job2, maintainer2);
+        TestMaintainer maintainer1 = new TestMaintainer(job1, jobControl);
+        TestMaintainer maintainer2 = new TestMaintainer(job2, jobControl);
         assertEquals(2, jobControl.jobs().size());
         assertTrue(jobControl.jobs().contains(job1));
         assertTrue(jobControl.jobs().contains(job2));
@@ -59,79 +47,36 @@ public class JobControlTest {
         // Run jobs on-demand
         jobControl.run(job1);
         jobControl.run(job1);
-        assertEquals(2, maintainer1.maintenanceInvocations);
+        assertEquals(2, maintainer1.totalRuns());
         jobControl.run(job2);
-        assertEquals(1, maintainer2.maintenanceInvocations);
+        assertEquals(1, maintainer2.totalRuns());
 
         // Running jobs on-demand ignores inactive flag
         state.setActive(job1, false);
         jobControl.run(job1);
-        assertEquals(3, maintainer1.maintenanceInvocations);
+        assertEquals(3, maintainer1.totalRuns());
     }
 
     @Test
     public void testJobControlMayDeactivateJobs() {
-        MockJobControlState state = new MockJobControlState();
+        JobControlStateMock state = new JobControlStateMock();
         JobControl jobControl = new JobControl(state);
-        MockMaintainer mockMaintainer = new MockMaintainer(jobControl);
+        TestMaintainer mockMaintainer = new TestMaintainer(null, jobControl);
 
-        assertTrue(jobControl.jobs().contains("MockMaintainer"));
+        assertTrue(jobControl.jobs().contains("TestMaintainer"));
 
-        assertEquals(0, mockMaintainer.maintenanceInvocations);
+        assertEquals(0, mockMaintainer.totalRuns());
 
         mockMaintainer.run();
-        assertEquals(1, mockMaintainer.maintenanceInvocations);
+        assertEquals(1, mockMaintainer.totalRuns());
 
-        state.setActive("MockMaintainer", false);
+        state.setActive("TestMaintainer", false);
         mockMaintainer.run();
-        assertEquals(1, mockMaintainer.maintenanceInvocations);
+        assertEquals(1, mockMaintainer.totalRuns());
 
-        state.setActive("MockMaintainer", true);
+        state.setActive("TestMaintainer", true);
         mockMaintainer.run();
-        assertEquals(2, mockMaintainer.maintenanceInvocations);
-    }
-
-    private static class MockJobControlState implements JobControlState {
-
-        private final Set<String> inactiveJobs = new HashSet<>();
-
-        @Override
-        public Set<String> readInactiveJobs() {
-            return new HashSet<>(inactiveJobs);
-        }
-
-        @Override
-        public Mutex lockMaintenanceJob(String job) {
-            return () -> {};
-        }
-
-        public void setActive(String job, boolean active) {
-            if (active) {
-                inactiveJobs.remove(job);
-            } else {
-                inactiveJobs.add(job);
-            }
-        }
-
-    }
-
-    private static class MockMaintainer extends Maintainer {
-
-        int maintenanceInvocations = 0;
-
-        private MockMaintainer(JobControl jobControl) {
-            super(null, Duration.ofHours(1), Instant.now(), jobControl, List.of());
-        }
-
-        private MockMaintainer() {
-            this(new JobControl(new MockJobControlState()));
-        }
-
-        @Override
-        protected void maintain() {
-            maintenanceInvocations++;
-        }
-
+        assertEquals(2, mockMaintainer.totalRuns());
     }
 
 }
diff --git a/vespajlib/src/test/java/com/yahoo/concurrent/maintenance/MaintainerTest.java b/vespajlib/src/test/java/com/yahoo/concurrent/maintenance/MaintainerTest.java
index 820d1fc3d1d..47ed010e95e 100644
--- a/vespajlib/src/test/java/com/yahoo/concurrent/maintenance/MaintainerTest.java
+++ b/vespajlib/src/test/java/com/yahoo/concurrent/maintenance/MaintainerTest.java
@@ -1,13 +1,16 @@
 // Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.concurrent.maintenance;
 
+import com.yahoo.test.ManualClock;
 import org.junit.Test;
 
 import java.time.Duration;
 import java.time.Instant;
 import java.util.List;
+import java.util.concurrent.atomic.AtomicReference;
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNull;
 
 /**
  * @author freva
@@ -36,4 +39,39 @@ public class MaintainerTest {
         assertEquals(300, Maintainer.staggeredDelay(interval, now, "cfg0", cluster).toMillis());
     }
 
+    @Test
+    public void success_metric() {
+        ManualClock clock = new ManualClock();
+        AtomicReference<Instant> lastSuccess = new AtomicReference<>();
+        JobMetrics jobMetrics = new JobMetrics(clock, (job, instant) -> lastSuccess.set(instant));
+        TestMaintainer maintainer = new TestMaintainer(jobMetrics);
+
+        // Maintainer not successful yet
+        maintainer.successOnNextRun(false).run();
+        assertNull(lastSuccess.get());
+
+        // Maintainer runs successfully
+        clock.advance(Duration.ofHours(1));
+        Instant lastSuccess0 = clock.instant();
+        maintainer.successOnNextRun(true).run();
+        assertEquals(lastSuccess0, lastSuccess.get());
+
+        // Maintainer runs successfully again
+        clock.advance(Duration.ofHours(2));
+        Instant lastSuccess1 = clock.instant();
+        maintainer.run();
+        assertEquals(lastSuccess1, lastSuccess.get());
+
+        // Maintainer throws
+        clock.advance(Duration.ofHours(5));
+        maintainer.throwOnNextRun(true).run();
+        assertEquals("Time of successful run is unchanged", lastSuccess1, lastSuccess.get());
+
+        // Maintainer recovers
+        clock.advance(Duration.ofHours(3));
+        Instant lastSuccess2 = clock.instant();
+        maintainer.throwOnNextRun(false).run();
+        assertEquals(lastSuccess2, lastSuccess.get());
+    }
+
 }
diff --git a/vespajlib/src/test/java/com/yahoo/concurrent/maintenance/TestMaintainer.java b/vespajlib/src/test/java/com/yahoo/concurrent/maintenance/TestMaintainer.java
new file mode 100644
index 00000000000..0ea24fb6c2b
--- /dev/null
+++ b/vespajlib/src/test/java/com/yahoo/concurrent/maintenance/TestMaintainer.java
@@ -0,0 +1,49 @@
+// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+package com.yahoo.concurrent.maintenance;
+
+import java.time.Clock;
+import java.time.Duration;
+
+/**
+ * @author mpolden
+ */
+class TestMaintainer extends Maintainer {
+
+    private int totalRuns = 0;
+    private boolean success = true;
+    private boolean throwing = false;
+
+    public TestMaintainer(String name, JobControl jobControl, JobMetrics jobMetrics) {
+        super(name, Duration.ofDays(1), Duration.ofDays(1), jobControl, jobMetrics);
+    }
+
+    public TestMaintainer(JobMetrics jobMetrics) {
+        this(null, new JobControl(new JobControlStateMock()), jobMetrics);
+    }
+
+    public TestMaintainer(String name, JobControl jobControl) {
+        this(name, jobControl, new JobMetrics(Clock.systemUTC(), (job, instant) -> {}));
+    }
+
+    public int totalRuns() {
+        return totalRuns;
+    }
+
+    public TestMaintainer successOnNextRun(boolean success) {
+        this.success = success;
+        return this;
+    }
+
+    public TestMaintainer throwOnNextRun(boolean throwing) {
+        this.throwing = throwing;
+        return this;
+    }
+
+    @Override
+    protected boolean maintain() {
+        if (throwing) throw new RuntimeException("Maintenance run failed");
+        totalRuns++;
+        return success;
+    }
+
+}