Revert "Jonmv/job runner thread metrics"

13 files changed, 19 insertions, 130 deletions

diff --git a/container-core/src/main/java/com/yahoo/container/handler/threadpool/ContainerThreadpoolImpl.java b/container-core/src/main/java/com/yahoo/container/handler/threadpool/ContainerThreadpoolImpl.java
index f92d218390f..befbda28ac0 100644
--- a/container-core/src/main/java/com/yahoo/container/handler/threadpool/ContainerThreadpoolImpl.java
+++ b/container-core/src/main/java/com/yahoo/container/handler/threadpool/ContainerThreadpoolImpl.java
@@ -54,7 +54,7 @@ public class ContainerThreadpoolImpl extends AbstractComponent implements AutoCl
                         createQueue(queueSize),
                         ThreadFactoryFactory.getThreadFactory(name),
                         threadPoolMetric);
-        // Pre-start needed, if not all threads will be created by the fist N tasks and hence they might also
+        // Prestart needed, if not all threads will be created by the fist N tasks and hence they might also
         // get the dreaded thread locals initialized even if they will never run.
         // That counters what we want to achieve with the Q that will prefer thread locality.
         executor.prestartAllCoreThreads();
diff --git a/container-core/src/main/java/com/yahoo/container/handler/threadpool/WorkerCompletionTimingThreadPoolExecutor.java b/container-core/src/main/java/com/yahoo/container/handler/threadpool/WorkerCompletionTimingThreadPoolExecutor.java
index cee2cc54b5b..e3c2c78abec 100644
--- a/container-core/src/main/java/com/yahoo/container/handler/threadpool/WorkerCompletionTimingThreadPoolExecutor.java
+++ b/container-core/src/main/java/com/yahoo/container/handler/threadpool/WorkerCompletionTimingThreadPoolExecutor.java
@@ -8,7 +8,8 @@ import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicLong;
 
 /**
- * A thread pool executor which maintains the last time a worker completed.
+ * A thread pool executor which maintains the last time a worker completed
+ * package private for testing
  *
  * @author Steinar Knutsen
  * @author baldersheim
diff --git a/container-core/src/main/java/com/yahoo/container/protect/ProcessTerminator.java b/container-core/src/main/java/com/yahoo/container/protect/ProcessTerminator.java
index 3c44ba2eaa6..315dc21ec38 100644
--- a/container-core/src/main/java/com/yahoo/container/protect/ProcessTerminator.java
+++ b/container-core/src/main/java/com/yahoo/container/protect/ProcessTerminator.java
@@ -4,8 +4,8 @@ package com.yahoo.container.protect;
 import com.yahoo.protect.Process;
 
 /**
- * An injectable terminator of the Java VM.
- * Components that encounter conditions where the VM should be terminated should
+ * An injectable terminator of the Java vm.
+ * Components that encounters conditions where the vm should be terminated should
  * request an instance of this injected. That makes termination testable
  * as tests can create subclasses of this which register the termination request
  * rather than terminating.
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunner.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunner.java
index 1080b379c4d..662b4018a34 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunner.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunner.java
@@ -727,8 +727,6 @@ public class InternalStepRunner implements StepRunner {
 
                     DeploymentSpec spec = controller.applications().requireApplication(TenantAndApplicationId.from(id.application())).deploymentSpec();
                     boolean requireTests = spec.steps().stream().anyMatch(step -> step.concerns(id.type().environment()));
-                    logger.log(WARNING, "No tests were actually run, but this test suite is explicitly declared in 'deployment.xml'. " +
-                                        "Either add tests, ensure they're correctly configured, or remove the test declaration.");
                     return Optional.of(requireTests ? testFailure : noTests);
                 }
             case SUCCESS:
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/JobRunner.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/JobRunner.java
index 0f482b1a015..6f00ff39637 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/JobRunner.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/JobRunner.java
@@ -1,9 +1,7 @@
 // Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.vespa.hosted.controller.maintenance;
 
-import ai.vespa.metrics.ControllerMetrics;
 import com.yahoo.concurrent.DaemonThreadFactory;
-import com.yahoo.jdisc.Metric;
 import com.yahoo.vespa.hosted.controller.Controller;
 import com.yahoo.vespa.hosted.controller.api.integration.deployment.RunId;
 import com.yahoo.vespa.hosted.controller.deployment.InternalStepRunner;
@@ -13,14 +11,11 @@ import com.yahoo.vespa.hosted.controller.deployment.Step;
 import com.yahoo.vespa.hosted.controller.deployment.StepRunner;
 
 import java.time.Duration;
-import java.util.Map;
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Executors;
-import java.util.concurrent.ScheduledExecutorService;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.TimeoutException;
 import java.util.concurrent.atomic.AtomicBoolean;
-import java.util.concurrent.atomic.AtomicInteger;
 import java.util.logging.Level;
 import java.util.logging.Logger;
 
@@ -37,29 +32,22 @@ public class JobRunner extends ControllerMaintainer {
     private final JobController jobs;
     private final ExecutorService executors;
     private final StepRunner runner;
-    private final Metrics metrics;
 
     public JobRunner(Controller controller, Duration duration) {
-        this(controller, duration, Executors.newFixedThreadPool(32, new DaemonThreadFactory("job-runner-")),
-             new InternalStepRunner(controller));
+        this(controller, duration, Executors.newFixedThreadPool(32, new DaemonThreadFactory("job-runner-")), new InternalStepRunner(controller));
     }
 
     public JobRunner(Controller controller, Duration duration, ExecutorService executors, StepRunner runner) {
-        this(controller, duration, executors, runner, new Metrics(controller.metric(), Duration.ofMillis(100)));
-    }
-
-    JobRunner(Controller controller, Duration duration, ExecutorService executors, StepRunner runner, Metrics metrics) {
         super(controller, duration);
         this.jobs = controller.jobController();
         this.jobs.setRunner(this::advance);
         this.executors = executors;
         this.runner = runner;
-        this.metrics = metrics;
     }
 
     @Override
     protected double maintain() {
-        execute(() -> jobs.active().forEach(this::advance));
+        executors.execute(() -> jobs.active().forEach(this::advance));
         jobs.collectGarbage();
         return 1.0;
     }
@@ -67,7 +55,6 @@ public class JobRunner extends ControllerMaintainer {
     @Override
     public void shutdown() {
         super.shutdown();
-        metrics.shutdown();
         executors.shutdown();
     }
 
@@ -96,14 +83,14 @@ public class JobRunner extends ControllerMaintainer {
         jobs.locked(id, run -> {
             if (   ! run.hasFailed()
                 &&   controller().clock().instant().isAfter(run.sleepUntil().orElse(run.start()).plus(jobTimeout)))
-                execute(() -> {
+                executors.execute(() -> {
                     jobs.abort(run.id(), "job timeout of " + jobTimeout + " reached", false);
                     advance(run.id());
                 });
             else if (run.readySteps().isEmpty())
-                execute(() -> finish(run.id()));
+                executors.execute(() -> finish(run.id()));
             else if (run.hasFailed() || run.sleepUntil().map(sleepUntil -> ! sleepUntil.isAfter(controller().clock().instant())).orElse(true))
-                run.readySteps().forEach(step -> execute(() -> advance(run.id(), step)));
+                run.readySteps().forEach(step -> executors.execute(() -> advance(run.id(), step)));
 
             return null;
         });
@@ -158,39 +145,4 @@ public class JobRunner extends ControllerMaintainer {
         }
     }
 
-    private void execute(Runnable task) {
-        metrics.queued.incrementAndGet();
-        executors.execute(() -> {
-            metrics.queued.decrementAndGet();
-            metrics.active.incrementAndGet();
-            try { task.run(); }
-            finally { metrics.active.decrementAndGet(); }
-        });
-    }
-
-    static class Metrics {
-
-        private final AtomicInteger queued = new AtomicInteger();
-        private final AtomicInteger active = new AtomicInteger();
-        private final ScheduledExecutorService reporter = Executors.newSingleThreadScheduledExecutor(new DaemonThreadFactory("job-runner-metrics-"));
-        private final Metric metric;
-        private final Metric.Context context;
-
-        Metrics(Metric metric, Duration interval) {
-            this.metric = metric;
-            this.context = metric.createContext(Map.of());
-            reporter.scheduleAtFixedRate(this::report, interval.toMillis(), interval.toMillis(), TimeUnit.MILLISECONDS);
-        }
-
-        void report() {
-            metric.set(ControllerMetrics.DEPLOYMENT_JOBS_QUEUED.baseName(), queued.get(), context);
-            metric.set(ControllerMetrics.DEPLOYMENT_JOBS_ACTIVE.baseName(), active.get(), context);
-        }
-
-        void shutdown() {
-            reporter.shutdown();
-        }
-
-    }
-
 }
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTester.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTester.java
index c16234b3948..1cc549ec6ca 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTester.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTester.java
@@ -75,7 +75,8 @@ public class DeploymentTester {
         tester = controllerTester;
         jobs = tester.controller().jobController();
         cloud = (MockTesterCloud) tester.controller().jobController().cloud();
-        runner = new JobRunner(tester.controller(), maintenanceInterval, JobRunnerTest.inThreadExecutor(), new InternalStepRunner(tester.controller()));
+        runner = new JobRunner(tester.controller(), maintenanceInterval, JobRunnerTest.inThreadExecutor(),
+                               new InternalStepRunner(tester.controller()));
         upgrader = new Upgrader(tester.controller(), maintenanceInterval);
         upgrader.setUpgradesPerMinute(1); // Anything that makes it at least one for any maintenance period is fine.
         readyJobsTrigger = new ReadyJobsTrigger(tester.controller(), maintenanceInterval);
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/JobRunnerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/JobRunnerTest.java
index 53be33fdfd7..e87d4f1f3f0 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/JobRunnerTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/JobRunnerTest.java
@@ -1,10 +1,8 @@
 // Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.vespa.hosted.controller.maintenance;
 
-import ai.vespa.metrics.ControllerMetrics;
 import com.yahoo.component.Version;
 import com.yahoo.config.provision.ApplicationId;
-import com.yahoo.jdisc.test.MockMetric;
 import com.yahoo.vespa.hosted.controller.api.integration.deployment.JobId;
 import com.yahoo.vespa.hosted.controller.api.integration.deployment.JobType;
 import com.yahoo.vespa.hosted.controller.api.integration.deployment.RevisionId;
@@ -24,7 +22,6 @@ import com.yahoo.vespa.hosted.controller.deployment.StepRunner;
 import com.yahoo.vespa.hosted.controller.deployment.Submission;
 import com.yahoo.vespa.hosted.controller.deployment.Versions;
 import com.yahoo.vespa.hosted.controller.integration.MetricsMock;
-import com.yahoo.vespa.hosted.controller.maintenance.JobRunner.Metrics;
 import org.junit.jupiter.api.Test;
 
 import java.time.Duration;
@@ -40,9 +37,7 @@ import java.util.concurrent.BrokenBarrierException;
 import java.util.concurrent.CyclicBarrier;
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Executors;
-import java.util.concurrent.LinkedBlockingQueue;
 import java.util.concurrent.Phaser;
-import java.util.concurrent.ThreadPoolExecutor;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.TimeoutException;
 import java.util.concurrent.atomic.AtomicBoolean;
@@ -126,60 +121,6 @@ public class JobRunnerTest {
     }
 
     @Test
-    void metrics() {
-        Phaser phaser = new Phaser(4);
-        StepRunner runner = (step, id) -> {
-            phaser.arrive();
-            phaser.arriveAndAwaitAdvance();
-            return Optional.of(running);
-        };
-        ExecutorService executor = new ThreadPoolExecutor(3, 3, 0, TimeUnit.SECONDS, new LinkedBlockingQueue<>(), (task, pool) -> task.run());
-        DeploymentTester tester = new DeploymentTester();
-        MockMetric metric = new MockMetric();
-        Metrics metrics = new Metrics(metric, Duration.ofDays(1));
-        JobRunner jobs = new JobRunner(tester.controller(), Duration.ofDays(1), executor, runner, metrics);
-        tester.newDeploymentContext().submit();
-
-        assertEquals(Map.of(), metric.metrics());
-        metrics.report();
-        assertEquals(Map.of(ControllerMetrics.DEPLOYMENT_JOBS_QUEUED.baseName(),
-                            Map.of(Map.of(), 0.0),
-                            ControllerMetrics.DEPLOYMENT_JOBS_ACTIVE.baseName(),
-                            Map.of(Map.of(), 0.0)),
-                     metric.metrics());
-        tester.triggerJobs();
-
-        assertEquals(2, tester.jobs().active().size());
-        jobs.maintain();
-        phaser.arriveAndAwaitAdvance();
-        metrics.report();
-        assertEquals(Map.of(ControllerMetrics.DEPLOYMENT_JOBS_QUEUED.baseName(),
-                            Map.of(Map.of(), 1.0),
-                            ControllerMetrics.DEPLOYMENT_JOBS_ACTIVE.baseName(),
-                            Map.of(Map.of(), 3.0)),
-                     metric.metrics());
-
-        phaser.arrive();
-        phaser.arriveAndAwaitAdvance();
-        metrics.report();
-        assertEquals(Map.of(ControllerMetrics.DEPLOYMENT_JOBS_QUEUED.baseName(),
-                            Map.of(Map.of(), 1.0),
-                            ControllerMetrics.DEPLOYMENT_JOBS_ACTIVE.baseName(),
-                            Map.of(Map.of(), 3.0)),
-                     metric.metrics());
-
-        jobs.shutdown();
-        phaser.forceTermination();
-        jobs.awaitShutdown();
-        metrics.report();
-        assertEquals(Map.of(ControllerMetrics.DEPLOYMENT_JOBS_QUEUED.baseName(),
-                            Map.of(Map.of(), 0.0),
-                            ControllerMetrics.DEPLOYMENT_JOBS_ACTIVE.baseName(),
-                            Map.of(Map.of(), 0.0)),
-                     metric.metrics());
-    }
-
-    @Test
     void stepLogic() {
         DeploymentTester tester = new DeploymentTester();
         JobController jobs = tester.controller().jobController();
diff --git a/documentapi/src/main/java/com/yahoo/documentapi/messagebus/protocol/AdaptiveLoadBalancer.java b/documentapi/src/main/java/com/yahoo/documentapi/messagebus/protocol/AdaptiveLoadBalancer.java
index ae934857e2c..4a4cf0fd5c8 100644
--- a/documentapi/src/main/java/com/yahoo/documentapi/messagebus/protocol/AdaptiveLoadBalancer.java
+++ b/documentapi/src/main/java/com/yahoo/documentapi/messagebus/protocol/AdaptiveLoadBalancer.java
@@ -7,7 +7,7 @@ import java.util.List;
 import java.util.Random;
 
 /**
- * Will pick 2 random candidates and select the one with the least pending operations.
+ * Will pick 2 random candidates and select the one with least pending operations.
  *
  * @author baldersheim
  */
