Record successFactor

4 files changed, 24 insertions, 13 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
index bb1bd0085ef..73a5dc77743 100644
--- a/vespajlib/src/main/java/com/yahoo/concurrent/maintenance/JobMetrics.java
+++ b/vespajlib/src/main/java/com/yahoo/concurrent/maintenance/JobMetrics.java
@@ -2,7 +2,6 @@
 package com.yahoo.concurrent.maintenance;
 
 import java.util.concurrent.ConcurrentHashMap;
-import java.util.function.BiConsumer;
 
 /**
  * Tracks and forwards maintenance job metrics.
@@ -13,8 +12,8 @@ public abstract class JobMetrics {
 
     private final ConcurrentHashMap<String, Long> incompleteRuns = new ConcurrentHashMap<>();
 
-    /** Record a run for given job */
-    public void recordRunOf(String job) {
+    /** Record starting of a run of a job */
+    public void starting(String job) {
         incompleteRuns.merge(job, 1L, Long::sum);
     }
 
@@ -23,14 +22,17 @@ public abstract class JobMetrics {
         incompleteRuns.put(job, 0L);
     }
 
-    /** Forward metrics for given job to metric consumer */
-    public void forward(String job) {
+    /**
+     * Records completion of a run of a job.
+     * This is guaranteed to always be called once whenever starting has been called.
+     */
+    public void completed(String job, double successFactor) {
         Long incompleteRuns = this.incompleteRuns.get(job);
         if (incompleteRuns != null) {
-            consume(job, incompleteRuns);
+            recordCompletion(job, incompleteRuns, successFactor);
         }
     }
 
-    protected abstract void consume(String job, Long incompleteRuns);
+    protected abstract void recordCompletion(String job, Long incompleteRuns, double successFactor);
 
 }
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 734c46a2819..fb2339a4334 100644
--- a/vespajlib/src/main/java/com/yahoo/concurrent/maintenance/Maintainer.java
+++ b/vespajlib/src/main/java/com/yahoo/concurrent/maintenance/Maintainer.java
@@ -83,7 +83,13 @@ public abstract class Maintainer implements Runnable {
     @Override
     public final String toString() { return name(); }
 
-    /** Called once each time this maintenance job should run. Returns whether the maintenance run was successful */
+    /**
+     * Called once each time this maintenance job should run.
+     *
+     * @return the degree to which the run was successful - a number between 0 (no success), to 1 (complete success).
+     *         Note that this indicates whether something is wrong, so e.g if the call did nothing because it should do
+     *         nothing,  1.0 should be returned.
+     */
     protected abstract boolean maintain();
 
     /** Returns the interval at which this job is set to run */
@@ -93,9 +99,12 @@ public abstract class Maintainer implements Runnable {
     public final void lockAndMaintain(boolean force) {
         if (!force && !jobControl.isActive(name())) return;
         log.log(Level.FINE, () -> "Running " + this.getClass().getSimpleName());
-        jobMetrics.recordRunOf(name());
+        jobMetrics.starting(name());
+        double successFactor = 0;
         try (var lock = jobControl.lockJob(name())) {
-            if (maintain()) jobMetrics.recordCompletionOf(name());
+            successFactor = maintain() ? 1.0 : 0.0;
+            if (successFactor > 0.0)
+                jobMetrics.recordCompletionOf(name());
         } catch (UncheckedTimeoutException e) {
             if (ignoreCollision) {
                 jobMetrics.recordCompletionOf(name());
@@ -105,7 +114,7 @@ public abstract class Maintainer implements Runnable {
         } catch (Throwable e) {
             log.log(Level.WARNING, this + " failed. Will retry in " + interval, e);
         } finally {
-            jobMetrics.forward(name());
+            jobMetrics.completed(name(), successFactor);
         }
         log.log(Level.FINE, () -> "Finished " + this.getClass().getSimpleName());
     }
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 fe66e61a1ba..01560c050ff 100644
--- a/vespajlib/src/test/java/com/yahoo/concurrent/maintenance/JobControlTest.java
+++ b/vespajlib/src/test/java/com/yahoo/concurrent/maintenance/JobControlTest.java
@@ -84,7 +84,7 @@ public class JobControlTest {
     private static class NoopJobMetrics extends JobMetrics {
 
         @Override
-        protected void consume(String job, Long incompleteRuns) { }
+        protected void recordCompletion(String job, Long incompleteRuns, double successFactor) { }
 
     }
 
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 871199c70d9..d2db380f4a1 100644
--- a/vespajlib/src/test/java/com/yahoo/concurrent/maintenance/MaintainerTest.java
+++ b/vespajlib/src/test/java/com/yahoo/concurrent/maintenance/MaintainerTest.java
@@ -77,7 +77,7 @@ public class MaintainerTest {
         AtomicLong consecutiveFailures = new AtomicLong();
 
         @Override
-        protected void consume(String job, Long incompleteRuns) {
+        protected void recordCompletion(String job, Long incompleteRuns, double successFactor) {
             consecutiveFailures.set(incompleteRuns);
         }