Remove deprecated `xpack.searchable.snapshot.allocate_on_rolling_rest…

…art` setting (elastic#114202) The setting was created as an escape-hatch in case elastic#66369 had some unintended side-effects. It has been deprecated since elastic#84959 (8.2.0). --------- Co-authored-by: David Turner <[email protected]>
salvatore-campagna · Nov 18, 2024 · de103e7 · de103e7
1 parent 31fac24
commit de103e7
Show file tree

Hide file tree

Showing 4 changed files with 25 additions and 60 deletions.
diff --git a/docs/changelog/114202.yaml b/docs/changelog/114202.yaml
@@ -0,0 +1,14 @@
+pr: 114202
+summary: Remove deprecated `xpack.searchable.snapshot.allocate_on_rolling_restart` setting
+area: Snapshot/Restore
+type: breaking
+issues: []
+breaking:
+  title: Remove deprecated `xpack.searchable.snapshot.allocate_on_rolling_restart` setting
+  area: 'Cluster and node setting'
+  details: >-
+    The `xpack.searchable.snapshot.allocate_on_rolling_restart` setting was created as an escape-hatch just in case
+    relying on the `cluster.routing.allocation.enable=primaries` setting for allocating searchable snapshots during
+    rolling restarts had some unintended side-effects. It has been deprecated since 8.2.0.
+  impact: Remove `xpack.searchable.snapshot.allocate_on_rolling_restart` from your settings if present.
+  notable: false
diff --git a/...k/searchablesnapshots/allocation/SearchableSnapshotEnableAllocationDeciderIntegTests.java b/...k/searchablesnapshots/allocation/SearchableSnapshotEnableAllocationDeciderIntegTests.java
@@ -15,7 +15,6 @@
 import org.elasticsearch.snapshots.SnapshotId;
 import org.elasticsearch.test.ESIntegTestCase;
 import org.elasticsearch.xpack.searchablesnapshots.BaseSearchableSnapshotsIntegTestCase;
-import org.elasticsearch.xpack.searchablesnapshots.allocation.decider.SearchableSnapshotEnableAllocationDecider;
 import org.hamcrest.Matchers;
 
 import java.util.List;
@@ -31,9 +30,7 @@ public void testAllocationDisabled() throws Exception {
         final String restoredIndexName = setupMountedIndex();
         int numPrimaries = getNumShards(restoredIndexName).numPrimaries;
         setEnableAllocation(EnableAllocationDecider.Allocation.PRIMARIES);
-        if (randomBoolean()) {
-            setAllocateOnRollingRestart(false);
-        }
+
         Set<String> indexNodes = internalCluster().nodesInclude(restoredIndexName);
         for (String indexNode : indexNodes) {
             internalCluster().restartNode(indexNode);
@@ -43,16 +40,13 @@ public void testAllocationDisabled() throws Exception {
             .actionGet();
         assertThat(response.getUnassignedShards(), Matchers.equalTo(numPrimaries));
 
-        setAllocateOnRollingRestart(true);
+        setEnableAllocation(null);
         ensureGreen(restoredIndexName);
     }
 
     public void testAllocateOnRollingRestartEnabled() throws Exception {
         final String restoredIndexName = setupMountedIndex();
-        if (randomBoolean()) {
-            setEnableAllocation(EnableAllocationDecider.Allocation.PRIMARIES);
-        }
-        setAllocateOnRollingRestart(true);
+        setEnableAllocation(null);
         Set<String> indexNodes = internalCluster().nodesInclude(restoredIndexName);
         for (String indexNode : indexNodes) {
             internalCluster().restartNode(indexNode);
@@ -74,14 +68,7 @@ private String setupMountedIndex() throws Exception {
     }
 
     public void setEnableAllocation(EnableAllocationDecider.Allocation allocation) {
-        setSetting(EnableAllocationDecider.CLUSTER_ROUTING_ALLOCATION_ENABLE_SETTING, allocation.name());
-    }
-
-    public void setAllocateOnRollingRestart(boolean allocateOnRollingRestart) {
-        setSetting(
-            SearchableSnapshotEnableAllocationDecider.SEARCHABLE_SNAPSHOTS_ALLOCATE_ON_ROLLING_RESTART,
-            Boolean.toString(allocateOnRollingRestart)
-        );
+        setSetting(EnableAllocationDecider.CLUSTER_ROUTING_ALLOCATION_ENABLE_SETTING, allocation != null ? allocation.name() : null);
     }
 
     private void setSetting(Setting<?> setting, String value) {

diff --git a/...pshots/src/main/java/org/elasticsearch/xpack/searchablesnapshots/SearchableSnapshots.java b/...pshots/src/main/java/org/elasticsearch/xpack/searchablesnapshots/SearchableSnapshots.java
@@ -304,7 +304,6 @@ public List<Setting<?>> getSettings() {
             CacheService.SNAPSHOT_CACHE_SYNC_INTERVAL_SETTING,
             CacheService.SNAPSHOT_CACHE_MAX_FILES_TO_SYNC_AT_ONCE_SETTING,
             CacheService.SNAPSHOT_CACHE_SYNC_SHUTDOWN_TIMEOUT,
-            SearchableSnapshotEnableAllocationDecider.SEARCHABLE_SNAPSHOTS_ALLOCATE_ON_ROLLING_RESTART,
             BlobStoreCacheMaintenanceService.SNAPSHOT_SNAPSHOT_CLEANUP_INTERVAL_SETTING,
             BlobStoreCacheMaintenanceService.SNAPSHOT_SNAPSHOT_CLEANUP_KEEP_ALIVE_SETTING,
             BlobStoreCacheMaintenanceService.SNAPSHOT_SNAPSHOT_CLEANUP_BATCH_SIZE_SETTING,

diff --git a/...ack/searchablesnapshots/allocation/decider/SearchableSnapshotEnableAllocationDecider.java b/...ack/searchablesnapshots/allocation/decider/SearchableSnapshotEnableAllocationDecider.java
@@ -15,50 +15,26 @@
 import org.elasticsearch.cluster.routing.allocation.decider.Decision;
 import org.elasticsearch.cluster.routing.allocation.decider.EnableAllocationDecider;
 import org.elasticsearch.common.settings.ClusterSettings;
-import org.elasticsearch.common.settings.Setting;
 import org.elasticsearch.common.settings.Settings;
-import org.elasticsearch.core.UpdateForV9;
 
 public class SearchableSnapshotEnableAllocationDecider extends AllocationDecider {
 
     static final String NAME = "searchable_snapshots_enable";
 
-    /**
-     * This setting describes whether searchable snapshots are allocated during rolling restarts. For now, whether a rolling restart is
-     * ongoing is determined by cluster.routing.allocation.enable=primaries. Notice that other values for that setting except "all" mean
-     * that no searchable snapshots are allocated anyway.
-     */
-    @UpdateForV9(owner = UpdateForV9.Owner.DISTRIBUTED_COORDINATION)
-    // xpack.searchable.snapshot.allocate_on_rolling_restart was only temporary, remove it in the next major
-    public static final Setting<Boolean> SEARCHABLE_SNAPSHOTS_ALLOCATE_ON_ROLLING_RESTART = Setting.boolSetting(
-        "xpack.searchable.snapshot.allocate_on_rolling_restart",
-        false,
-        Setting.Property.Dynamic,
-        Setting.Property.NodeScope,
-        Setting.Property.Deprecated
-    );
-
     private volatile EnableAllocationDecider.Allocation enableAllocation;
-    private volatile boolean allocateOnRollingRestart;
 
     public SearchableSnapshotEnableAllocationDecider(Settings settings, ClusterSettings clusterSettings) {
         this.enableAllocation = EnableAllocationDecider.CLUSTER_ROUTING_ALLOCATION_ENABLE_SETTING.get(settings);
-        this.allocateOnRollingRestart = SEARCHABLE_SNAPSHOTS_ALLOCATE_ON_ROLLING_RESTART.get(settings);
         clusterSettings.addSettingsUpdateConsumer(
             EnableAllocationDecider.CLUSTER_ROUTING_ALLOCATION_ENABLE_SETTING,
             this::setEnableAllocation
         );
-        clusterSettings.addSettingsUpdateConsumer(SEARCHABLE_SNAPSHOTS_ALLOCATE_ON_ROLLING_RESTART, this::setAllocateOnRollingRestart);
     }
 
     private void setEnableAllocation(EnableAllocationDecider.Allocation allocation) {
         this.enableAllocation = allocation;
     }
 
-    private void setAllocateOnRollingRestart(boolean allocateOnRollingRestart) {
-        this.allocateOnRollingRestart = allocateOnRollingRestart;
-    }
-
     @Override
     public Decision canAllocate(ShardRouting shardRouting, RoutingNode node, RoutingAllocation allocation) {
         return canAllocate(shardRouting, allocation);
@@ -73,25 +49,14 @@ public Decision canAllocate(ShardRouting shardRouting, RoutingAllocation allocat
         final IndexMetadata indexMetadata = allocation.metadata().getIndexSafe(shardRouting.index());
         if (indexMetadata.isSearchableSnapshot()) {
             EnableAllocationDecider.Allocation enableAllocationCopy = this.enableAllocation;
-            boolean allocateOnRollingRestartCopy = this.allocateOnRollingRestart;
             if (enableAllocationCopy == EnableAllocationDecider.Allocation.PRIMARIES) {
-                if (allocateOnRollingRestartCopy == false) {
-                    return allocation.decision(
-                        Decision.NO,
-                        NAME,
-                        "no allocations of searchable snapshots allowed during rolling restart due to [%s=%s] and [%s=false]",
-                        EnableAllocationDecider.CLUSTER_ROUTING_ALLOCATION_ENABLE_SETTING.getKey(),
-                        enableAllocationCopy,
-                        SEARCHABLE_SNAPSHOTS_ALLOCATE_ON_ROLLING_RESTART.getKey()
-                    );
-                } else {
-                    return allocation.decision(
-                        Decision.YES,
-                        NAME,
-                        "allocate on rolling restart enabled [%s=true]",
-                        SEARCHABLE_SNAPSHOTS_ALLOCATE_ON_ROLLING_RESTART.getKey()
-                    );
-                }
+                return allocation.decision(
+                    Decision.NO,
+                    NAME,
+                    "no allocations of searchable snapshots allowed during rolling restart due to [%s=%s]",
+                    EnableAllocationDecider.CLUSTER_ROUTING_ALLOCATION_ENABLE_SETTING.getKey(),
+                    enableAllocationCopy
+                );
             } else {
                 return allocation.decision(
                     Decision.YES,