Deprecations for single data node setting (#73733)

In #55805, we added a setting to allow single data node clusters to respect the high watermark. This commit adds deprecation warnings to 7.x to ensure we can make only true valid in 8.0. Co-authored-by: David Turner <[email protected]>
elastic · Jun 7, 2021 · 3e59bc0 · 3e59bc0
1 parent eba76db
commit 3e59bc0
Show file tree

Hide file tree

Showing 8 changed files with 224 additions and 42 deletions.
diff --git a/client/rest-high-level/build.gradle b/client/rest-high-level/build.gradle
@@ -116,4 +116,6 @@ testClusters.all {
 
   setting 'xpack.searchable.snapshot.shared_cache.size', '1mb'
   setting 'xpack.searchable.snapshot.shared_cache.region_size', '16kb'
+
+  setting 'cluster.routing.allocation.disk.watermark.enable_for_single_data_node', 'true'
 }
diff --git a/docs/reference/migration/migrate_7_14.asciidoc b/docs/reference/migration/migrate_7_14.asciidoc
@@ -55,4 +55,31 @@ cluster then you must upgrade the affected nodes. Once upgraded, they will join
 the cluster again.
 ====
 
+[discrete]
+[[breaking_714_core_deprecations]]
+=== Core deprecations
+
+[discrete]
+[[deprecate-single-data-node-watermark]]
+.Setting `cluster.routing.allocation.disk.watermark.enable_for_single_data_node=false` is deprecated.
+[%collapsible]
+====
+*Details* +
+The setting `cluster.routing.allocation.disk.watermark.enable_for_single_data_node`
+should never be explicitly set to false. In 8.0, the only legal value will be
+true. In a future release, the setting will be removed completely, with same
+behavior as if the setting was `true`.
+
+*Impact* +
+
+If your cluster has a single data node then set
+`cluster.routing.allocation.disk.watermark.enable_for_single_data_node: true`
+to opt in to the future behaviour today. If you wish to disable the disk
+watermarks then set `cluster.routing.allocation.disk.threshold_enabled: false`.
+
+If your cluster has multiple data nodes then the
+`cluster.routing.allocation.disk.watermark.enable_for_single_data_node` setting
+has no effect and you should discontinue its use.
+====
+
 // end::notable-breaking-changes[]
diff --git a/.../main/java/org/elasticsearch/cluster/routing/allocation/decider/DiskThresholdDecider.java b/.../main/java/org/elasticsearch/cluster/routing/allocation/decider/DiskThresholdDecider.java
@@ -26,6 +26,8 @@
 import org.elasticsearch.cluster.routing.allocation.RoutingAllocation;
 import org.elasticsearch.common.Strings;
 import org.elasticsearch.common.collect.ImmutableOpenMap;
+import org.elasticsearch.common.logging.DeprecationCategory;
+import org.elasticsearch.common.logging.DeprecationLogger;
 import org.elasticsearch.common.settings.ClusterSettings;
 import org.elasticsearch.common.settings.Setting;
 import org.elasticsearch.common.settings.Settings;
@@ -35,6 +37,7 @@
 import org.elasticsearch.snapshots.SnapshotShardSizeInfo;
 
 import java.util.List;
+import java.util.Map;
 import java.util.Set;
 
 import static org.elasticsearch.cluster.routing.allocation.DiskThresholdSettings.CLUSTER_ROUTING_ALLOCATION_HIGH_DISK_WATERMARK_SETTING;
@@ -66,11 +69,28 @@
 public class DiskThresholdDecider extends AllocationDecider {
 
     private static final Logger logger = LogManager.getLogger(DiskThresholdDecider.class);
+    private static final DeprecationLogger deprecationLogger = DeprecationLogger.getLogger(DiskThresholdDecider.class);
 
     public static final String NAME = "disk_threshold";
 
     public static final Setting<Boolean> ENABLE_FOR_SINGLE_DATA_NODE =
-        Setting.boolSetting("cluster.routing.allocation.disk.watermark.enable_for_single_data_node", false, Setting.Property.NodeScope);
+        Setting.boolSetting("cluster.routing.allocation.disk.watermark.enable_for_single_data_node", false,
+            new Setting.Validator<Boolean>() {
+                @Override
+                public void validate(Boolean value) {
+                    // empty
+                }
+
+                @Override
+                public void validate(Boolean value, Map<Setting<?>, Object> settings, boolean isPresent) {
+                    if (value == Boolean.FALSE && isPresent) {
+                        deprecationLogger.deprecate(DeprecationCategory.SETTINGS, "watermark_enable_for_single_data_node",
+                            "setting [{}=false] is deprecated and will not be available in a future version",
+                            ENABLE_FOR_SINGLE_DATA_NODE.getKey());
+                    }
+                }
+            },
+            Setting.Property.NodeScope);
 
     public static final Setting<Boolean> SETTING_IGNORE_DISK_WATERMARKS =
         Setting.boolSetting("index.routing.allocation.disk.watermark.ignore", false,

diff --git a/.../java/org/elasticsearch/cluster/routing/allocation/decider/DiskThresholdDeciderTests.java b/.../java/org/elasticsearch/cluster/routing/allocation/decider/DiskThresholdDeciderTests.java
@@ -1133,6 +1133,17 @@ Settings.EMPTY, new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLU
                 " actual free: [20.0%]"));
     }
 
