Skip to content

Commit

Permalink
Remove deprecated settings to defer cluster recovery
Browse files Browse the repository at this point in the history
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 committed Feb 14, 2025
1 parent 38e4b33 commit 99b25aa
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 99b25aa

Please sign in to comment.