Skip to content

Commit

Permalink
Remove deprecated settings to defer cluster recovery (opensearch-proj…
Browse files Browse the repository at this point in the history
…ect#17357)

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]>
  • Loading branch information
andrross authored Feb 14, 2025
1 parent 9de21d1 commit 2d2b41d
Show file tree
Hide file tree
Showing 10 changed files with 22 additions and 202 deletions.
1 change: 1 addition & 0 deletions CHANGELOG-3.0.md
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
2 changes: 1 addition & 1 deletion distribution/src/config/opensearch.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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.
#
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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()
Expand Down Expand Up @@ -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)");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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();
}
Expand All @@ -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));

Expand Down Expand Up @@ -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)");
Expand Down Expand Up @@ -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();
}
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Loading

0 comments on commit 2d2b41d

Please sign in to comment.