From 8a43ad511f02f5979bd8a00c15116655441c9059 Mon Sep 17 00:00:00 2001 From: gjoranv Date: Fri, 22 Jan 2021 11:36:39 +0100 Subject: Remove metrics handling from the StateMonitor class. --- .../jdisc/state/MetricsPacketsHandler.java | 3 +- .../yahoo/container/jdisc/state/StateHandler.java | 3 +- .../yahoo/container/jdisc/state/StateMonitor.java | 104 ++------------------- .../jdisc/state/StateHandlerTestBase.java | 2 +- 4 files changed, 11 insertions(+), 101 deletions(-) diff --git a/container-core/src/main/java/com/yahoo/container/jdisc/state/MetricsPacketsHandler.java b/container-core/src/main/java/com/yahoo/container/jdisc/state/MetricsPacketsHandler.java index a3bfe53e972..3d3f0e4b677 100644 --- a/container-core/src/main/java/com/yahoo/container/jdisc/state/MetricsPacketsHandler.java +++ b/container-core/src/main/java/com/yahoo/container/jdisc/state/MetricsPacketsHandler.java @@ -143,7 +143,8 @@ public class MetricsPacketsHandler extends AbstractRequestHandler { private MetricSnapshot getSnapshot() { if (snapshotPreprocessor == null) { - return monitor.snapshot(); + // TODO: throw exception in ctor instead + return new MetricSnapshot(0L, 0L, TimeUnit.MILLISECONDS); } else { return snapshotPreprocessor.latestSnapshot(); } diff --git a/container-core/src/main/java/com/yahoo/container/jdisc/state/StateHandler.java b/container-core/src/main/java/com/yahoo/container/jdisc/state/StateHandler.java index 81959756b14..fc13159663b 100644 --- a/container-core/src/main/java/com/yahoo/container/jdisc/state/StateHandler.java +++ b/container-core/src/main/java/com/yahoo/container/jdisc/state/StateHandler.java @@ -205,7 +205,8 @@ public class StateHandler extends AbstractRequestHandler { private MetricSnapshot getSnapshot() { if (snapshotPreprocessor == null) { - return monitor.snapshot(); + // TODO: throw exception in ctor instead + return new MetricSnapshot(0L, 0L, TimeUnit.MILLISECONDS); } else { return snapshotPreprocessor.latestSnapshot(); } diff --git a/container-core/src/main/java/com/yahoo/container/jdisc/state/StateMonitor.java b/container-core/src/main/java/com/yahoo/container/jdisc/state/StateMonitor.java index 40a0ef10fbc..93c757ece18 100644 --- a/container-core/src/main/java/com/yahoo/container/jdisc/state/StateMonitor.java +++ b/container-core/src/main/java/com/yahoo/container/jdisc/state/StateMonitor.java @@ -4,25 +4,13 @@ package com.yahoo.container.jdisc.state; import com.google.inject.Inject; import com.yahoo.component.AbstractComponent; import com.yahoo.container.jdisc.config.HealthMonitorConfig; -import com.yahoo.jdisc.Timer; import com.yahoo.jdisc.application.MetricConsumer; -import com.yahoo.jdisc.core.SystemTimer; -import java.util.Map; -import java.util.Optional; -import java.util.TreeSet; -import java.util.concurrent.CopyOnWriteArrayList; -import java.util.concurrent.Executors; -import java.util.concurrent.ScheduledExecutorService; -import java.util.concurrent.ThreadFactory; -import java.util.concurrent.TimeUnit; import java.util.logging.Level; import java.util.logging.Logger; /** - * A state monitor keeps track of the current health and metrics state of a container. - * It is used by jDisc to hand out metric update API endpoints to workers through {@link #newMetricConsumer}, - * and to inspect the current accumulated state of metrics through {@link #snapshot}. + * A state monitor keeps track of the current health state of a container. * * @author Simon Thoresen Hult */ @@ -32,44 +20,20 @@ public class StateMonitor extends AbstractComponent { public enum Status {up, down, initializing} - private final CopyOnWriteArrayList consumers = new CopyOnWriteArrayList<>(); - private final Optional executor; - private final Timer timer; - private final long snapshotIntervalMs; - private volatile long lastSnapshotTimeMs; - private volatile MetricSnapshot snapshot; private volatile Status status; - private final TreeSet valueNames = new TreeSet<>(); @Inject - public StateMonitor(HealthMonitorConfig config, Timer timer) { - this(config, timer, runnable -> { - Thread thread = new Thread(runnable, "StateMonitor"); - thread.setDaemon(true); - return thread; - }); + public StateMonitor(HealthMonitorConfig config) { + this.status = Status.valueOf(config.initialStatus()); } public static StateMonitor createForTesting() { - return new StateMonitor(new HealthMonitorConfig.Builder().build(), new SystemTimer()); + return new StateMonitor(new HealthMonitorConfig.Builder().build()); } - /** Non-private only for unit testing this class. */ - StateMonitor(HealthMonitorConfig config, Timer timer, ThreadFactory threadFactory) { - this.timer = timer; - this.snapshotIntervalMs = (long)(config.snapshot_interval() * TimeUnit.SECONDS.toMillis(1)); - this.lastSnapshotTimeMs = timer.currentTimeMillis(); - this.status = Status.valueOf(config.initialStatus()); - this.executor = Optional.ofNullable(threadFactory).map(Executors::newSingleThreadScheduledExecutor); - this.executor.ifPresent(exec -> exec.scheduleAtFixedRate(this::updateSnapshot, snapshotIntervalMs, - snapshotIntervalMs, TimeUnit.MILLISECONDS)); - } - - /** Returns a metric consumer for jDisc which will write metrics back to this */ + /** TODO: Remove */ public MetricConsumer newMetricConsumer() { - StateMetricConsumer consumer = new StateMetricConsumer(); - consumers.add(consumer); - return consumer; + return new StateMetricConsumer(); } public void status(Status status) { @@ -81,60 +45,4 @@ public class StateMonitor extends AbstractComponent { public Status status() { return status; } - /** Returns the last snapshot taken of the metrics in this system */ - public MetricSnapshot snapshot() { - return snapshot; - } - - /** Returns the interval between each metrics snapshot used by this */ - public long getSnapshotIntervalMillis() { return snapshotIntervalMs; } - - /** NOTE: Non-private for unit testing only. **/ - void updateSnapshot() { - long now = timer.currentTimeMillis(); - snapshot = createSnapshot(lastSnapshotTimeMs, now); - lastSnapshotTimeMs = now; - } - - private MetricSnapshot createSnapshot(long fromMillis, long toMillis) { - MetricSnapshot snapshot = new MetricSnapshot(fromMillis, toMillis, TimeUnit.MILLISECONDS); - for (StateMetricConsumer consumer : consumers) { - snapshot.add(consumer.createSnapshot()); - } - updateNames(snapshot); - return snapshot; - } - - private void updateNames(MetricSnapshot current) { - TreeSet seen = new TreeSet<>(); - for (Map.Entry dimensionAndMetric : current) { - for (Map.Entry nameAndMetric : dimensionAndMetric.getValue()) { - seen.add(nameAndMetric.getKey()); - } - } - synchronized (valueNames) { - for (String name : valueNames) { - if (!seen.contains(name)) { - current.add((MetricDimensions) StateMetricConsumer.NULL_CONTEXT, name, 0); - } - } - valueNames.addAll(seen); - } - } - - @Override - public void deconstruct() { - executor.ifPresent(exec -> { - exec.shutdown(); - try { - exec.awaitTermination(5, TimeUnit.SECONDS); - } catch (InterruptedException e) { - } - - if (!exec.isTerminated()) { - log.warning("StateMonitor failed to terminate within 5 seconds of interrupt signal. Ignoring."); - } - }); - } - } diff --git a/container-core/src/test/java/com/yahoo/container/jdisc/state/StateHandlerTestBase.java b/container-core/src/test/java/com/yahoo/container/jdisc/state/StateHandlerTestBase.java index 6f1eb5359d0..68c1ae10ec6 100644 --- a/container-core/src/test/java/com/yahoo/container/jdisc/state/StateHandlerTestBase.java +++ b/container-core/src/test/java/com/yahoo/container/jdisc/state/StateHandlerTestBase.java @@ -55,7 +55,7 @@ public class StateHandlerTestBase { snapshotProvider = new MockSnapshotProvider(); snapshotProviderRegistry = new ComponentRegistry<>(); snapshotProviderRegistry.register(new ComponentId("foo"), snapshotProvider); - monitor = new StateMonitor(healthMonitorConfig, timer, null); + monitor = new StateMonitor(healthMonitorConfig); } String requestAsString(String requestUri) { -- cgit v1.2.3