Remove deprecated settings to defer cluster recovery (#17357)

andrross · web-flow · commit 2d2b41d34544 · 2025-02-14T11:02:37.000-06:00
The following settings were deprecated in ES 7.7 prior to the fork:

- gateway.expected_nodes
- gateway.expected_master_nodes
- gateway.recover_after_nodes
- gateway.recover_after_master_nodes

This commit removes the deprecated settings and replaces their usages
in tests with `recover_after_data_nodes`.

Signed-off-by: Andrew Ross &lt;andrross@amazon.com&gt;
diff --git a/CHANGELOG-3.0.md b/CHANGELOG-3.0.md
@@ -58,6 +58,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 - Remove `index.store.hybrid.mmap.extensions` setting in favor of `index.store.hybrid.nio.extensions` setting ([#9392](https://github.com/opensearch-project/OpenSearch/pull/9392))
 - Remove package org.opensearch.action.support.master ([#4856](https://github.com/opensearch-project/OpenSearch/issues/4856))
 - Remove transport-nio plugin ([#16887](https://github.com/opensearch-project/OpenSearch/issues/16887))
+- Remove deprecated 'gateway' settings used to defer cluster recovery ([#3117](https://github.com/opensearch-project/OpenSearch/issues/3117))
 
 ### Fixed
 - Fix 'org.apache.hc.core5.http.ParseException: Invalid protocol version' under JDK 16+ ([#4827](https://github.com/opensearch-project/OpenSearch/pull/4827))
diff --git a/distribution/src/config/opensearch.yml b/distribution/src/config/opensearch.yml
@@ -77,7 +77,7 @@ ${path.logs}
 #
 # Block initial recovery after a full cluster restart until N nodes are started:
 #
-#gateway.recover_after_nodes: 3
+#gateway.recover_after_data_nodes: 3
 #
 # For more information, consult the gateway module documentation.
 #
diff --git a/server/src/internalClusterTest/java/org/opensearch/action/admin/cluster/stats/ClusterStatsIT.java b/server/src/internalClusterTest/java/org/opensearch/action/admin/cluster/stats/ClusterStatsIT.java
@@ -46,6 +46,7 @@
 import org.opensearch.common.settings.Settings;
 import org.opensearch.common.util.concurrent.OpenSearchExecutors;
 import org.opensearch.core.common.unit.ByteSizeValue;
+import org.opensearch.gateway.GatewayService;
 import org.opensearch.monitor.os.OsStats;
 import org.opensearch.node.NodeRoleSettings;
 import org.opensearch.test.OpenSearchIntegTestCase;
@@ -383,19 +384,18 @@ public void testAllocatedProcessors() throws Exception {
     }
 
     public void testClusterStatusWhenStateNotRecovered() throws Exception {
-        internalCluster().startClusterManagerOnlyNode(Settings.builder().put("gateway.recover_after_nodes", 2).build());
+        internalCluster().startClusterManagerOnlyNode(
+            Settings.builder().put(GatewayService.RECOVER_AFTER_DATA_NODES_SETTING.getKey(), 2).build()
+        );
         ClusterStatsResponse response = client().admin()
             .cluster()
             .prepareClusterStats()
             .useAggregatedNodeLevelResponses(randomBoolean())
             .get();
         assertThat(response.getStatus(), equalTo(ClusterHealthStatus.RED));
 
-        if (randomBoolean()) {
-            internalCluster().startClusterManagerOnlyNode();
-        } else {
-            internalCluster().startDataOnlyNode();
-        }
+        internalCluster().startDataOnlyNodes(2);
+
         // wait for the cluster status to settle
         ensureGreen();
         response = client().admin().cluster().prepareClusterStats().useAggregatedNodeLevelResponses(randomBoolean()).get();
diff --git a/server/src/internalClusterTest/java/org/opensearch/action/admin/cluster/tasks/PendingTasksBlocksIT.java b/server/src/internalClusterTest/java/org/opensearch/action/admin/cluster/tasks/PendingTasksBlocksIT.java
@@ -95,7 +95,7 @@ public void testPendingTasksWithClusterNotRecoveredBlock() throws Exception {
         internalCluster().fullRestart(new InternalTestCluster.RestartCallback() {
             @Override
             public Settings onNodeStopped(String nodeName) {
-                return Settings.builder().put(GatewayService.RECOVER_AFTER_NODES_SETTING.getKey(), nodeCount + 1).build();
+                return Settings.builder().put(GatewayService.RECOVER_AFTER_DATA_NODES_SETTING.getKey(), nodeCount + 1).build();
             }
 
             @Override
diff --git a/server/src/internalClusterTest/java/org/opensearch/action/admin/indices/exists/IndicesExistsIT.java b/server/src/internalClusterTest/java/org/opensearch/action/admin/indices/exists/IndicesExistsIT.java
@@ -49,7 +49,7 @@ public class IndicesExistsIT extends OpenSearchIntegTestCase {
 
     public void testIndexExistsWithBlocksInPlace() throws IOException {
         internalCluster().setBootstrapClusterManagerNodeIndex(0);
-        Settings settings = Settings.builder().put(GatewayService.RECOVER_AFTER_NODES_SETTING.getKey(), 99).build();
+        Settings settings = Settings.builder().put(GatewayService.RECOVER_AFTER_DATA_NODES_SETTING.getKey(), 99).build();
         String node = internalCluster().startNode(settings);
 
         assertRequestBuilderThrows(
diff --git a/server/src/internalClusterTest/java/org/opensearch/gateway/RecoverAfterNodesIT.java b/server/src/internalClusterTest/java/org/opensearch/gateway/RecoverAfterNodesIT.java
@@ -77,7 +77,7 @@ public Client startNode(Settings.Builder settings) {
     public void testRecoverAfterNodes() throws Exception {
         internalCluster().setBootstrapClusterManagerNodeIndex(0);
         logger.info("--> start node (1)");
-        Client clientNode1 = startNode(Settings.builder().put("gateway.recover_after_nodes", 3));
+        Client clientNode1 = startNode(Settings.builder().put(GatewayService.RECOVER_AFTER_DATA_NODES_SETTING.getKey(), 3));
         assertThat(
             clientNode1.admin()
                 .cluster()
@@ -92,7 +92,7 @@ public void testRecoverAfterNodes() throws Exception {
         );
 
         logger.info("--> start node (2)");
-        Client clientNode2 = startNode(Settings.builder().put("gateway.recover_after_nodes", 3));
+        Client clientNode2 = startNode(Settings.builder().put(GatewayService.RECOVER_AFTER_DATA_NODES_SETTING.getKey(), 3));
         Thread.sleep(BLOCK_WAIT_TIMEOUT.millis());
         assertThat(
             clientNode1.admin()
@@ -120,104 +120,13 @@ public void testRecoverAfterNodes() throws Exception {
         );
 
         logger.info("--> start node (3)");
-        Client clientNode3 = startNode(Settings.builder().put("gateway.recover_after_nodes", 3));
+        Client clientNode3 = startNode(Settings.builder().put(GatewayService.RECOVER_AFTER_DATA_NODES_SETTING.getKey(), 3));
 
         assertThat(waitForNoBlocksOnNode(BLOCK_WAIT_TIMEOUT, clientNode1).isEmpty(), equalTo(true));
         assertThat(waitForNoBlocksOnNode(BLOCK_WAIT_TIMEOUT, clientNode2).isEmpty(), equalTo(true));
         assertThat(waitForNoBlocksOnNode(BLOCK_WAIT_TIMEOUT, clientNode3).isEmpty(), equalTo(true));
     }
 
-    public void testRecoverAfterClusterManagerNodes() throws Exception {
-        internalCluster().setBootstrapClusterManagerNodeIndex(0);
-        logger.info("--> start cluster_manager_node (1)");
-        Client clusterManager1 = startNode(Settings.builder().put("gateway.recover_after_master_nodes", 2).put(clusterManagerOnlyNode()));
-        assertThat(
-            clusterManager1.admin()
-                .cluster()
-                .prepareState()
-                .setLocal(true)
-                .execute()
-                .actionGet()
-                .getState()
-                .blocks()
-                .global(ClusterBlockLevel.METADATA_WRITE),
-            hasItem(GatewayService.STATE_NOT_RECOVERED_BLOCK)
-        );
-
-        logger.info("--> start data_node (1)");
-        Client data1 = startNode(Settings.builder().put("gateway.recover_after_master_nodes", 2).put(dataOnlyNode()));
-        assertThat(
-            clusterManager1.admin()
-                .cluster()
-                .prepareState()
-                .setLocal(true)
-                .execute()
-                .actionGet()
-                .getState()
-                .blocks()
-                .global(ClusterBlockLevel.METADATA_WRITE),
-            hasItem(GatewayService.STATE_NOT_RECOVERED_BLOCK)
-        );
-        assertThat(
-            data1.admin()
-                .cluster()
-                .prepareState()
-                .setLocal(true)
-                .execute()
-                .actionGet()
-                .getState()
-                .blocks()
-                .global(ClusterBlockLevel.METADATA_WRITE),
-            hasItem(GatewayService.STATE_NOT_RECOVERED_BLOCK)
-        );
-
-        logger.info("--> start data_node (2)");
-        Client data2 = startNode(Settings.builder().put("gateway.recover_after_master_nodes", 2).put(dataOnlyNode()));
-        assertThat(
-            clusterManager1.admin()
-                .cluster()
-                .prepareState()
-                .setLocal(true)
-                .execute()
-                .actionGet()
-                .getState()
-                .blocks()
-                .global(ClusterBlockLevel.METADATA_WRITE),
-            hasItem(GatewayService.STATE_NOT_RECOVERED_BLOCK)
-        );
-        assertThat(
-            data1.admin()
-                .cluster()
-                .prepareState()
-                .setLocal(true)
-                .execute()
-                .actionGet()
-                .getState()
-                .blocks()
-                .global(ClusterBlockLevel.METADATA_WRITE),
-            hasItem(GatewayService.STATE_NOT_RECOVERED_BLOCK)
-        );
-        assertThat(
-            data2.admin()
-                .cluster()
-                .prepareState()
-                .setLocal(true)
-                .execute()
-                .actionGet()
-                .getState()
-                .blocks()
-                .global(ClusterBlockLevel.METADATA_WRITE),
-            hasItem(GatewayService.STATE_NOT_RECOVERED_BLOCK)
-        );
-
-        logger.info("--> start cluster_manager_node (2)");
-        Client clusterManager2 = startNode(Settings.builder().put("gateway.recover_after_master_nodes", 2).put(clusterManagerOnlyNode()));
-        assertThat(waitForNoBlocksOnNode(BLOCK_WAIT_TIMEOUT, clusterManager1).isEmpty(), equalTo(true));
-        assertThat(waitForNoBlocksOnNode(BLOCK_WAIT_TIMEOUT, clusterManager2).isEmpty(), equalTo(true));
-        assertThat(waitForNoBlocksOnNode(BLOCK_WAIT_TIMEOUT, data1).isEmpty(), equalTo(true));
-        assertThat(waitForNoBlocksOnNode(BLOCK_WAIT_TIMEOUT, data2).isEmpty(), equalTo(true));
-    }
-
     public void testRecoverAfterDataNodes() throws Exception {
         internalCluster().setBootstrapClusterManagerNodeIndex(0);
         logger.info("--> start cluster_manager_node (1)");
diff --git a/server/src/internalClusterTest/java/org/opensearch/gateway/RecoveryFromGatewayIT.java b/server/src/internalClusterTest/java/org/opensearch/gateway/RecoveryFromGatewayIT.java
@@ -119,7 +119,7 @@
 import static org.opensearch.gateway.GatewayRecoveryTestUtils.corruptShard;
 import static org.opensearch.gateway.GatewayRecoveryTestUtils.getDiscoveryNodes;
 import static org.opensearch.gateway.GatewayRecoveryTestUtils.prepareRequestMap;
-import static org.opensearch.gateway.GatewayService.RECOVER_AFTER_NODES_SETTING;
+import static org.opensearch.gateway.GatewayService.RECOVER_AFTER_DATA_NODES_SETTING;
 import static org.opensearch.index.query.QueryBuilders.matchAllQuery;
 import static org.opensearch.index.query.QueryBuilders.termQuery;
 import static org.opensearch.test.hamcrest.OpenSearchAssertions.assertAcked;
@@ -411,7 +411,7 @@ public void testTwoNodeFirstNodeCleared() throws Exception {
             @Override
             public Settings onNodeStopped(String nodeName) {
                 return Settings.builder()
-                    .put(RECOVER_AFTER_NODES_SETTING.getKey(), 2)
+                    .put(RECOVER_AFTER_DATA_NODES_SETTING.getKey(), 2)
                     .putList(INITIAL_CLUSTER_MANAGER_NODES_SETTING.getKey()) // disable bootstrapping
                     .build();
             }
@@ -436,7 +436,7 @@ public boolean clearData(String nodeName) {
 
     public void testLatestVersionLoaded() throws Exception {
         // clean two nodes
-        List<String> nodes = internalCluster().startNodes(2, Settings.builder().put("gateway.recover_after_nodes", 2).build());
+        List<String> nodes = internalCluster().startNodes(2, Settings.builder().put(RECOVER_AFTER_DATA_NODES_SETTING.getKey(), 2).build());
         Settings node1DataPathSettings = internalCluster().dataPathSettings(nodes.get(0));
         Settings node2DataPathSettings = internalCluster().dataPathSettings(nodes.get(1));
 
@@ -520,8 +520,8 @@ public void testLatestVersionLoaded() throws Exception {
         logger.info("--> starting the two nodes back");
 
         internalCluster().startNodes(
-            Settings.builder().put(node1DataPathSettings).put("gateway.recover_after_nodes", 2).build(),
-            Settings.builder().put(node2DataPathSettings).put("gateway.recover_after_nodes", 2).build()
+            Settings.builder().put(node1DataPathSettings).put(RECOVER_AFTER_DATA_NODES_SETTING.getKey(), 2).build(),
+            Settings.builder().put(node2DataPathSettings).put(RECOVER_AFTER_DATA_NODES_SETTING.getKey(), 2).build()
         );
 
         logger.info("--> running cluster_health (wait for the shards to startup)");
@@ -710,7 +710,7 @@ public void testStartedShardFoundIfStateNotYetProcessed() throws Exception {
             @Override
             public Settings onNodeStopped(String nodeName) throws Exception {
                 // make sure state is not recovered
-                return Settings.builder().put(RECOVER_AFTER_NODES_SETTING.getKey(), 2).build();
+                return Settings.builder().put(RECOVER_AFTER_DATA_NODES_SETTING.getKey(), 2).build();
             }
         });
 
diff --git a/server/src/main/java/org/opensearch/common/settings/ClusterSettings.java b/server/src/main/java/org/opensearch/common/settings/ClusterSettings.java
@@ -346,11 +346,7 @@ public void apply(Settings value, Settings current, Settings previous) {
                 NoClusterManagerBlockService.NO_MASTER_BLOCK_SETTING,  // deprecated
                 NoClusterManagerBlockService.NO_CLUSTER_MANAGER_BLOCK_SETTING,
                 GatewayService.EXPECTED_DATA_NODES_SETTING,
-                GatewayService.EXPECTED_MASTER_NODES_SETTING,
-                GatewayService.EXPECTED_NODES_SETTING,
                 GatewayService.RECOVER_AFTER_DATA_NODES_SETTING,
-                GatewayService.RECOVER_AFTER_MASTER_NODES_SETTING,
-                GatewayService.RECOVER_AFTER_NODES_SETTING,
                 GatewayService.RECOVER_AFTER_TIME_SETTING,
                 ShardsBatchGatewayAllocator.GATEWAY_ALLOCATOR_BATCH_SIZE,
                 ShardsBatchGatewayAllocator.PRIMARY_BATCH_ALLOCATOR_TIMEOUT_SETTING,
diff --git a/server/src/main/java/org/opensearch/gateway/GatewayService.java b/server/src/main/java/org/opensearch/gateway/GatewayService.java
diff --git a/server/src/test/java/org/opensearch/gateway/GatewayServiceTests.java b/server/src/test/java/org/opensearch/gateway/GatewayServiceTests.java

Original file line number	Diff line number	Diff line change
`@@ -77,7 +77,7 @@ ${path.logs}`
`77`	`77`	`#`
`78`	`78`	`# Block initial recovery after a full cluster restart until N nodes are started:`
`79`	`79`	`#`
`80`		`-#gateway.recover_after_nodes: 3`
	`80`	`+#gateway.recover_after_data_nodes: 3`
`81`	`81`	`#`
`82`	`82`	`# For more information, consult the gateway module documentation.`
`83`	`83`	`#`
Original file line number	Diff line number	Diff line change
`@@ -95,7 +95,7 @@ public void testPendingTasksWithClusterNotRecoveredBlock() throws Exception {`
`95`	`95`	`internalCluster().fullRestart(new InternalTestCluster.RestartCallback() {`
`96`	`96`	`@Override`
`97`	`97`	`public Settings onNodeStopped(String nodeName) {`
`98`		`- return Settings.builder().put(GatewayService.RECOVER_AFTER_NODES_SETTING.getKey(), nodeCount + 1).build();`
	`98`	`+ return Settings.builder().put(GatewayService.RECOVER_AFTER_DATA_NODES_SETTING.getKey(), nodeCount + 1).build();`
`99`	`99`	`}`
`100`	`100`
`101`	`101`	`@Override`