Create version-specific helper for poison pills (#101767)

* Add static assertRemoveBeforeV9() * Switch from assertion to annotation + some usage examples * Fixup ReplicationTracker * Update name, use annotation on fields --------- Co-authored-by: David Turner <david.turner@elastic.co>
elastic · Nov 15, 2023 · 7123898 · 7123898
1 parent 184d993
commit 7123898
Show file tree

Hide file tree

Showing 10 changed files with 80 additions and 44 deletions.
diff --git a/libs/core/src/main/java/org/elasticsearch/core/UpdateForV9.java b/libs/core/src/main/java/org/elasticsearch/core/UpdateForV9.java
@@ -0,0 +1,23 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License
+ * 2.0 and the Server Side Public License, v 1; you may not use this file except
+ * in compliance with, at your election, the Elastic License 2.0 or the Server
+ * Side Public License, v 1.
+ */
+
+package org.elasticsearch.core;
+
+import java.lang.annotation.ElementType;
+import java.lang.annotation.Retention;
+import java.lang.annotation.RetentionPolicy;
+import java.lang.annotation.Target;
+
+/**
+ * Annotation to identify a block of code (a whole class, a method, or a field) that needs to be reviewed (for cleanup, remove or change)
+ * before releasing 9.0
+ */
+@Retention(RetentionPolicy.SOURCE)
+@Target({ ElementType.LOCAL_VARIABLE, ElementType.CONSTRUCTOR, ElementType.FIELD, ElementType.METHOD, ElementType.TYPE })
+public @interface UpdateForV9 {
+}
diff --git a/server/src/main/java/org/elasticsearch/action/bulk/BulkRequestParser.java b/server/src/main/java/org/elasticsearch/action/bulk/BulkRequestParser.java
@@ -8,7 +8,6 @@
 
 package org.elasticsearch.action.bulk;
 
-import org.elasticsearch.Version;
 import org.elasticsearch.action.DocWriteRequest;
 import org.elasticsearch.action.delete.DeleteRequest;
 import org.elasticsearch.action.index.IndexRequest;
@@ -19,6 +18,7 @@
 import org.elasticsearch.common.xcontent.LoggingDeprecationHandler;
 import org.elasticsearch.core.Nullable;
 import org.elasticsearch.core.RestApiVersion;
+import org.elasticsearch.core.UpdateForV9;
 import org.elasticsearch.index.VersionType;
 import org.elasticsearch.index.seqno.SequenceNumbers;
 import org.elasticsearch.rest.action.document.RestBulkAction;
@@ -430,32 +430,32 @@ public void parse(
         }
     }
 
