From 065f9aa834447ab7d61606e5ceeb06c2364fe658 Mon Sep 17 00:00:00 2001 From: Chris Earle Date: Mon, 26 Feb 2018 17:53:09 -0800 Subject: [PATCH] [Monitoring] Remove support for setting interval -1 (elastic/x-pack-elasticsearch#4035) This removes the ability to set `xpack.monitoring.collection.interval` to `-1`. Original commit: elastic/x-pack-elasticsearch@60f2db4bd19a150df7283512aa6c79887bbb64e7 --- docs/en/release-notes/7.0.0-alpha1.asciidoc | 5 +++ docs/en/settings/monitoring-settings.asciidoc | 25 ++++++------ .../xpack/monitoring/MonitoringService.java | 39 ++----------------- .../monitoring/MonitoringServiceTests.java | 30 +++++--------- 4 files changed, 31 insertions(+), 68 deletions(-) diff --git a/docs/en/release-notes/7.0.0-alpha1.asciidoc b/docs/en/release-notes/7.0.0-alpha1.asciidoc index 07f644269a3..b68970c4c9d 100644 --- a/docs/en/release-notes/7.0.0-alpha1.asciidoc +++ b/docs/en/release-notes/7.0.0-alpha1.asciidoc @@ -9,6 +9,11 @@ Machine Learning:: * The `max_running_jobs` node property is removed in this release. Use the `xpack.ml.max_open_jobs` setting instead. For more information, see <>. +Monitoring:: +* The `xpack.monitoring.collection.interval` setting can no longer be set to `-1` +to disable monitoring data collection. Use `xpack.monitoring.collection.enabled` +and set it to `false` (its default), which was added in 6.3.0. + Security:: * The fields returned as part of the mappings section by get index, get mappings, get field mappings and field capabilities API are now only the diff --git a/docs/en/settings/monitoring-settings.asciidoc b/docs/en/settings/monitoring-settings.asciidoc index c07d625af5e..fddf6a24b1d 100644 --- a/docs/en/settings/monitoring-settings.asciidoc +++ b/docs/en/settings/monitoring-settings.asciidoc @@ -41,6 +41,19 @@ Enable the collection of monitoring data. Defaults to `false`. added[6.3.0] You can update this setting through the <>. +`xpack.monitoring.collection.interval`:: + +Setting to `-1` to disable data collection is no longer supported beginning with +7.0.0. deprecated[6.3.0, Use `xpack.monitoring.collection.enabled` set to +`false` instead.] ++ +Controls how often data samples are collected. Defaults to `10s`. If you +modify the collection interval, set the `xpack.monitoring.min_interval_seconds` +option in `kibana.yml` to the same value. ++ +You can update this setting through the +<>. + `xpack.monitoring.collection.cluster.stats.timeout`:: Sets the timeout for collecting the cluster statistics. Defaults to `10s`. @@ -73,18 +86,6 @@ collect only active recoveries. Defaults to `false`. Sets the timeout for collecting the recovery information. Defaults to `10s`. -`xpack.monitoring.collection.interval`:: - -Setting to `-1` to disable data collection has been deprecated. added[6.3.0, -Use `xpack.monitoring.collection.enabled` set to `false` instead.] - -Controls how often data samples are collected. Defaults to `10s`. If you -modify the collection interval, set the `xpack.monitoring.min_interval_seconds` -option in `kibana.yml` to the same value. -+ -You can update this setting through the -<>. - `xpack.monitoring.history.duration`:: Sets the retention duration beyond which the indices created by a Monitoring diff --git a/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/MonitoringService.java b/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/MonitoringService.java index 9f27b439ce3..07d24826f86 100644 --- a/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/MonitoringService.java +++ b/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/MonitoringService.java @@ -5,14 +5,12 @@ */ package org.elasticsearch.xpack.monitoring; -import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.message.ParameterizedMessage; import org.apache.logging.log4j.util.Supplier; import org.elasticsearch.action.ActionListener; import org.elasticsearch.cluster.ClusterState; import org.elasticsearch.cluster.service.ClusterService; import org.elasticsearch.common.component.AbstractLifecycleComponent; -import org.elasticsearch.common.logging.DeprecationLogger; import org.elasticsearch.common.settings.Setting; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.unit.TimeValue; @@ -40,24 +38,6 @@ import java.util.concurrent.atomic.AtomicBoolean; */ public class MonitoringService extends AbstractLifecycleComponent { - /** - * Log a deprecation warning if {@code value} is -1. - *

- * This should be removed in 7.0. - * - * @param value The value being set for the collection interval. - */ - private static void deprecateMinusOne(final TimeValue value) { - if (TimeValue.MINUS_ONE.equals(value)) { - final DeprecationLogger deprecationLogger = new DeprecationLogger(LogManager.getLogger(MonitoringService.class)); - - deprecationLogger.deprecated( - "Setting [xpack.monitoring.collection.interval] to [-1] has been deprecated as the way to disable collection. Use " + - "[xpack.monitoring.collection.enabled] set to [false] instead." - ); - } - } - /** * Minimum value for sampling interval (1 second) */ @@ -73,17 +53,9 @@ public class MonitoringService extends AbstractLifecycleComponent { /** * Sampling interval between two collections (default to 10s) */ - public static final Setting INTERVAL = new Setting<>("xpack.monitoring.collection.interval", "10s", - (s) -> { - TimeValue value = TimeValue.parseTimeValue(s, null, "xpack.monitoring.collection.interval"); - if (TimeValue.MINUS_ONE.equals(value) || value.millis() >= MIN_INTERVAL.millis()) { - deprecateMinusOne(value); - - return value; - } - throw new IllegalArgumentException("Failed to parse monitoring interval [" + s + "], value must be >= " + MIN_INTERVAL); - }, - Setting.Property.Dynamic, Setting.Property.NodeScope); + public static final Setting INTERVAL = + Setting.timeSetting("xpack.monitoring.collection.interval", TimeValue.timeValueSeconds(10), MIN_INTERVAL, + Setting.Property.Dynamic, Setting.Property.NodeScope); /** State of the monitoring service, either started or stopped **/ private final AtomicBoolean started = new AtomicBoolean(false); @@ -129,10 +101,7 @@ public class MonitoringService extends AbstractLifecycleComponent { } public boolean isMonitoringActive() { - return isStarted() - && enabled - && interval != null - && interval.millis() >= MIN_INTERVAL.millis(); + return isStarted() && enabled; } private String threadPoolName() { diff --git a/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/MonitoringServiceTests.java b/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/MonitoringServiceTests.java index f262d56fac8..af77b3b1caf 100644 --- a/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/MonitoringServiceTests.java +++ b/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/MonitoringServiceTests.java @@ -78,12 +78,6 @@ public class MonitoringServiceTests extends ESTestCase { monitoringService.setMonitoringActive(true); assertTrue(monitoringService.isMonitoringActive()); - monitoringService.setInterval(TimeValue.MINUS_ONE); - assertFalse(monitoringService.isMonitoringActive()); - - monitoringService.setInterval(TimeValue.timeValueSeconds(10)); - assertTrue(monitoringService.isMonitoringActive()); - monitoringService.stop(); assertBusy(() -> assertFalse(monitoringService.isStarted())); assertFalse(monitoringService.isMonitoringActive()); @@ -100,31 +94,25 @@ public class MonitoringServiceTests extends ESTestCase { public void testInterval() throws Exception { final Settings settings = Settings.builder() - .put(MonitoringService.ENABLED.getKey(), true) - .put(MonitoringService.INTERVAL.getKey(), TimeValue.MINUS_ONE) + .put("xpack.monitoring.collection.interval", MonitoringService.MIN_INTERVAL) .build(); CountingExporter exporter = new CountingExporter(); monitoringService = new MonitoringService(settings, clusterService, threadPool, emptySet(), exporter); - assertWarnings( - "Setting [xpack.monitoring.collection.interval] to [-1] has been deprecated as the way to disable collection. Use " + - "[xpack.monitoring.collection.enabled] set to [false] instead."); - monitoringService.start(); assertBusy(() -> assertTrue(monitoringService.isStarted())); - assertFalse("interval -1 does not start the monitoring execution", monitoringService.isMonitoringActive()); + assertFalse("interval does not start the monitoring execution", monitoringService.isMonitoringActive()); assertEquals(0, exporter.getExportsCount()); - monitoringService.setInterval(TimeValue.timeValueSeconds(1)); + monitoringService.setMonitoringActive(true); assertTrue(monitoringService.isMonitoringActive()); + + // now the interval should take place assertBusy(() -> assertThat(exporter.getExportsCount(), greaterThan(0))); - monitoringService.setInterval(TimeValue.timeValueMillis(100)); - assertFalse(monitoringService.isMonitoringActive()); - - monitoringService.setInterval(TimeValue.MINUS_ONE); - assertFalse(monitoringService.isMonitoringActive()); + // take down threads + monitoringService.setMonitoringActive(false); } public void testSkipExecution() throws Exception { @@ -132,8 +120,8 @@ public class MonitoringServiceTests extends ESTestCase { final BlockingExporter exporter = new BlockingExporter(latch); final Settings settings = Settings.builder() - .put(MonitoringService.ENABLED.getKey(), true) - .put(MonitoringService.INTERVAL.getKey(), MonitoringService.MIN_INTERVAL) + .put("xpack.monitoring.collection.enabled", true) + .put("xpack.monitoring.collection.interval", MonitoringService.MIN_INTERVAL) .build(); monitoringService = new MonitoringService(settings, clusterService, threadPool, emptySet(), exporter);