+    public void testSingleDataNodeDeprecationWarning() {
+        Settings settings = Settings.builder()
+            .put(DiskThresholdDecider.ENABLE_FOR_SINGLE_DATA_NODE.getKey(), false)
+            .build();
+
+        new DiskThresholdDecider(settings, new ClusterSettings(settings, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS));
+
+        assertWarnings("setting [cluster.routing.allocation.disk.watermark.enable_for_single_data_node=false] is deprecated and" +
+            " will not be available in a future version");
+    }
+
     public void testDiskThresholdWithSnapshotShardSizes() {
         final long shardSizeInBytes = randomBoolean() ? 10L : 50L;
         logger.info("--> using shard size [{}]", shardSizeInBytes);

diff --git a/...ugin/deprecation/src/main/java/org/elasticsearch/xpack/deprecation/DeprecationChecks.java b/...ugin/deprecation/src/main/java/org/elasticsearch/xpack/deprecation/DeprecationChecks.java
@@ -10,6 +10,7 @@
 import org.elasticsearch.cluster.ClusterState;
 import org.elasticsearch.cluster.metadata.IndexMetadata;
 import org.elasticsearch.cluster.node.DiscoveryNode;
+import org.elasticsearch.common.TriFunction;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.xpack.core.XPackSettings;
 import org.elasticsearch.xpack.core.deprecation.DeprecationInfoAction;
@@ -19,7 +20,6 @@
 import java.util.Collections;
 import java.util.List;
 import java.util.Objects;
-import java.util.function.BiFunction;
 import java.util.function.Function;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
@@ -42,13 +42,14 @@ private DeprecationChecks() {
             ClusterDeprecationChecks::checkTemplatesWithMultipleTypes
         ));
 