+    @UpdateForV9
+    // Warnings will need to be replaced with XContentEOFException from 9.x
+    private static void warnBulkActionNotProperlyClosed(String message) {
+        deprecationLogger.compatibleCritical(STRICT_ACTION_PARSING_WARNING_KEY, message);
+    }
+
     private static void checkBulkActionIsProperlyClosed(XContentParser parser) throws IOException {
         XContentParser.Token token;
         try {
             token = parser.nextToken();
         } catch (XContentEOFException ignore) {
-            assert Version.CURRENT.major == Version.V_7_17_0.major + 1;
-            deprecationLogger.compatibleCritical(
-                STRICT_ACTION_PARSING_WARNING_KEY,
+            warnBulkActionNotProperlyClosed(
                 "A bulk action wasn't closed properly with the closing brace. Malformed objects are currently accepted but will be "
                     + "rejected in a future version."
             );
             return;
         }
         if (token != XContentParser.Token.END_OBJECT) {
-            assert Version.CURRENT.major == Version.V_7_17_0.major + 1;
-            deprecationLogger.compatibleCritical(
-                STRICT_ACTION_PARSING_WARNING_KEY,
+            warnBulkActionNotProperlyClosed(
                 "A bulk action object contained multiple keys. Additional keys are currently ignored but will be rejected in a "
                     + "future version."
             );
             return;
         }
         if (parser.nextToken() != null) {
-            assert Version.CURRENT.major == Version.V_7_17_0.major + 1;
-            deprecationLogger.compatibleCritical(
-                STRICT_ACTION_PARSING_WARNING_KEY,
+            warnBulkActionNotProperlyClosed(
                 "A bulk action contained trailing data after the closing brace. This is currently ignored but will be rejected in a "
                     + "future version."
             );

diff --git a/server/src/main/java/org/elasticsearch/cluster/ClusterModule.java b/server/src/main/java/org/elasticsearch/cluster/ClusterModule.java
@@ -8,7 +8,6 @@
 
 package org.elasticsearch.cluster;
 
-import org.elasticsearch.Version;
 import org.elasticsearch.cluster.action.index.MappingUpdatedAction;
 import org.elasticsearch.cluster.action.shard.ShardStateAction;
 import org.elasticsearch.cluster.metadata.ComponentTemplateMetadata;
@@ -65,6 +64,7 @@
 import org.elasticsearch.common.settings.Setting;
 import org.elasticsearch.common.settings.Setting.Property;
 import org.elasticsearch.common.settings.Settings;
+import org.elasticsearch.core.UpdateForV9;
 import org.elasticsearch.gateway.GatewayAllocator;
 import org.elasticsearch.health.metadata.HealthMetadataService;
 import org.elasticsearch.health.node.selection.HealthNodeTaskExecutor;
@@ -373,6 +373,7 @@ private static void addAllocationDecider(Map<Class<?>, AllocationDecider> decide
         }
     }
 
+    @UpdateForV9 // in v9 there is only one allocator
     private static ShardsAllocator createShardsAllocator(
         Settings settings,
         ClusterSettings clusterSettings,
@@ -404,7 +405,6 @@ private static ShardsAllocator createShardsAllocator(
             });
         }
         String allocatorName = SHARDS_ALLOCATOR_TYPE_SETTING.get(settings);
-        assert Version.CURRENT.major == Version.V_7_17_0.major + 1; // in v9 there is only one allocator
         Supplier<ShardsAllocator> allocatorSupplier = allocators.get(allocatorName);
         if (allocatorSupplier == null) {
             throw new IllegalArgumentException("Unknown ShardsAllocator [" + allocatorName + "]");

diff --git a/server/src/main/java/org/elasticsearch/cluster/routing/allocation/DiskThresholdSettings.java b/server/src/main/java/org/elasticsearch/cluster/routing/allocation/DiskThresholdSettings.java
@@ -8,14 +8,14 @@
 
 package org.elasticsearch.cluster.routing.allocation;
 
-import org.elasticsearch.Version;
 import org.elasticsearch.common.settings.ClusterSettings;
 import org.elasticsearch.common.settings.Setting;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.unit.ByteSizeValue;
 import org.elasticsearch.common.unit.RelativeByteSizeValue;
 import org.elasticsearch.core.Strings;
 import org.elasticsearch.core.TimeValue;
+import org.elasticsearch.core.UpdateForV9;
 
 import java.util.Iterator;
 import java.util.List;
@@ -152,7 +152,11 @@ public class DiskThresholdSettings {
     private volatile TimeValue rerouteInterval;
 
     static {
-        assert Version.CURRENT.major == Version.V_7_0_0.major + 1; // this check is unnecessary in v9
+        checkAutoReleaseIndexEnabled();
+    }
+
+    @UpdateForV9 // this check is unnecessary in v9
+    private static void checkAutoReleaseIndexEnabled() {
         final String AUTO_RELEASE_INDEX_ENABLED_KEY = "es.disk.auto_release_flood_stage_block";
         final String property = System.getProperty(AUTO_RELEASE_INDEX_ENABLED_KEY);
         if (property != null) {

diff --git a/.../main/java/org/elasticsearch/cluster/routing/allocation/decider/DiskThresholdDecider.java b/.../main/java/org/elasticsearch/cluster/routing/allocation/decider/DiskThresholdDecider.java
@@ -10,7 +10,6 @@
 
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
-import org.elasticsearch.Version;
 import org.elasticsearch.cluster.ClusterInfo;
 import org.elasticsearch.cluster.DiskUsage;
 import org.elasticsearch.cluster.metadata.IndexMetadata;
@@ -27,6 +26,7 @@
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.settings.SettingsException;
 import org.elasticsearch.common.unit.ByteSizeValue;
+import org.elasticsearch.core.UpdateForV9;
 
 import java.util.Map;
 
@@ -69,6 +69,7 @@ public class DiskThresholdDecider extends AllocationDecider {
 
     public static final String NAME = "disk_threshold";
 
+    @UpdateForV9
     public static final Setting<Boolean> ENABLE_FOR_SINGLE_DATA_NODE = Setting.boolSetting(
         "cluster.routing.allocation.disk.watermark.enable_for_single_data_node",
         true,
@@ -98,7 +99,6 @@ public void validate(Boolean value) {
 
     public DiskThresholdDecider(Settings settings, ClusterSettings clusterSettings) {
         this.diskThresholdSettings = new DiskThresholdSettings(settings, clusterSettings);
-        assert Version.CURRENT.major < 9 : "remove enable_for_single_data_node in 9";
         // get deprecation warnings.
         boolean enabledForSingleDataNode = ENABLE_FOR_SINGLE_DATA_NODE.get(settings);
         assert enabledForSingleDataNode;

diff --git a/server/src/main/java/org/elasticsearch/discovery/DiscoveryModule.java b/server/src/main/java/org/elasticsearch/discovery/DiscoveryModule.java
@@ -10,7 +10,6 @@
 
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
-import org.elasticsearch.Version;
 import org.elasticsearch.client.internal.Client;
 import org.elasticsearch.cluster.ClusterState;
 import org.elasticsearch.cluster.coordination.Coordinator;
@@ -35,6 +34,7 @@
 import org.elasticsearch.common.settings.Setting.Property;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.transport.TransportAddress;
+import org.elasticsearch.core.UpdateForV9;
 import org.elasticsearch.features.FeatureService;
 import org.elasticsearch.gateway.GatewayMetaState;
 import org.elasticsearch.indices.breaker.CircuitBreakerService;
@@ -173,19 +173,7 @@ public DiscoveryModule(
             throw new IllegalArgumentException("Unknown election strategy " + ELECTION_STRATEGY_SETTING.get(settings));
         }
 
-        if (LEGACY_MULTI_NODE_DISCOVERY_TYPE.equals(discoveryType)) {
-            assert Version.CURRENT.major == Version.V_7_0_0.major + 1;
-            DeprecationLogger.getLogger(DiscoveryModule.class)
-                .critical(
-                    DeprecationCategory.SETTINGS,
-                    "legacy-discovery-type",
-                    "Support for setting [{}] to [{}] is deprecated and will be removed in a future version. Set this setting to [{}] "
-                        + "instead.",
-                    DISCOVERY_TYPE_SETTING.getKey(),
-                    LEGACY_MULTI_NODE_DISCOVERY_TYPE,
-                    MULTI_NODE_DISCOVERY_TYPE
-                );
-        }
+        checkLegacyMultiNodeDiscoveryType(discoveryType);
 
         this.reconfigurator = getReconfigurator(settings, clusterSettings, clusterCoordinationPlugins);
         var preVoteCollectorFactory = getPreVoteCollectorFactory(clusterCoordinationPlugins);
@@ -225,6 +213,22 @@ public DiscoveryModule(
         logger.info("using discovery type [{}] and seed hosts providers {}", discoveryType, seedProviderNames);
     }
 
+    @UpdateForV9
+    private static void checkLegacyMultiNodeDiscoveryType(String discoveryType) {
+        if (LEGACY_MULTI_NODE_DISCOVERY_TYPE.equals(discoveryType)) {
+            DeprecationLogger.getLogger(DiscoveryModule.class)
+                .critical(
+                    DeprecationCategory.SETTINGS,
+                    "legacy-discovery-type",
+                    "Support for setting [{}] to [{}] is deprecated and will be removed in a future version. Set this setting to [{}] "
+                        + "instead.",
+                    DISCOVERY_TYPE_SETTING.getKey(),
+                    LEGACY_MULTI_NODE_DISCOVERY_TYPE,
+                    MULTI_NODE_DISCOVERY_TYPE
+                );
+        }
+    }
+
     // visible for testing
     static Reconfigurator getReconfigurator(
         Settings settings,

diff --git a/server/src/main/java/org/elasticsearch/env/NodeMetadata.java b/server/src/main/java/org/elasticsearch/env/NodeMetadata.java
@@ -10,6 +10,7 @@
 
 import org.elasticsearch.Build;
 import org.elasticsearch.Version;
+import org.elasticsearch.core.UpdateForV9;
 import org.elasticsearch.gateway.MetadataStateFormat;
 import org.elasticsearch.index.IndexVersion;
 import org.elasticsearch.index.IndexVersions;
@@ -162,15 +163,15 @@ public void setOldestIndexVersion(int oldestIndexVersion) {
             this.oldestIndexVersion = IndexVersion.fromId(oldestIndexVersion);
         }
 
+        private Version getVersionOrFallbackToEmpty() {
+            return Objects.requireNonNullElse(this.nodeVersion, Version.V_EMPTY);
+        }
+
         public NodeMetadata build() {
-            final Version nodeVersion;
+            @UpdateForV9 // version is required in the node metadata from v9 onwards
+            final Version nodeVersion = getVersionOrFallbackToEmpty();
             final IndexVersion oldestIndexVersion;
-            if (this.nodeVersion == null) {
-                assert Version.CURRENT.major <= Version.V_7_0_0.major + 1 : "version is required in the node metadata from v9 onwards";
-                nodeVersion = Version.V_EMPTY;
-            } else {
-                nodeVersion = this.nodeVersion;
-            }
+
             if (this.previousNodeVersion == null) {
                 previousNodeVersion = nodeVersion;
             }

diff --git a/server/src/main/java/org/elasticsearch/gateway/GatewayMetaState.java b/server/src/main/java/org/elasticsearch/gateway/GatewayMetaState.java
@@ -34,6 +34,7 @@
 import org.elasticsearch.common.util.concurrent.EsThreadPoolExecutor;
 import org.elasticsearch.core.IOUtils;
 import org.elasticsearch.core.Tuple;
+import org.elasticsearch.core.UpdateForV9;
 import org.elasticsearch.env.NodeMetadata;
 import org.elasticsearch.index.IndexVersions;
 import org.elasticsearch.node.Node;
@@ -184,7 +185,7 @@ private PersistedState createOnDiskPersistedState(
         long currentTerm = onDiskState.currentTerm;
 
         if (onDiskState.empty()) {
-            assert Version.CURRENT.major <= Version.V_7_0_0.major + 1 : "legacy metadata loader is not needed anymore from v9 onwards";
+            @UpdateForV9 // legacy metadata loader is not needed anymore from v9 onwards
             final Tuple<Manifest, Metadata> legacyState = metaStateService.loadFullState();
             if (legacyState.v1().isEmpty() == false) {
                 metadata = legacyState.v2();

diff --git a/server/src/main/java/org/elasticsearch/gateway/MetaStateService.java b/server/src/main/java/org/elasticsearch/gateway/MetaStateService.java
@@ -17,6 +17,7 @@
 import org.elasticsearch.cluster.metadata.Metadata;
 import org.elasticsearch.core.Nullable;
 import org.elasticsearch.core.Tuple;
+import org.elasticsearch.core.UpdateForV9;
 import org.elasticsearch.env.NodeEnvironment;
 import org.elasticsearch.index.Index;
 import org.elasticsearch.xcontent.NamedXContentRegistry;
@@ -54,6 +55,7 @@ public MetaStateService(NodeEnvironment nodeEnv, NamedXContentRegistry namedXCon
      * meta state with globalGeneration -1 and empty meta data is returned.
      * @throws IOException if some IOException when loading files occurs or there is no metadata referenced by manifest file.
      */
+    @UpdateForV9
     public Tuple<Manifest, Metadata> loadFullState() throws IOException {
         final Manifest manifest = Manifest.FORMAT.loadLatestState(logger, namedXContentRegistry, nodeEnv.nodeDataPaths());
         if (manifest == null) {

diff --git a/server/src/main/java/org/elasticsearch/index/seqno/ReplicationTracker.java b/server/src/main/java/org/elasticsearch/index/seqno/ReplicationTracker.java
@@ -21,6 +21,7 @@
 import org.elasticsearch.common.io.stream.Writeable;
 import org.elasticsearch.common.util.Maps;
 import org.elasticsearch.core.SuppressForbidden;
+import org.elasticsearch.core.UpdateForV9;
 import org.elasticsearch.gateway.WriteStateException;
 import org.elasticsearch.index.IndexSettings;
 import org.elasticsearch.index.IndexVersions;
@@ -466,13 +467,13 @@ public RetentionLeases loadRetentionLeases(final Path path) throws IOException {
         synchronized (retentionLeasePersistenceLock) {
             retentionLeases = RetentionLeases.FORMAT.loadLatestState(logger, NamedXContentRegistry.EMPTY, path);
         }
+        return emptyIfNull(retentionLeases);
+    }
 
-        // TODO after backporting we expect this never to happen in 8.x, so adjust this to throw an exception instead.
-        assert Version.CURRENT.major <= 8 : "throw an exception instead of returning EMPTY on null";
-        if (retentionLeases == null) {
-            return RetentionLeases.EMPTY;
-        }
-        return retentionLeases;
+    @UpdateForV9
+    private static RetentionLeases emptyIfNull(RetentionLeases retentionLeases) {
+        // we expect never to see a null in 8.x, so adjust this to throw an exception from v9 onwards.
+        return retentionLeases == null ? RetentionLeases.EMPTY : retentionLeases;
     }
 
     private final Object retentionLeasePersistenceLock = new Object();