Skip to content

Commit

Permalink
Deprecate minimum_master_nodes (#37868)
Browse files Browse the repository at this point in the history
Today we pass `discovery.zen.minimum_master_nodes` to nodes started up in
tests, but for 7.x nodes this setting is not required as it has no effect.
This commit removes this setting so that nodes are started with more realistic
configurations, and deprecates it.
  • Loading branch information
DaveCTurner authored Jan 30, 2019
1 parent 6a78b6a commit 81c443c
Show file tree
Hide file tree
Showing 23 changed files with 140 additions and 278 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -373,18 +373,15 @@ class ClusterFormationTasks {
'path.repo' : "${node.sharedDir}/repo",
'path.shared_data' : "${node.sharedDir}/",
// Define a node attribute so we can test that it exists
'node.attr.testattr' : 'test'
'node.attr.testattr' : 'test',
// Don't wait for state, just start up quickly. This will also allow new and old nodes in the BWC case to become the master
'discovery.initial_state_timeout' : '0s'
]
int minimumMasterNodes = node.config.minimumMasterNodes.call()
if (minimumMasterNodes > 0) {
if (node.nodeVersion.before("7.0.0") && minimumMasterNodes > 0) {
esConfig['discovery.zen.minimum_master_nodes'] = minimumMasterNodes
}
if (minimumMasterNodes > 1) {
// don't wait for state.. just start up quickly
// this will also allow new and old nodes in the BWC case to become the master
esConfig['discovery.initial_state_timeout'] = '0s'
}
if (esConfig.containsKey('discovery.zen.master_election.wait_for_joins_timeout') == false) {
if (node.nodeVersion.before("7.0.0") && esConfig.containsKey('discovery.zen.master_election.wait_for_joins_timeout') == false) {
// If a node decides to become master based on partial information from the pinging, don't let it hang for 30 seconds to correct
// its mistake. Instead, only wait 5s to do another round of pinging.
// This is necessary since we use 30s as the default timeout in REST requests waiting for cluster formation
Expand Down
4 changes: 2 additions & 2 deletions docs/reference/migration/migrate_7_0/discovery.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@ settings summary>> for an example, and the
<<modules-discovery-bootstrap-cluster,cluster bootstrapping reference
documentation>> describes this setting in more detail.

The `discovery.zen.minimum_master_nodes` setting is required during a rolling
upgrade from 6.x, but can be removed in all other circumstances.
The `discovery.zen.minimum_master_nodes` setting is permitted, but ignored, on
7.x nodes.

[float]
==== Removing master-eligible nodes sometimes requires voting exclusions
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,7 @@
import org.elasticsearch.cluster.routing.UnassignedInfo;
import org.elasticsearch.common.Priority;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.settings.Settings.Builder;
import org.elasticsearch.common.unit.TimeValue;
import org.elasticsearch.discovery.zen.ElectMasterService;
import org.elasticsearch.http.HttpServerTransport;
import org.elasticsearch.test.ESIntegTestCase;
import org.elasticsearch.test.InternalTestCluster;
Expand All @@ -58,11 +56,7 @@ public class Zen2RestApiIT extends ESNetty4IntegTestCase {

@Override
protected Settings nodeSettings(int nodeOrdinal) {
final Builder builder = Settings.builder().put(super.nodeSettings(nodeOrdinal))
.put(TestZenDiscovery.USE_ZEN2.getKey(), true)
.put(ElectMasterService.DISCOVERY_ZEN_MINIMUM_MASTER_NODES_SETTING.getKey(), Integer.MAX_VALUE);

return builder.build();
return Settings.builder().put(super.nodeSettings(nodeOrdinal)).put(TestZenDiscovery.USE_ZEN2.getKey(), true).build();
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@


import org.elasticsearch.action.admin.cluster.settings.ClusterUpdateSettingsResponse;
import org.elasticsearch.common.UUIDs;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.discovery.DiscoveryModule;
import org.elasticsearch.test.ESIntegTestCase.ClusterScope;
Expand All @@ -41,13 +42,14 @@ public void testMinimumMasterNodesStart() {
.build();
internalCluster().startNode(nodeSettings);

// We try to update minimum_master_nodes now
ClusterUpdateSettingsResponse response = client().admin().cluster().prepareUpdateSettings()
.setPersistentSettings(Settings.builder().put("discovery.zen.minimum_master_nodes", 1))
.setTransientSettings(Settings.builder().put("discovery.zen.minimum_master_nodes", 1))
// We try to update a setting now
final String expectedValue = UUIDs.randomBase64UUID(random());
final String settingName = "cluster.routing.allocation.exclude.any_attribute";
final ClusterUpdateSettingsResponse response = client().admin().cluster().prepareUpdateSettings()
.setPersistentSettings(Settings.builder().put(settingName, expectedValue))
.get();

Integer min = response.getPersistentSettings().getAsInt("discovery.zen.minimum_master_nodes", null);
assertThat(min, is(1));
final String value = response.getPersistentSettings().get(settingName);
assertThat(value, is(expectedValue));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@
* The intention is to confirm that users can still run their Elasticsearch instances with previous configurations.
*/
public class CustomLoggingConfigIT extends ESRestTestCase {
private static final String NODE_STARTED = ".*node-0.*cluster.uuid.*node.id.*started.*";
private static final String NODE_STARTED = ".*node-0.*cluster.uuid.*node.id.*recovered.*cluster_state.*";

public void testSuccessfulStartupWithCustomConfig() throws Exception {
assertBusy(() -> {
Expand Down
1 change: 0 additions & 1 deletion qa/rolling-upgrade/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,6 @@ for (Version version : bwcVersions.wireCompatible) {
dependsOn lastRunner, "${baseName}#oldClusterTestCluster#node${stopNode}.stop"
clusterName = 'rolling-upgrade'
otherUnicastHostAddresses = { getOtherUnicastHostAddresses() }
minimumMasterNodes = { 2 }
autoSetInitialMasterNodes = false
/* Override the data directory so the new node always gets the node we
* just stopped's data directory. */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,22 +4,22 @@
cluster.put_settings:
body:
transient:
discovery.zen.minimum_master_nodes: 1
cluster.routing.allocation.enable: "none"
flat_settings: true

- match: {transient: {discovery.zen.minimum_master_nodes: "1"}}
- match: {transient: {cluster.routing.allocation.enable: "none"}}

- do:
cluster.get_settings:
flat_settings: true

- match: {transient: {discovery.zen.minimum_master_nodes: "1"}}
- match: {transient: {cluster.routing.allocation.enable: "none"}}

- do:
cluster.put_settings:
body:
transient:
discovery.zen.minimum_master_nodes: null
cluster.routing.allocation.enable: null
flat_settings: true

- match: {transient: {}}
Expand All @@ -35,22 +35,22 @@
cluster.put_settings:
body:
persistent:
cluster.routing.allocation.disk.threshold_enabled: false
cluster.routing.allocation.enable: "none"
flat_settings: true

- match: {persistent: {cluster.routing.allocation.disk.threshold_enabled: "false"}}
- match: {persistent: {cluster.routing.allocation.enable: "none"}}

- do:
cluster.get_settings:
flat_settings: true

- match: {persistent: {cluster.routing.allocation.disk.threshold_enabled: "false"}}
- match: {persistent: {cluster.routing.allocation.enable: "none"}}

- do:
cluster.put_settings:
body:
persistent:
cluster.routing.allocation.disk.threshold_enabled: null
cluster.routing.allocation.enable: null
flat_settings: true

- match: {persistent: {}}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ public class ElectMasterService {
private static final Logger logger = LogManager.getLogger(ElectMasterService.class);

public static final Setting<Integer> DISCOVERY_ZEN_MINIMUM_MASTER_NODES_SETTING =
Setting.intSetting("discovery.zen.minimum_master_nodes", -1, Property.Dynamic, Property.NodeScope);
Setting.intSetting("discovery.zen.minimum_master_nodes", -1, Property.Dynamic, Property.NodeScope, Property.Deprecated);

private volatile int minimumMasterNodes;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.unit.TimeValue;
import org.elasticsearch.discovery.MasterNotDiscoveredException;
import org.elasticsearch.discovery.zen.ElectMasterService;
import org.elasticsearch.gateway.GatewayService;
import org.elasticsearch.test.ESIntegTestCase;
import org.elasticsearch.test.ESIntegTestCase.ClusterScope;
Expand Down Expand Up @@ -51,8 +50,7 @@ protected List<Settings> addExtraClusterBootstrapSettings(List<Settings> allNode

public void testIndexExistsWithBlocksInPlace() throws IOException {
Settings settings = Settings.builder()
.put(GatewayService.RECOVER_AFTER_NODES_SETTING.getKey(), 99)
.put(ElectMasterService.DISCOVERY_ZEN_MINIMUM_MASTER_NODES_SETTING.getKey(), Integer.MAX_VALUE).build();
.put(GatewayService.RECOVER_AFTER_NODES_SETTING.getKey(), 99).build();
String node = internalCluster().startNode(settings);

assertThrows(client(node).admin().indices().prepareExists("test").setMasterNodeTimeout(TimeValue.timeValueSeconds(0)),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,14 +26,13 @@
import org.elasticsearch.action.admin.cluster.health.ClusterHealthResponse;
import org.elasticsearch.client.Client;
import org.elasticsearch.cluster.coordination.ClusterBootstrapService;
import org.elasticsearch.cluster.coordination.FailedToCommitClusterStateException;
import org.elasticsearch.cluster.metadata.MetaData;
import org.elasticsearch.cluster.node.DiscoveryNode;
import org.elasticsearch.cluster.service.ClusterService;
import org.elasticsearch.common.Priority;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.discovery.DiscoverySettings;
import org.elasticsearch.cluster.coordination.FailedToCommitClusterStateException;
import org.elasticsearch.discovery.zen.ElectMasterService;
import org.elasticsearch.discovery.zen.ZenDiscovery;
import org.elasticsearch.index.query.QueryBuilders;
import org.elasticsearch.node.Node;
Expand Down Expand Up @@ -102,7 +101,6 @@ public void testTwoNodesNoMasterBlock() throws Exception {
bootstrapNodeId = 2;

Settings settings = Settings.builder()
.put("discovery.zen.minimum_master_nodes", 2)
.put(ZenDiscovery.PING_TIMEOUT_SETTING.getKey(), "200ms")
.put("discovery.initial_state_timeout", "500ms")
.build();
Expand Down Expand Up @@ -237,7 +235,6 @@ public void testThreeNodesNoMasterBlock() throws Exception {
bootstrapNodeId = 3;

Settings settings = Settings.builder()
.put("discovery.zen.minimum_master_nodes", 3)
.put(ZenDiscovery.PING_TIMEOUT_SETTING.getKey(), "1s")
.put("discovery.initial_state_timeout", "500ms")
.build();
Expand Down Expand Up @@ -316,11 +313,9 @@ public void testCannotCommitStateThreeNodes() throws Exception {
Settings settings = Settings.builder()
.put(ZenDiscovery.PING_TIMEOUT_SETTING.getKey(), "200ms")
.put("discovery.initial_state_timeout", "500ms")
.put(ElectMasterService.DISCOVERY_ZEN_MINIMUM_MASTER_NODES_SETTING.getKey(), 2)
.put(DiscoverySettings.COMMIT_TIMEOUT_SETTING.getKey(), "100ms") // speed things up
.build();


internalCluster().startNodes(3, settings);
ensureGreen(); // ensure cluster state is recovered before we disrupt things

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@
import java.util.Collections;
import java.util.List;

import static org.elasticsearch.discovery.zen.ElectMasterService.DISCOVERY_ZEN_MINIMUM_MASTER_NODES_SETTING;
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.nullValue;
Expand All @@ -47,12 +46,6 @@
@TestLogging("_root:DEBUG,org.elasticsearch.action.admin.cluster.state:TRACE")
public class SpecificMasterNodesIT extends ESIntegTestCase {

@Override
protected Settings nodeSettings(int nodeOrdinal) {
return Settings.builder().put(super.nodeSettings(nodeOrdinal))
.put(DISCOVERY_ZEN_MINIMUM_MASTER_NODES_SETTING.getKey(), 1).build();
}

@Override
protected List<Settings> addExtraClusterBootstrapSettings(List<Settings> allNodesSettings) {
// if it's the first master in the cluster bootstrap the cluster with this node name
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@
import org.elasticsearch.cluster.metadata.MetaData;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.discovery.DiscoverySettings;
import org.elasticsearch.discovery.zen.ElectMasterService;
import org.elasticsearch.env.Environment;
import org.elasticsearch.env.NodeEnvironment;
import org.elasticsearch.env.NodeMetaData;
Expand Down Expand Up @@ -155,7 +154,6 @@ public void testNoNodeMetaData() throws IOException {
public void testNotBootstrappedCluster() throws Exception {
internalCluster().startNode(
Settings.builder()
.put(ElectMasterService.DISCOVERY_ZEN_MINIMUM_MASTER_NODES_SETTING.getKey(), Integer.MAX_VALUE)
.put(DiscoverySettings.INITIAL_STATE_TIMEOUT_SETTING.getKey(), "0s") // to ensure quick node startup
.build());
assertBusy(() -> {
Expand All @@ -172,9 +170,7 @@ public void testNotBootstrappedCluster() throws Exception {

public void testNoManifestFile() throws IOException {
bootstrapNodeId = 1;
internalCluster().startNode(Settings.builder()
.put(ElectMasterService.DISCOVERY_ZEN_MINIMUM_MASTER_NODES_SETTING.getKey(), Integer.MAX_VALUE)
.build());
internalCluster().startNode();
ensureStableCluster(1);
NodeEnvironment nodeEnvironment = internalCluster().getMasterNodeInstance(NodeEnvironment.class);
internalCluster().stopRandomDataNode();
Expand All @@ -186,9 +182,7 @@ public void testNoManifestFile() throws IOException {

public void testNoMetaData() throws IOException {
bootstrapNodeId = 1;
internalCluster().startNode(Settings.builder()
.put(ElectMasterService.DISCOVERY_ZEN_MINIMUM_MASTER_NODES_SETTING.getKey(), Integer.MAX_VALUE)
.build());
internalCluster().startNode();
ensureStableCluster(1);
NodeEnvironment nodeEnvironment = internalCluster().getMasterNodeInstance(NodeEnvironment.class);
internalCluster().stopRandomDataNode();
Expand All @@ -201,9 +195,7 @@ public void testNoMetaData() throws IOException {

public void testAbortedByUser() throws IOException {
bootstrapNodeId = 1;
internalCluster().startNode(Settings.builder()
.put(ElectMasterService.DISCOVERY_ZEN_MINIMUM_MASTER_NODES_SETTING.getKey(), Integer.MAX_VALUE)
.build());
internalCluster().startNode();
ensureStableCluster(1);
internalCluster().stopRandomDataNode();

Expand All @@ -213,13 +205,9 @@ public void testAbortedByUser() throws IOException {

public void test3MasterNodes2Failed() throws Exception {
bootstrapNodeId = 3;
List<String> masterNodes = internalCluster().startMasterOnlyNodes(3, Settings.builder()
.put(ElectMasterService.DISCOVERY_ZEN_MINIMUM_MASTER_NODES_SETTING.getKey(), Integer.MAX_VALUE)
.build());
List<String> masterNodes = internalCluster().startMasterOnlyNodes(3, Settings.EMPTY);

String dataNode = internalCluster().startDataOnlyNode(Settings.builder()
.put(ElectMasterService.DISCOVERY_ZEN_MINIMUM_MASTER_NODES_SETTING.getKey(), Integer.MAX_VALUE)
.build());
String dataNode = internalCluster().startDataOnlyNode();
createIndex("test");

Client dataNodeClient = internalCluster().client(dataNode);
Expand All @@ -246,9 +234,7 @@ public void test3MasterNodes2Failed() throws Exception {
String.format(Locale.ROOT, UnsafeBootstrapMasterCommand.CLUSTER_STATE_TERM_VERSION_MSG_FORMAT,
metaData.coordinationMetaData().term(), metaData.version())));

internalCluster().startMasterOnlyNode(Settings.builder()
.put(ElectMasterService.DISCOVERY_ZEN_MINIMUM_MASTER_NODES_SETTING.getKey(), Integer.MAX_VALUE)
.build());
internalCluster().startMasterOnlyNode();

assertBusy(() -> {
ClusterState state = dataNodeClient.admin().cluster().prepareState().setLocal(true)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
import org.elasticsearch.common.Priority;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.unit.TimeValue;
import org.elasticsearch.discovery.zen.ElectMasterService;
import org.elasticsearch.env.NodeEnvironment;
import org.elasticsearch.gateway.MetaStateService;
import org.elasticsearch.test.ESIntegTestCase;
Expand All @@ -50,6 +51,7 @@
import static org.elasticsearch.cluster.routing.allocation.decider.EnableAllocationDecider.CLUSTER_ROUTING_ALLOCATION_ENABLE_SETTING;
import static org.elasticsearch.cluster.routing.allocation.decider.FilterAllocationDecider.CLUSTER_ROUTING_EXCLUDE_GROUP_SETTING;
import static org.elasticsearch.node.Node.NODE_NAME_SETTING;
import static org.elasticsearch.test.InternalTestCluster.REMOVED_MINIMUM_MASTER_NODES;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.is;

Expand Down Expand Up @@ -194,7 +196,8 @@ public Settings onNodeStopped(String nodeName) {
}
ClusterHealthResponse clusterHealthResponse = clusterHealthRequestBuilder.get();
assertFalse(nodeName, clusterHealthResponse.isTimedOut());
return Coordinator.addZen1Attribute(false, Settings.builder().put(ZEN2_SETTINGS)).build();
return Coordinator.addZen1Attribute(false, Settings.builder().put(ZEN2_SETTINGS)
.put(ElectMasterService.DISCOVERY_ZEN_MINIMUM_MASTER_NODES_SETTING.getKey(), REMOVED_MINIMUM_MASTER_NODES)).build();
}
});

Expand Down Expand Up @@ -289,6 +292,7 @@ public Settings onNodeStopped(String nodeName) throws Exception {
return Coordinator.addZen1Attribute(false, Settings.builder())
.put(ZEN2_SETTINGS)
.putList(INITIAL_MASTER_NODES_SETTING.getKey(), nodeNames)
.put(ElectMasterService.DISCOVERY_ZEN_MINIMUM_MASTER_NODES_SETTING.getKey(), REMOVED_MINIMUM_MASTER_NODES)
.build();
}
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.unit.TimeValue;
import org.elasticsearch.common.xcontent.XContentType;
import org.elasticsearch.discovery.zen.ElectMasterService;
import org.elasticsearch.discovery.zen.ZenDiscovery;
import org.elasticsearch.monitor.jvm.HotThreads;
import org.elasticsearch.test.ESIntegTestCase;
Expand Down Expand Up @@ -125,11 +124,6 @@ public void testNodesFDAfterMasterReelection() throws Exception {

ensureStableCluster(3);

logger.info("--> reducing min master nodes to 2");
assertAcked(client().admin().cluster().prepareUpdateSettings()
.setTransientSettings(Settings.builder().put(ElectMasterService.DISCOVERY_ZEN_MINIMUM_MASTER_NODES_SETTING.getKey(), 2))
.get());

String master = internalCluster().getMasterName();
String nonMaster = null;
for (String node : internalCluster().getNodeNames()) {
Expand All @@ -154,7 +148,7 @@ public void testNodesFDAfterMasterReelection() throws Exception {
*/
@TestLogging("_root:DEBUG,org.elasticsearch.cluster.service:TRACE,org.elasticsearch.test.disruption:TRACE")
public void testStaleMasterNotHijackingMajority() throws Exception {
// 3 node cluster with unicast discovery and minimum_master_nodes set to the default of 2:
// 3 node cluster with unicast discovery:
final List<String> nodes = startCluster(3);

// Save the current master node as old master node, because that node will get frozen
Expand Down
Loading

0 comments on commit 81c443c

Please sign in to comment.