diff options
Diffstat (limited to 'vespajlib/src')
5 files changed, 33 insertions, 63 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 73a5dc77743..fcc5b8e57a2 100644 --- a/vespajlib/src/main/java/com/yahoo/concurrent/maintenance/JobMetrics.java +++ b/vespajlib/src/main/java/com/yahoo/concurrent/maintenance/JobMetrics.java @@ -2,18 +2,25 @@ package com.yahoo.concurrent.maintenance; import java.util.concurrent.ConcurrentHashMap; +import java.util.function.BiConsumer; /** * Tracks and forwards maintenance job metrics. * * @author mpolden */ -public abstract class JobMetrics { +public class JobMetrics { + + private final BiConsumer<String, Long> metricConsumer; private final ConcurrentHashMap<String, Long> incompleteRuns = new ConcurrentHashMap<>(); - /** Record starting of a run of a job */ - public void starting(String job) { + public JobMetrics(BiConsumer<String, Long> metricConsumer) { + this.metricConsumer = metricConsumer; + } + + /** Record a run for given job */ + public void recordRunOf(String job) { incompleteRuns.merge(job, 1L, Long::sum); } @@ -22,17 +29,12 @@ public abstract class JobMetrics { incompleteRuns.put(job, 0L); } - /** - * 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) { + /** Forward metrics for given job to metric consumer */ + public void forward(String job) { Long incompleteRuns = this.incompleteRuns.get(job); if (incompleteRuns != null) { - recordCompletion(job, incompleteRuns, successFactor); + metricConsumer.accept(job, 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 2a9e6dda6b6..734c46a2819 100644 --- a/vespajlib/src/main/java/com/yahoo/concurrent/maintenance/Maintainer.java +++ b/vespajlib/src/main/java/com/yahoo/concurrent/maintenance/Maintainer.java @@ -83,19 +83,8 @@ public abstract class Maintainer implements Runnable { @Override public final String toString() { return name(); } - /** - * 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 double maintain(); - - /** Convenience methods to convert attempts and failures into a success factor */ - protected final double asSuccessFactor(int attempts, int failures) { - return attempts == 0 ? 1.0 : 1 - (double)failures / attempts; - } + /** Called once each time this maintenance job should run. Returns whether the maintenance run was successful */ + protected abstract boolean maintain(); /** Returns the interval at which this job is set to run */ protected Duration interval() { return interval; } @@ -104,12 +93,9 @@ 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.starting(name()); - double successFactor = 0; + jobMetrics.recordRunOf(name()); try (var lock = jobControl.lockJob(name())) { - successFactor = maintain(); - if (successFactor > 0.0) - jobMetrics.recordCompletionOf(name()); + if (maintain()) jobMetrics.recordCompletionOf(name()); } catch (UncheckedTimeoutException e) { if (ignoreCollision) { jobMetrics.recordCompletionOf(name()); @@ -119,7 +105,7 @@ public abstract class Maintainer implements Runnable { } catch (Throwable e) { log.log(Level.WARNING, this + " failed. Will retry in " + interval, e); } finally { - jobMetrics.completed(name(), successFactor); + jobMetrics.forward(name()); } 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 01560c050ff..139a2901cd3 100644 --- a/vespajlib/src/test/java/com/yahoo/concurrent/maintenance/JobControlTest.java +++ b/vespajlib/src/test/java/com/yahoo/concurrent/maintenance/JobControlTest.java @@ -3,8 +3,6 @@ package com.yahoo.concurrent.maintenance; import org.junit.Test; -import java.util.concurrent.atomic.AtomicLong; - import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; @@ -21,8 +19,9 @@ public class JobControlTest { String job1 = "Job1"; String job2 = "Job2"; - TestMaintainer maintainer1 = new TestMaintainer(job1, jobControl, new NoopJobMetrics()); - TestMaintainer maintainer2 = new TestMaintainer(job2, jobControl, new NoopJobMetrics()); + JobMetrics metrics = new JobMetrics((job, instant) -> {}); + TestMaintainer maintainer1 = new TestMaintainer(job1, jobControl, metrics); + TestMaintainer maintainer2 = new TestMaintainer(job2, jobControl, metrics); assertEquals(2, jobControl.jobs().size()); assertTrue(jobControl.jobs().contains(job1)); assertTrue(jobControl.jobs().contains(job2)); @@ -63,7 +62,7 @@ public class JobControlTest { public void testJobControlMayDeactivateJobs() { JobControlStateMock state = new JobControlStateMock(); JobControl jobControl = new JobControl(state); - TestMaintainer mockMaintainer = new TestMaintainer(null, jobControl, new NoopJobMetrics()); + TestMaintainer mockMaintainer = new TestMaintainer(null, jobControl, new JobMetrics((job, instant) -> {})); assertTrue(jobControl.jobs().contains("TestMaintainer")); @@ -81,11 +80,4 @@ public class JobControlTest { assertEquals(2, mockMaintainer.totalRuns()); } - private static class NoopJobMetrics extends JobMetrics { - - @Override - 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 d2db380f4a1..e881d4b3ff6 100644 --- a/vespajlib/src/test/java/com/yahoo/concurrent/maintenance/MaintainerTest.java +++ b/vespajlib/src/test/java/com/yahoo/concurrent/maintenance/MaintainerTest.java @@ -42,45 +42,35 @@ public class MaintainerTest { @Test public void success_metric() { - TestJobMetrics jobMetrics = new TestJobMetrics(); + AtomicLong consecutiveFailures = new AtomicLong(); + JobMetrics jobMetrics = new JobMetrics((job, count) -> consecutiveFailures.set(count)); TestMaintainer maintainer = new TestMaintainer(null, jobControl, jobMetrics); // Maintainer fails twice in a row maintainer.successOnNextRun(false).run(); - assertEquals(1, jobMetrics.consecutiveFailures.get()); + assertEquals(1, consecutiveFailures.get()); maintainer.successOnNextRun(false).run(); - assertEquals(2, jobMetrics.consecutiveFailures.get()); + assertEquals(2, consecutiveFailures.get()); // Maintainer runs successfully maintainer.successOnNextRun(true).run(); - assertEquals(0, jobMetrics.consecutiveFailures.get()); + assertEquals(0, consecutiveFailures.get()); // Maintainer runs successfully again maintainer.run(); - assertEquals(0, jobMetrics.consecutiveFailures.get()); + assertEquals(0, consecutiveFailures.get()); // Maintainer throws maintainer.throwOnNextRun(new RuntimeException()).run(); - assertEquals(1, jobMetrics.consecutiveFailures.get()); + assertEquals(1, consecutiveFailures.get()); // Maintainer recovers maintainer.throwOnNextRun(null).run(); - assertEquals(0, jobMetrics.consecutiveFailures.get()); + assertEquals(0, consecutiveFailures.get()); // Lock exception is treated as a failure maintainer.throwOnNextRun(new UncheckedTimeoutException()).run(); - assertEquals(1, jobMetrics.consecutiveFailures.get()); - } - - private static class TestJobMetrics extends JobMetrics { - - AtomicLong consecutiveFailures = new AtomicLong(); - - @Override - protected void recordCompletion(String job, Long incompleteRuns, double successFactor) { - consecutiveFailures.set(incompleteRuns); - } - + assertEquals(1, consecutiveFailures.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 index 7424b17cab2..44a00a37a83 100644 --- a/vespajlib/src/test/java/com/yahoo/concurrent/maintenance/TestMaintainer.java +++ b/vespajlib/src/test/java/com/yahoo/concurrent/maintenance/TestMaintainer.java @@ -33,10 +33,10 @@ class TestMaintainer extends Maintainer { } @Override - protected double maintain() { + protected boolean maintain() { if (exceptionToThrow != null) throw exceptionToThrow; totalRuns++; - return success ? 1.0 : 0.0; + return success; } } |