diff options
3 files changed, 50 insertions, 23 deletions
diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/monitoring/Metrics.java b/configserver/src/main/java/com/yahoo/vespa/config/server/monitoring/Metrics.java index 87e9fa287a4..249c8639ea9 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/monitoring/Metrics.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/monitoring/Metrics.java @@ -3,6 +3,8 @@ package com.yahoo.vespa.config.server.monitoring; import com.google.inject.Inject; import com.yahoo.cloud.config.ZookeeperServerConfig; +import com.yahoo.component.AbstractComponent; +import com.yahoo.concurrent.DaemonThreadFactory; import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.TenantName; import com.yahoo.container.jdisc.config.HealthMonitorConfig; @@ -14,14 +16,17 @@ import com.yahoo.statistics.Counter; import java.util.*; import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.ScheduledExecutorService; +import java.util.concurrent.ScheduledThreadPoolExecutor; +import java.util.concurrent.TimeUnit; /** - * Statistics for server. The statistics framework takes care of logging. + * Metrics for config server. The statistics framework takes care of logging. * * @author Harald Musum - * @since 4.2 */ -public class Metrics extends TimerTask implements MetricUpdaterFactory { +public class Metrics extends AbstractComponent implements MetricUpdaterFactory, Runnable { private static final java.util.logging.Logger log = java.util.logging.Logger.getLogger(Metrics.class.getName()); private static final String METRIC_REQUESTS = getMetricName("requests"); @@ -33,23 +38,34 @@ public class Metrics extends TimerTask implements MetricUpdaterFactory { private final Counter failedRequests; private final Counter procTimeCounter; private final Metric metric; - private final ZKMetricUpdater zkMetricUpdater; + private final Optional<ZKMetricUpdater> zkMetricUpdater; // TODO The map is the key for now private final Map<Map<String, String>, MetricUpdater> metricUpdaters = new ConcurrentHashMap<>(); - private final Timer timer = new Timer(); + private final Optional<ScheduledExecutorService> executorService; @Inject public Metrics(Metric metric, Statistics statistics, HealthMonitorConfig healthMonitorConfig, ZookeeperServerConfig zkServerConfig) { + this(metric, statistics, healthMonitorConfig, zkServerConfig, true); + } + + private Metrics(Metric metric, Statistics statistics, HealthMonitorConfig healthMonitorConfig, + ZookeeperServerConfig zkServerConfig, boolean createZkMetricUpdater) { this.metric = metric; requests = createCounter(METRIC_REQUESTS, statistics); failedRequests = createCounter(METRIC_FAILED_REQUESTS, statistics); procTimeCounter = createCounter("procTime", statistics); - log.log(LogLevel.DEBUG, "Metric update interval is " + healthMonitorConfig.snapshot_interval() + " seconds"); - long intervalMs = (long) (healthMonitorConfig.snapshot_interval() * 1000); - timer.scheduleAtFixedRate(this, 20000, intervalMs); - zkMetricUpdater = new ZKMetricUpdater(zkServerConfig, 19500, intervalMs); + if (createZkMetricUpdater) { + log.log(LogLevel.DEBUG, "Metric update interval is " + healthMonitorConfig.snapshot_interval() + " seconds"); + long intervalMs = (long) (healthMonitorConfig.snapshot_interval() * 1000); + executorService = Optional.of(new ScheduledThreadPoolExecutor(1, new DaemonThreadFactory("configserver-metrics"))); + executorService.get().scheduleAtFixedRate(this, 20000, intervalMs, TimeUnit.MILLISECONDS); + zkMetricUpdater = Optional.of(new ZKMetricUpdater(zkServerConfig, 19500, intervalMs)); + } else { + executorService = Optional.empty(); + zkMetricUpdater = Optional.empty(); + } } public static Metrics createTestMetrics() { @@ -58,14 +74,13 @@ public class Metrics extends TimerTask implements MetricUpdaterFactory { HealthMonitorConfig.Builder builder = new HealthMonitorConfig.Builder(); builder.snapshot_interval(60.0); ZookeeperServerConfig.Builder zkBuilder = new ZookeeperServerConfig.Builder().myid(1); - return new Metrics(metric, statistics, new HealthMonitorConfig(builder), new ZookeeperServerConfig(zkBuilder)); + return new Metrics(metric, statistics, new HealthMonitorConfig(builder), new ZookeeperServerConfig(zkBuilder), false); } private Counter createCounter(String name, Statistics statistics) { return new Counter(name, statistics, false); } - void incrementRequests(Metric.Context metricContext) { requests.increment(1); metric.add(METRIC_REQUESTS, 1, metricContext); @@ -126,8 +141,12 @@ public class Metrics extends TimerTask implements MetricUpdaterFactory { } } setRegularMetrics(); - zkMetricUpdater.getZKMetrics().forEach((attr, val) -> metric.set(attr, val, null)); - timer.purge(); + zkMetricUpdater.ifPresent(updater -> updater.getZKMetrics().forEach((attr, val) -> metric.set(attr, val, null))); + } + + public void deconstruct() { + executorService.ifPresent(ExecutorService::shutdown); + zkMetricUpdater.ifPresent(ZKMetricUpdater::shutdown); } private void setRegularMetrics() { diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/monitoring/ZKMetricUpdater.java b/configserver/src/main/java/com/yahoo/vespa/config/server/monitoring/ZKMetricUpdater.java index b2813be5456..62da6fcffbe 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/monitoring/ZKMetricUpdater.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/monitoring/ZKMetricUpdater.java @@ -2,6 +2,7 @@ package com.yahoo.vespa.config.server.monitoring; import com.yahoo.cloud.config.ZookeeperServerConfig; +import com.yahoo.concurrent.DaemonThreadFactory; import java.io.ByteArrayOutputStream; import java.io.IOException; @@ -17,6 +18,8 @@ import java.util.Timer; import java.util.TimerTask; import java.util.concurrent.ExecutionException; import java.util.concurrent.Future; +import java.util.concurrent.ScheduledExecutorService; +import java.util.concurrent.ScheduledThreadPoolExecutor; import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeoutException; import java.util.concurrent.atomic.AtomicReference; @@ -26,7 +29,7 @@ import java.util.regex.Pattern; import static com.yahoo.vespa.config.server.monitoring.Metrics.getMetricName; -public class ZKMetricUpdater extends TimerTask { +public class ZKMetricUpdater implements Runnable { private static final Logger log = Logger.getLogger(ZKMetricUpdater.class.getName()); public static final String METRIC_ZK_ZNODES = getMetricName("zkZNodes"); @@ -35,19 +38,19 @@ public class ZKMetricUpdater extends TimerTask { public static final String METRIC_ZK_CONNECTIONS = getMetricName("zkConnections"); public static final String METRIC_ZK_OUTSTANDING_REQUESTS = getMetricName("zkOutstandingRequests"); - private final int CONNECTION_TIMEOUT_MS = 500; - private final int WRITE_TIMEOUT_MS = 250; - private final int READ_TIMEOUT_MS = 500; + private static final int CONNECTION_TIMEOUT_MS = 500; + private static final int WRITE_TIMEOUT_MS = 250; + private static final int READ_TIMEOUT_MS = 500; private AtomicReference<Map<String, Long>> zkMetrics = new AtomicReference<>(new HashMap<>()); - private final Timer timer = new Timer(); + private final ScheduledExecutorService executorService; private final int zkPort; public ZKMetricUpdater(ZookeeperServerConfig zkServerConfig, long delayMS, long intervalMS) { this.zkPort = zkServerConfig.clientPort(); - if (intervalMS > 0) { - timer.scheduleAtFixedRate(this, delayMS, intervalMS); - } + if (intervalMS <= 0 ) throw new IllegalArgumentException("interval must be positive, was " + intervalMS + " ms"); + this.executorService = new ScheduledThreadPoolExecutor(1, new DaemonThreadFactory("zkmetricupdater")); + this.executorService.scheduleAtFixedRate(this, delayMS, intervalMS, TimeUnit.MILLISECONDS); } private void setMetricAttribute(String attribute, long value, Map<String, Long> data) { @@ -74,7 +77,10 @@ public class ZKMetricUpdater extends TimerTask { public void run() { Optional<String> report = retrieveReport(); report.ifPresent(this::parseReport); - timer.purge(); + } + + public void shutdown() { + executorService.shutdown(); } private Optional<String> retrieveReport() { diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/monitoring/ZKMetricUpdaterTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/monitoring/ZKMetricUpdaterTest.java index ed089109759..607a2dca6c6 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/monitoring/ZKMetricUpdaterTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/monitoring/ZKMetricUpdaterTest.java @@ -57,12 +57,14 @@ public class ZKMetricUpdaterTest { assertThat(reportedMetrics.get(ZKMetricUpdater.METRIC_ZK_LATENCY_MAX), equalTo(1234L)); assertThat(reportedMetrics.get(ZKMetricUpdater.METRIC_ZK_OUTSTANDING_REQUESTS), equalTo(12L)); assertThat(reportedMetrics.get(ZKMetricUpdater.METRIC_ZK_ZNODES), equalTo(4L)); + + updater.shutdown(); } private ZKMetricUpdater buildUpdater() { ZookeeperServerConfig zkServerConfig = new ZookeeperServerConfig( new ZookeeperServerConfig.Builder().clientPort(serverPort).myid(12345)); - return new ZKMetricUpdater(zkServerConfig, 0, -1); + return new ZKMetricUpdater(zkServerConfig, 0, 100000); } private void setupTcpServer(Supplier<String> reportProvider) throws IOException { |