diff --git a/documentapi/src/main/java/com/yahoo/documentapi/messagebus/protocol/LocalServicePolicy.java b/documentapi/src/main/java/com/yahoo/documentapi/messagebus/protocol/LocalServicePolicy.java
index 4f8227b35a0..ddd04a3ca53 100755
--- a/documentapi/src/main/java/com/yahoo/documentapi/messagebus/protocol/LocalServicePolicy.java
+++ b/documentapi/src/main/java/com/yahoo/documentapi/messagebus/protocol/LocalServicePolicy.java
@@ -23,7 +23,7 @@ public class LocalServicePolicy implements DocumentProtocolRoutingPolicy {
     private final Map<String, CacheEntry> cache = new HashMap<>();
 
     /**
-     * Constructs a policy that will choose local services that match the slobrok pattern in which this policy occurred.
+     * Constructs a policy that will choose local services that match the slobrok pattern in which this policy occured.
      * If no local service can be found, this policy simply returns the asterisk to allow the network to choose any.
      *
      * @param param The address to use for this, if empty this will resolve to hostname.
diff --git a/messagebus/src/main/java/com/yahoo/messagebus/ErrorCode.java b/messagebus/src/main/java/com/yahoo/messagebus/ErrorCode.java
index 88e3e1a89bc..8fab523d4ae 100644
--- a/messagebus/src/main/java/com/yahoo/messagebus/ErrorCode.java
+++ b/messagebus/src/main/java/com/yahoo/messagebus/ErrorCode.java
@@ -20,7 +20,7 @@ public final class ErrorCode {
     /** No addresses found for the services of the message route. */
     public static final int NO_ADDRESS_FOR_SERVICE = TRANSIENT_ERROR + 2;
 
-    /** A connection problem occurred while sending. */
+    /** A connection problem occured while sending. */
     public static final int CONNECTION_ERROR       = TRANSIENT_ERROR + 3;
 
     /** The session specified for the message is unknown. */
@@ -50,10 +50,10 @@ public final class ErrorCode {
     /** No services found for the message route. */
     public static final int NO_SERVICES_FOR_ROUTE  = FATAL_ERROR + 3;
 
-    /** An error occurred while encoding the message. */
+    /** An error occured while encoding the message. */
     public static final int ENCODE_ERROR           = FATAL_ERROR + 5;
 
-    /** A fatal network error occurred while sending. */
+    /** A fatal network error occured while sending. */
     public static final int NETWORK_ERROR          = FATAL_ERROR + 6;
 
     /** The protocol specified for the message is unknown. */
@@ -77,7 +77,7 @@ public final class ErrorCode {
     /** Exception thrown by routing policy. */
     public static final int POLICY_ERROR           = FATAL_ERROR + 13;
 
-    /** An error occurred while sequencing a message. */
+    /** An error occured while sequencing a message. */
     public static final int SEQUENCE_ERROR         = FATAL_ERROR + 14;
 
     /** An application specific non-recoverable error. */
diff --git a/messagebus/src/main/java/com/yahoo/messagebus/routing/RoutingContext.java b/messagebus/src/main/java/com/yahoo/messagebus/routing/RoutingContext.java
index 18b5de34bb4..227dd546ad8 100755
--- a/messagebus/src/main/java/com/yahoo/messagebus/routing/RoutingContext.java
+++ b/messagebus/src/main/java/com/yahoo/messagebus/routing/RoutingContext.java
@@ -19,7 +19,7 @@ public class RoutingContext {
 
     private final RoutingNode node;
     private final int directive;
-    private final Set<Integer> consumableErrors = new HashSet<>();
+    private final Set<Integer> consumableErrors = new HashSet<Integer>();
     private boolean selectOnRetry = true;
     private Object context = null;
 
diff --git a/metrics/src/main/java/ai/vespa/metrics/ControllerMetrics.java b/metrics/src/main/java/ai/vespa/metrics/ControllerMetrics.java
index f03c54aa822..3676be90cd4 100644
--- a/metrics/src/main/java/ai/vespa/metrics/ControllerMetrics.java
+++ b/metrics/src/main/java/ai/vespa/metrics/ControllerMetrics.java
@@ -9,8 +9,6 @@ public enum ControllerMetrics implements VespaMetrics {
     ATHENZ_REQUEST_ERROR("athenz.request.error", Unit.REQUEST, "Controller: Athenz request error"),
     ARCHIVE_BUCKET_COUNT("archive.bucketCount", Unit.BUCKET, "Controller: Archive bucket count"),
 
-    DEPLOYMENT_JOBS_QUEUED("deployment.jobsQueued", Unit.TASK, "The number of deployment jobs queued"),
-    DEPLOYMENT_JOBS_ACTIVE("deployment.jobsActive", Unit.TASK, "The number of deployment jobs active"),
     DEPLOYMENT_START("deployment.start", Unit.DEPLOYMENT, "The number of started deployment jobs"),
     DEPLOYMENT_NODE_ALLOCATION_FAILURE("deployment.nodeAllocationFailure", Unit.DEPLOYMENT, "The number of deployments failed due to node allocation failures"),
     DEPLOYMENT_ENDPOINT_CERTIFICATE_TIMEOUT("deployment.endpointCertificateTimeout", Unit.DEPLOYMENT, "The number of deployments failed due to timeout acquiring endpoint certificate"),
diff --git a/metrics/src/main/java/ai/vespa/metrics/set/InfrastructureMetricSet.java b/metrics/src/main/java/ai/vespa/metrics/set/InfrastructureMetricSet.java
index 9443a08e28b..6bffddb885a 100644
--- a/metrics/src/main/java/ai/vespa/metrics/set/InfrastructureMetricSet.java
+++ b/metrics/src/main/java/ai/vespa/metrics/set/InfrastructureMetricSet.java
@@ -143,8 +143,6 @@ public class InfrastructureMetricSet {
         addMetric(metrics, ControllerMetrics.ARCHIVE_BUCKET_COUNT.max());
         addMetric(metrics, ControllerMetrics.BILLING_TENANTS.max());
 
-        addMetric(metrics, ControllerMetrics.DEPLOYMENT_JOBS_QUEUED, EnumSet.of(count, sum));
-        addMetric(metrics, ControllerMetrics.DEPLOYMENT_JOBS_ACTIVE, EnumSet.of(count, sum));
         addMetric(metrics, ControllerMetrics.DEPLOYMENT_ABORT.count());
         addMetric(metrics, ControllerMetrics.DEPLOYMENT_AVERAGE_DURATION.max());
         addMetric(metrics, ControllerMetrics.DEPLOYMENT_CONVERGENCE_FAILURE.count());