Remove deprecated settings to defer cluster recovery

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 <[email protected]>
andrross · Feb 14, 2025 · 34e05f6 · 34e05f6
1 parent 38e4b33
commit 34e05f6
Show file tree

Hide file tree

Showing 9 changed files with 21 additions and 202 deletions.
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/...rc/internalClusterTest/java/org/opensearch/action/admin/cluster/stats/ClusterStatsIT.java b/...rc/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/...ernalClusterTest/java/org/opensearch/action/admin/cluster/tasks/PendingTasksBlocksIT.java b/...ernalClusterTest/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/.../internalClusterTest/java/org/opensearch/action/admin/indices/exists/IndicesExistsIT.java b/.../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
@@ -68,51 +68,23 @@
 public class GatewayService extends AbstractLifecycleComponent implements ClusterStateListener {
     private static final Logger logger = LogManager.getLogger(GatewayService.class);
 
-    public static final Setting<Integer> EXPECTED_NODES_SETTING = Setting.intSetting(
-        "gateway.expected_nodes",
-        -1,
-        -1,
-        Property.NodeScope,
-        Property.Deprecated
-    );
     public static final Setting<Integer> EXPECTED_DATA_NODES_SETTING = Setting.intSetting(
         "gateway.expected_data_nodes",
         -1,
         -1,
         Property.NodeScope
     );
-    public static final Setting<Integer> EXPECTED_MASTER_NODES_SETTING = Setting.intSetting(
-        "gateway.expected_master_nodes",
-        -1,
-        -1,
-        Property.NodeScope,
-        Property.Deprecated
-    );
     public static final Setting<TimeValue> RECOVER_AFTER_TIME_SETTING = Setting.positiveTimeSetting(
         "gateway.recover_after_time",
         TimeValue.timeValueMillis(0),
         Property.NodeScope
     );
-    public static final Setting<Integer> RECOVER_AFTER_NODES_SETTING = Setting.intSetting(
-        "gateway.recover_after_nodes",
-        -1,
-        -1,
-        Property.NodeScope,
-        Property.Deprecated
-    );
     public static final Setting<Integer> RECOVER_AFTER_DATA_NODES_SETTING = Setting.intSetting(
         "gateway.recover_after_data_nodes",
         -1,
         -1,
         Property.NodeScope
     );
-    public static final Setting<Integer> RECOVER_AFTER_MASTER_NODES_SETTING = Setting.intSetting(
-        "gateway.recover_after_master_nodes",
-        0,
-        0,
-        Property.NodeScope,
-        Property.Deprecated
-    );
 
     public static final ClusterBlock STATE_NOT_RECOVERED_BLOCK = new ClusterBlock(
         1,
@@ -133,12 +105,8 @@ public class GatewayService extends AbstractLifecycleComponent implements Cluste
     private final ClusterService clusterService;
 
     private final TimeValue recoverAfterTime;
-    private final int recoverAfterNodes;
-    private final int expectedNodes;
     private final int recoverAfterDataNodes;
     private final int expectedDataNodes;
-    private final int recoverAfterClusterManagerNodes;
-    private final int expectedClusterManagerNodes;
 
     private final Runnable recoveryRunnable;
 
@@ -158,25 +126,16 @@ public GatewayService(
         this.clusterService = clusterService;
         this.threadPool = threadPool;
         // allow to control a delay of when indices will get created
-        this.expectedNodes = EXPECTED_NODES_SETTING.get(settings);
         this.expectedDataNodes = EXPECTED_DATA_NODES_SETTING.get(settings);
-        this.expectedClusterManagerNodes = EXPECTED_MASTER_NODES_SETTING.get(settings);
 
         if (RECOVER_AFTER_TIME_SETTING.exists(settings)) {
             recoverAfterTime = RECOVER_AFTER_TIME_SETTING.get(settings);
-        } else if (expectedNodes >= 0 || expectedDataNodes >= 0 || expectedClusterManagerNodes >= 0) {
+        } else if (expectedDataNodes >= 0) {
             recoverAfterTime = DEFAULT_RECOVER_AFTER_TIME_IF_EXPECTED_NODES_IS_SET;
         } else {
             recoverAfterTime = null;
         }
-        this.recoverAfterNodes = RECOVER_AFTER_NODES_SETTING.get(settings);
         this.recoverAfterDataNodes = RECOVER_AFTER_DATA_NODES_SETTING.get(settings);
-        // default the recover after cluster-manager nodes to the minimum cluster-manager nodes in the discovery
-        if (RECOVER_AFTER_MASTER_NODES_SETTING.exists(settings)) {
-            recoverAfterClusterManagerNodes = RECOVER_AFTER_MASTER_NODES_SETTING.get(settings);
-        } else {
-            recoverAfterClusterManagerNodes = -1;
-        }
 
         if (discovery instanceof Coordinator) {
             recoveryRunnable = () -> clusterService.submitStateUpdateTask("local-gateway-elected-state", new RecoverStateUpdateTask());
@@ -222,54 +181,26 @@ public void clusterChanged(final ClusterChangedEvent event) {
         final DiscoveryNodes nodes = state.nodes();
         if (state.nodes().getClusterManagerNodeId() == null) {
             logger.debug("not recovering from gateway, no cluster-manager elected yet");
-        } else if (recoverAfterNodes != -1 && (nodes.getClusterManagerAndDataNodes().size()) < recoverAfterNodes) {
-            logger.debug(
-                "not recovering from gateway, nodes_size (data+master) [{}] < recover_after_nodes [{}]",
-                nodes.getClusterManagerAndDataNodes().size(),
-                recoverAfterNodes
-            );
         } else if (recoverAfterDataNodes != -1 && nodes.getDataNodes().size() < recoverAfterDataNodes) {
             logger.debug(
                 "not recovering from gateway, nodes_size (data) [{}] < recover_after_data_nodes [{}]",
                 nodes.getDataNodes().size(),
                 recoverAfterDataNodes
             );
-        } else if (recoverAfterClusterManagerNodes != -1 && nodes.getClusterManagerNodes().size() < recoverAfterClusterManagerNodes) {
-            logger.debug(
-                "not recovering from gateway, nodes_size (master) [{}] < recover_after_master_nodes [{}]",
-                nodes.getClusterManagerNodes().size(),
-                recoverAfterClusterManagerNodes
-            );
         } else {
             boolean enforceRecoverAfterTime;
             String reason;
-            if (expectedNodes == -1 && expectedClusterManagerNodes == -1 && expectedDataNodes == -1) {
+            if (expectedDataNodes == -1) {
                 // no expected is set, honor the setting if they are there
                 enforceRecoverAfterTime = true;
                 reason = "recover_after_time was set to [" + recoverAfterTime + "]";
             } else {
                 // one of the expected is set, see if all of them meet the need, and ignore the timeout in this case
                 enforceRecoverAfterTime = false;
                 reason = "";
-                if (expectedNodes != -1 && (nodes.getClusterManagerAndDataNodes().size() < expectedNodes)) { // does not meet the
-                                                                                                             // expected...
-                    enforceRecoverAfterTime = true;
-                    reason = "expecting ["
-                        + expectedNodes
-                        + "] nodes, but only have ["
-                        + nodes.getClusterManagerAndDataNodes().size()
-                        + "]";
-                } else if (expectedDataNodes != -1 && (nodes.getDataNodes().size() < expectedDataNodes)) { // does not meet the expected...
+                if (nodes.getDataNodes().size() < expectedDataNodes) { // does not meet the expected...
                     enforceRecoverAfterTime = true;
                     reason = "expecting [" + expectedDataNodes + "] data nodes, but only have [" + nodes.getDataNodes().size() + "]";
-                } else if (expectedClusterManagerNodes != -1 && (nodes.getClusterManagerNodes().size() < expectedClusterManagerNodes)) {
-                    // does not meet the expected...
-                    enforceRecoverAfterTime = true;
-                    reason = "expecting ["
-                        + expectedClusterManagerNodes
-                        + "] cluster-manager nodes, but only have ["
-                        + nodes.getClusterManagerNodes().size()
-                        + "]";
                 }
             }
             performStateRecovery(enforceRecoverAfterTime, reason);