From 4ea035b70e31781783113a4b6eb56c21c7c02bf3 Mon Sep 17 00:00:00 2001 From: Øyvind Grønnesby Date: Mon, 12 Dec 2022 16:50:14 +0100 Subject: Create a metric for maintainer execution time --- .../server/maintenance/ApplicationPackageMaintainer.java | 2 +- .../config/server/maintenance/ConfigServerMaintainer.java | 11 +++++++---- .../config/server/maintenance/FileDistributionMaintainer.java | 2 +- .../vespa/config/server/maintenance/ReindexingMaintainer.java | 2 +- .../vespa/config/server/maintenance/SessionsMaintainer.java | 2 +- .../vespa/config/server/maintenance/TenantsMaintainer.java | 2 +- .../hosted/controller/maintenance/ControllerMaintainer.java | 5 +++-- .../provision/maintenance/NodeRepositoryMaintainer.java | 8 +++++--- .../java/com/yahoo/concurrent/maintenance/JobMetrics.java | 2 +- .../java/com/yahoo/concurrent/maintenance/Maintainer.java | 11 ++++++++--- .../java/com/yahoo/concurrent/maintenance/JobControlTest.java | 2 +- .../java/com/yahoo/concurrent/maintenance/MaintainerTest.java | 4 +++- .../java/com/yahoo/concurrent/maintenance/TestMaintainer.java | 3 ++- 13 files changed, 35 insertions(+), 21 deletions(-) diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/ApplicationPackageMaintainer.java b/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/ApplicationPackageMaintainer.java index 12972e5c465..48d2a4c420f 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/ApplicationPackageMaintainer.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/ApplicationPackageMaintainer.java @@ -52,7 +52,7 @@ public class ApplicationPackageMaintainer extends ConfigServerMaintainer { Curator curator, Duration interval, FlagSource flagSource) { - super(applicationRepository, curator, flagSource, applicationRepository.clock().instant(), interval, false); + super(applicationRepository, curator, flagSource, applicationRepository.clock(), interval, false); this.applicationRepository = applicationRepository; this.configserverConfig = applicationRepository.configserverConfig(); this.downloadDirectory = new File(Defaults.getDefaults().underVespaHome(configserverConfig.fileReferencesDir())); 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 39904a5b2f2..a1a1a16ab9e 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 @@ -14,6 +14,7 @@ import com.yahoo.vespa.flags.FlagSource; import com.yahoo.vespa.flags.ListFlag; import com.yahoo.vespa.flags.PermanentFlags; +import java.time.Clock; import java.time.Duration; import java.time.Instant; import java.util.Arrays; @@ -33,8 +34,8 @@ 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, boolean useLock) { - super(null, interval, now, new JobControl(new JobControlFlags(curator, flagSource, useLock)), + Clock clock, Duration interval, boolean useLock) { + super(null, interval, clock, new JobControl(new JobControlFlags(curator, flagSource, useLock)), new ConfigServerJobMetrics(applicationRepository.metric()), cluster(curator), false); this.applicationRepository = applicationRepository; } @@ -48,8 +49,10 @@ public abstract class ConfigServerMaintainer extends Maintainer { } @Override - public void completed(String job, double successFactor) { - metric.set("maintenance.successFactor", successFactor, metric.createContext(Map.of("job", job))); + public void completed(String job, double successFactor, long durationMs) { + var context = metric.createContext(Map.of("job", job)); + metric.set("maintenance.successFactor", successFactor, context); + metric.set("maintenance.duration", durationMs, context); } } diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/FileDistributionMaintainer.java b/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/FileDistributionMaintainer.java index 0187f35cce0..42a037344a8 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/FileDistributionMaintainer.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/FileDistributionMaintainer.java @@ -27,7 +27,7 @@ public class FileDistributionMaintainer extends ConfigServerMaintainer { Duration interval, FlagSource flagSource, FileDirectory fileDirectory) { - super(applicationRepository, curator, flagSource, applicationRepository.clock().instant(), interval, false); + super(applicationRepository, curator, flagSource, applicationRepository.clock(), interval, false); this.applicationRepository = applicationRepository; ConfigserverConfig configserverConfig = applicationRepository.configserverConfig(); this.maxUnusedFileReferenceAge = Duration.ofMinutes(configserverConfig.keepUnusedFileReferencesMinutes()); diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/ReindexingMaintainer.java b/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/ReindexingMaintainer.java index 05f3f2c6483..a9672455d09 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/ReindexingMaintainer.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/ReindexingMaintainer.java @@ -48,7 +48,7 @@ public class ReindexingMaintainer extends ConfigServerMaintainer { public ReindexingMaintainer(ApplicationRepository applicationRepository, Curator curator, FlagSource flagSource, Duration interval, ConfigConvergenceChecker convergence, Clock clock) { - super(applicationRepository, curator, flagSource, clock.instant(), interval, true); + super(applicationRepository, curator, flagSource, clock, interval, true); this.convergence = convergence; this.clock = clock; } diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/SessionsMaintainer.java b/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/SessionsMaintainer.java index 38ac41d0eb9..2ebae30aa2d 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/SessionsMaintainer.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/SessionsMaintainer.java @@ -18,7 +18,7 @@ import java.util.logging.Level; public class SessionsMaintainer extends ConfigServerMaintainer { SessionsMaintainer(ApplicationRepository applicationRepository, Curator curator, Duration interval, FlagSource flagSource) { - super(applicationRepository, curator, flagSource, applicationRepository.clock().instant(), interval, true); + super(applicationRepository, curator, flagSource, applicationRepository.clock(), interval, true); } @Override diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/TenantsMaintainer.java b/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/TenantsMaintainer.java index ec09e89568c..8a00755188a 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/TenantsMaintainer.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/TenantsMaintainer.java @@ -25,7 +25,7 @@ public class TenantsMaintainer extends ConfigServerMaintainer { TenantsMaintainer(ApplicationRepository applicationRepository, Curator curator, FlagSource flagSource, Duration interval, Clock clock) { - super(applicationRepository, curator, flagSource, applicationRepository.clock().instant(), interval, true); + super(applicationRepository, curator, flagSource, applicationRepository.clock(), interval, true); this.ttlForUnusedTenant = defaultTtlForUnusedTenant; this.clock = clock; } 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 f1223c7d162..4e6eda9f0fa 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 activeSystems) { - super(name, interval, controller.clock().instant(), controller.jobControl(), + super(name, interval, controller.clock(), controller.jobControl(), new ControllerJobMetrics(controller.metric()), controller.curator().cluster(), true); this.controller = controller; this.activeSystems = Set.copyOf(Objects.requireNonNull(activeSystems)); @@ -56,8 +56,9 @@ public abstract class ControllerMaintainer extends Maintainer { } @Override - public void completed(String job, double successFactor) { + public void completed(String job, double successFactor, long durationMs) { metric.set("maintenance.successFactor", successFactor, metric.createContext(Map.of("job", job))); + metric.set("maintenance.duration", durationMs, metric.createContext(Map.of("job", job))); } } 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 77fe4ba129d..3c00e3b708d 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 @@ -24,7 +24,7 @@ public abstract class NodeRepositoryMaintainer extends Maintainer { private final NodeRepository nodeRepository; public NodeRepositoryMaintainer(NodeRepository nodeRepository, Duration interval, Metric metric) { - super(null, interval, nodeRepository.clock().instant(), nodeRepository.jobControl(), + super(null, interval, nodeRepository.clock(), nodeRepository.jobControl(), new NodeRepositoryJobMetrics(metric), nodeRepository.database().cluster(), true); this.nodeRepository = nodeRepository; } @@ -57,8 +57,10 @@ public abstract class NodeRepositoryMaintainer extends Maintainer { } @Override - public void completed(String job, double successFactor) { - metric.set("maintenance.successFactor", successFactor, metric.createContext(Map.of("job", job))); + public void completed(String job, double successFactor, long duration) { + var context = metric.createContext(Map.of("job", job)); + metric.set("maintenance.successFactor", successFactor, context); + metric.set("maintenance.duration", duration, context); } } 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 705176e8f3e..60d66ae4d91 100644 --- a/vespajlib/src/main/java/com/yahoo/concurrent/maintenance/JobMetrics.java +++ b/vespajlib/src/main/java/com/yahoo/concurrent/maintenance/JobMetrics.java @@ -14,6 +14,6 @@ public abstract class JobMetrics { * Records completion of a run of a job. * This is guaranteed to always be called once after each maintainer run. */ - public abstract void completed(String job, double successFactor); + public abstract void completed(String job, double successFactor, long durationMs); } 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 b038106843e..1e2c0900ff7 100644 --- a/vespajlib/src/main/java/com/yahoo/concurrent/maintenance/Maintainer.java +++ b/vespajlib/src/main/java/com/yahoo/concurrent/maintenance/Maintainer.java @@ -6,6 +6,7 @@ import com.yahoo.net.HostName; import java.math.BigDecimal; import java.math.RoundingMode; +import java.time.Clock; import java.time.Duration; import java.time.Instant; import java.util.List; @@ -35,15 +36,17 @@ public abstract class Maintainer implements Runnable { private final ScheduledExecutorService service; private final AtomicBoolean shutDown = new AtomicBoolean(); private final boolean ignoreCollision; + private final Clock clock; - public Maintainer(String name, Duration interval, Instant startedAt, JobControl jobControl, + public Maintainer(String name, Duration interval, Clock clock, JobControl jobControl, JobMetrics jobMetrics, List clusterHostnames, boolean ignoreCollision) { this.name = name; this.interval = requireInterval(interval); this.jobControl = Objects.requireNonNull(jobControl); this.jobMetrics = Objects.requireNonNull(jobMetrics); this.ignoreCollision = ignoreCollision; - Objects.requireNonNull(startedAt); + this.clock = clock; + var startedAt = clock.instant(); Objects.requireNonNull(clusterHostnames); Duration initialDelay = staggeredDelay(interval, startedAt, HostName.getLocalhost(), clusterHostnames) .plus(Duration.ofSeconds(30)); // Let the system stabilize before maintenance @@ -109,6 +112,7 @@ public abstract class Maintainer implements Runnable { log.log(Level.FINE, () -> "Running " + this.getClass().getSimpleName()); double successFactor = 0; + long startTime = clock.millis(); try (var lock = jobControl.lockJob(name())) { successFactor = maintain(); } @@ -122,7 +126,8 @@ public abstract class Maintainer implements Runnable { log.log(Level.WARNING, this + " failed. Will retry in " + interval, e); } finally { - jobMetrics.completed(name(), successFactor); + long endTime = clock.millis(); + jobMetrics.completed(name(), successFactor, endTime - startTime); } 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 62fcd885494..0e183b05ee8 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 - public void completed(String job, double successFactor) { } + public void completed(String job, double successFactor, long durationMs) { } } 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 604c29e7289..bb62b1189a1 100644 --- a/vespajlib/src/test/java/com/yahoo/concurrent/maintenance/MaintainerTest.java +++ b/vespajlib/src/test/java/com/yahoo/concurrent/maintenance/MaintainerTest.java @@ -70,10 +70,12 @@ public class MaintainerTest { private static class TestJobMetrics extends JobMetrics { double successFactor = 0.0; + long durationMs = 0; @Override - public void completed(String job, double successFactor) { + public void completed(String job, double successFactor, long durationMs) { this.successFactor = successFactor; + this.durationMs = durationMs; } } 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 1946f688df6..d8191b98a51 100644 --- a/vespajlib/src/test/java/com/yahoo/concurrent/maintenance/TestMaintainer.java +++ b/vespajlib/src/test/java/com/yahoo/concurrent/maintenance/TestMaintainer.java @@ -1,6 +1,7 @@ // Copyright Yahoo. 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; import java.time.Instant; import java.util.List; @@ -15,7 +16,7 @@ class TestMaintainer extends Maintainer { private RuntimeException exceptionToThrow = null; public TestMaintainer(String name, JobControl jobControl, JobMetrics jobMetrics) { - super(name, Duration.ofDays(1), Instant.now(), jobControl, jobMetrics, List.of(), false); + super(name, Duration.ofDays(1), Clock.systemUTC(), jobControl, jobMetrics, List.of(), false); } public int totalRuns() { -- cgit v1.2.3