diff options
7 files changed, 11 insertions, 44 deletions
diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/ConfigServerMaintainer.java b/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/ConfigServerMaintainer.java index 3391e51f09a..0b70fca8908 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/ConfigServerMaintainer.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/ConfigServerMaintainer.java @@ -34,7 +34,7 @@ public abstract class ConfigServerMaintainer extends Maintainer { /** Creates a maintainer where maintainers on different nodes in this cluster run with even delay. */ ConfigServerMaintainer(ApplicationRepository applicationRepository, Curator curator, FlagSource flagSource, Instant now, Duration interval) { - super(null, Mode.exclusive, interval, now, new JobControl(new JobControlFlags(curator, flagSource)), + super(null, interval, now, new JobControl(new JobControlFlags(curator, flagSource)), jobMetrics(applicationRepository.metric()), cluster(curator)); this.applicationRepository = applicationRepository; } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ControllerMaintainer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ControllerMaintainer.java index dc59286d704..9bf6352813a 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ControllerMaintainer.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ControllerMaintainer.java @@ -33,7 +33,7 @@ public abstract class ControllerMaintainer extends Maintainer { } public ControllerMaintainer(Controller controller, Duration interval, String name, Set<SystemName> activeSystems) { - super(name, Mode.shared, interval, controller.clock().instant(), controller.jobControl(), + super(name, interval, controller.clock().instant(), controller.jobControl(), jobMetrics(controller.metric()), controller.curator().cluster()); this.controller = controller; this.activeSystems = Set.copyOf(Objects.requireNonNull(activeSystems)); diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeRepositoryMaintainer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeRepositoryMaintainer.java index 31db8a64b86..fdbf199898a 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeRepositoryMaintainer.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeRepositoryMaintainer.java @@ -25,7 +25,7 @@ public abstract class NodeRepositoryMaintainer extends Maintainer { private final NodeRepository nodeRepository; public NodeRepositoryMaintainer(NodeRepository nodeRepository, Duration interval, Metric metric) { - super(null, Mode.shared, interval, nodeRepository.clock().instant(), nodeRepository.jobControl(), + super(null, interval, nodeRepository.clock().instant(), nodeRepository.jobControl(), jobMetrics(metric), nodeRepository.database().cluster()); this.nodeRepository = nodeRepository; } 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 2426ca6c5e8..daad1f8fb4b 100644 --- a/vespajlib/src/main/java/com/yahoo/concurrent/maintenance/Maintainer.java +++ b/vespajlib/src/main/java/com/yahoo/concurrent/maintenance/Maintainer.java @@ -1,7 +1,6 @@ // 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.google.common.util.concurrent.UncheckedTimeoutException; import com.yahoo.net.HostName; import java.time.Duration; @@ -27,17 +26,15 @@ public abstract class Maintainer implements Runnable { protected final Logger log = Logger.getLogger(this.getClass().getName()); private final String name; - private final Mode mode; private final JobControl jobControl; private final JobMetrics jobMetrics; private final Duration interval; private final ScheduledExecutorService service; private final AtomicBoolean shutDown = new AtomicBoolean(); - public Maintainer(String name, Mode mode, Duration interval, Instant startedAt, JobControl jobControl, + public Maintainer(String name, Duration interval, Instant startedAt, JobControl jobControl, JobMetrics jobMetrics, List<String> clusterHostnames) { this.name = name; - this.mode = Objects.requireNonNull(mode); this.interval = requireInterval(interval); this.jobControl = Objects.requireNonNull(jobControl); this.jobMetrics = Objects.requireNonNull(jobMetrics); @@ -90,11 +87,6 @@ public abstract class Maintainer implements Runnable { jobMetrics.recordRunOf(name()); try (var lock = jobControl.lockJob(name())) { if (maintain()) jobMetrics.recordSuccessOf(name()); - } catch (UncheckedTimeoutException ignored) { - if (mode == Mode.shared) { - // This is fine as we're colliding with a run on another node - jobMetrics.recordSuccessOf(name()); - } } catch (Throwable e) { log.log(Level.WARNING, this + " failed. Will retry in " + interval.toMinutes() + " minutes", e); } finally { @@ -125,14 +117,4 @@ public abstract class Maintainer implements Runnable { return interval; } - public enum Mode { - - /** Completing a scheduled run on any node is sufficient */ - shared, - - /** Completing a scheduled run is always required */ - exclusive, - - } - } 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 61413c2973c..139a2901cd3 100644 --- a/vespajlib/src/test/java/com/yahoo/concurrent/maintenance/JobControlTest.java +++ b/vespajlib/src/test/java/com/yahoo/concurrent/maintenance/JobControlTest.java @@ -20,8 +20,8 @@ public class JobControlTest { String job1 = "Job1"; String job2 = "Job2"; JobMetrics metrics = new JobMetrics((job, instant) -> {}); - TestMaintainer maintainer1 = new TestMaintainer(job1, Maintainer.Mode.shared, jobControl, metrics); - TestMaintainer maintainer2 = new TestMaintainer(job2, Maintainer.Mode.shared, jobControl, metrics); + 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)); @@ -62,7 +62,7 @@ public class JobControlTest { public void testJobControlMayDeactivateJobs() { JobControlStateMock state = new JobControlStateMock(); JobControl jobControl = new JobControl(state); - TestMaintainer mockMaintainer = new TestMaintainer(null, Maintainer.Mode.shared, jobControl, new JobMetrics((job, instant) -> {})); + TestMaintainer mockMaintainer = new TestMaintainer(null, jobControl, new JobMetrics((job, instant) -> {})); assertTrue(jobControl.jobs().contains("TestMaintainer")); 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 4b726eb6637..e881d4b3ff6 100644 --- a/vespajlib/src/test/java/com/yahoo/concurrent/maintenance/MaintainerTest.java +++ b/vespajlib/src/test/java/com/yahoo/concurrent/maintenance/MaintainerTest.java @@ -44,7 +44,7 @@ public class MaintainerTest { public void success_metric() { AtomicLong consecutiveFailures = new AtomicLong(); JobMetrics jobMetrics = new JobMetrics((job, count) -> consecutiveFailures.set(count)); - TestMaintainer maintainer = new TestMaintainer(null, Maintainer.Mode.shared, jobControl, jobMetrics); + TestMaintainer maintainer = new TestMaintainer(null, jobControl, jobMetrics); // Maintainer fails twice in a row maintainer.successOnNextRun(false).run(); @@ -68,24 +68,9 @@ public class MaintainerTest { maintainer.throwOnNextRun(null).run(); assertEquals(0, consecutiveFailures.get()); - // Lock exception is considered successful for shared maintainer - maintainer.throwOnNextRun(new UncheckedTimeoutException()).run(); - assertEquals(0, consecutiveFailures.get()); - } - - @Test - public void success_metric_exclusive_maintainer() { - AtomicLong consecutiveFailures = new AtomicLong(); - JobMetrics jobMetrics = new JobMetrics((job, count) -> consecutiveFailures.set(count)); - TestMaintainer maintainer = new TestMaintainer(null, Maintainer.Mode.exclusive, jobControl, jobMetrics); - - // Timeout is considered a failure + // Lock exception is treated as a failure maintainer.throwOnNextRun(new UncheckedTimeoutException()).run(); assertEquals(1, consecutiveFailures.get()); - - // Maintainer recovers - maintainer.throwOnNextRun(null).run(); - assertEquals(0, 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 291268a416f..ea32af60208 100644 --- a/vespajlib/src/test/java/com/yahoo/concurrent/maintenance/TestMaintainer.java +++ b/vespajlib/src/test/java/com/yahoo/concurrent/maintenance/TestMaintainer.java @@ -14,8 +14,8 @@ class TestMaintainer extends Maintainer { private boolean success = true; private RuntimeException exceptionToThrow = null; - public TestMaintainer(String name, Mode mode, JobControl jobControl, JobMetrics jobMetrics) { - super(name, mode, Duration.ofDays(1), Instant.now(), jobControl, jobMetrics, List.of()); + public TestMaintainer(String name, JobControl jobControl, JobMetrics jobMetrics) { + super(name, Duration.ofDays(1), Instant.now(), jobControl, jobMetrics, List.of()); } public int totalRuns() { |