-    static final List<BiFunction<Settings, PluginsAndModules, DeprecationIssue>> NODE_SETTINGS_CHECKS;
+    static final List<TriFunction<Settings, PluginsAndModules, ClusterState, DeprecationIssue>> NODE_SETTINGS_CHECKS;
 
         static {
-            final Stream<BiFunction<Settings, PluginsAndModules, DeprecationIssue>> legacyRoleSettings = DiscoveryNode.getPossibleRoles()
+            final Stream<TriFunction<Settings, PluginsAndModules, ClusterState, DeprecationIssue>> legacyRoleSettings =
+                DiscoveryNode.getPossibleRoles()
                 .stream()
                 .filter(r -> r.legacySetting() != null)
-                .map(r -> (s, p) -> NodeDeprecationChecks.checkLegacyRoleSettings(r.legacySetting(), s, p));
+                .map(r -> (s, p, cs) -> NodeDeprecationChecks.checkLegacyRoleSettings(r.legacySetting(), s, p));
             NODE_SETTINGS_CHECKS = Stream.concat(
                 legacyRoleSettings,
                 Stream.of(
@@ -58,35 +59,36 @@ private DeprecationChecks() {
                     NodeDeprecationChecks::checkMissingRealmOrders,
                     NodeDeprecationChecks::checkUniqueRealmOrders,
                     NodeDeprecationChecks::checkImplicitlyDisabledBasicRealms,
-                    (settings, pluginsAndModules) -> NodeDeprecationChecks.checkThreadPoolListenerQueueSize(settings),
-                    (settings, pluginsAndModules) -> NodeDeprecationChecks.checkThreadPoolListenerSize(settings),
+                    (settings, pluginsAndModules, cs) -> NodeDeprecationChecks.checkThreadPoolListenerQueueSize(settings),
+                    (settings, pluginsAndModules, cs) -> NodeDeprecationChecks.checkThreadPoolListenerSize(settings),
                     NodeDeprecationChecks::checkClusterRemoteConnectSetting,
                     NodeDeprecationChecks::checkNodeLocalStorageSetting,
                     NodeDeprecationChecks::checkGeneralScriptSizeSetting,
                     NodeDeprecationChecks::checkGeneralScriptExpireSetting,
                     NodeDeprecationChecks::checkGeneralScriptCompileSettings,
-                    (settings, pluginsAndModules) -> NodeDeprecationChecks.checkNodeBasicLicenseFeatureEnabledSetting(settings,
+                    (settings, pluginsAndModules, cs) -> NodeDeprecationChecks.checkNodeBasicLicenseFeatureEnabledSetting(settings,
                         XPackSettings.ENRICH_ENABLED_SETTING),
-                    (settings, pluginsAndModules) -> NodeDeprecationChecks.checkNodeBasicLicenseFeatureEnabledSetting(settings,
+                    (settings, pluginsAndModules, cs) -> NodeDeprecationChecks.checkNodeBasicLicenseFeatureEnabledSetting(settings,
                         XPackSettings.FLATTENED_ENABLED),
-                    (settings, pluginsAndModules) -> NodeDeprecationChecks.checkNodeBasicLicenseFeatureEnabledSetting(settings,
+                    (settings, pluginsAndModules, cs) -> NodeDeprecationChecks.checkNodeBasicLicenseFeatureEnabledSetting(settings,
                         XPackSettings.INDEX_LIFECYCLE_ENABLED),
-                    (settings, pluginsAndModules) -> NodeDeprecationChecks.checkNodeBasicLicenseFeatureEnabledSetting(settings,
+                    (settings, pluginsAndModules, cs) -> NodeDeprecationChecks.checkNodeBasicLicenseFeatureEnabledSetting(settings,
                         XPackSettings.MONITORING_ENABLED),
-                    (settings, pluginsAndModules) -> NodeDeprecationChecks.checkNodeBasicLicenseFeatureEnabledSetting(settings,
+                    (settings, pluginsAndModules, cs) -> NodeDeprecationChecks.checkNodeBasicLicenseFeatureEnabledSetting(settings,
                         XPackSettings.ROLLUP_ENABLED),
-                    (settings, pluginsAndModules) -> NodeDeprecationChecks.checkNodeBasicLicenseFeatureEnabledSetting(settings,
+                    (settings, pluginsAndModules, cs) -> NodeDeprecationChecks.checkNodeBasicLicenseFeatureEnabledSetting(settings,
                         XPackSettings.SNAPSHOT_LIFECYCLE_ENABLED),
-                    (settings, pluginsAndModules) -> NodeDeprecationChecks.checkNodeBasicLicenseFeatureEnabledSetting(settings,
+                    (settings, pluginsAndModules, cs) -> NodeDeprecationChecks.checkNodeBasicLicenseFeatureEnabledSetting(settings,
                         XPackSettings.SQL_ENABLED),
-                    (settings, pluginsAndModules) -> NodeDeprecationChecks.checkNodeBasicLicenseFeatureEnabledSetting(settings,
+                    (settings, pluginsAndModules, cs) -> NodeDeprecationChecks.checkNodeBasicLicenseFeatureEnabledSetting(settings,
                         XPackSettings.TRANSFORM_ENABLED),
-                    (settings, pluginsAndModules) -> NodeDeprecationChecks.checkNodeBasicLicenseFeatureEnabledSetting(settings,
+                    (settings, pluginsAndModules, cs) -> NodeDeprecationChecks.checkNodeBasicLicenseFeatureEnabledSetting(settings,
                         XPackSettings.VECTORS_ENABLED),
                     NodeDeprecationChecks::checkMultipleDataPaths,
                     NodeDeprecationChecks::checkDataPathsList,
                     NodeDeprecationChecks::checkBootstrapSystemCallFilterSetting,
-                    NodeDeprecationChecks::checkSharedDataPathSetting
+                    NodeDeprecationChecks::checkSharedDataPathSetting,
+                    NodeDeprecationChecks::checkSingleDataNodeWatermarkSetting
                 )
             ).collect(Collectors.toList());
         }