Don't accept a dynamic update to `min_master_nodes` which is larger then current master node count #8321

Closed
wants to merge 1 commit into
from

Projects

None yet

3 participants

@bleskes
Member
bleskes commented Nov 2, 2014

The discovery.zen.minimum_master_nodes setting can be updated dynamically. Settings it to a value higher then the current number of master nodes will cause the current master to step down. This is dangerous because if done by mistake (typo) there is no way to restore the settings (this requires an active master).

@bleskes bleskes Discovery: don't accept a dynamic update to min_master_nodes which is…
… larger then current master node count

The discovery.zen.minimum_master_nodes setting can be updated dynamically. Settings it to a value higher then the current number of master nodes will cause the current master to step down. This is dangerous because if done by mistake (typo) there is no way to restore the settings (this requires an active master).
31c2f55
@martijnvg martijnvg and 1 other commented on an outdated diff Nov 3, 2014
...rg/elasticsearch/cluster/MinimumMasterNodesTests.java
@@ -308,4 +312,40 @@ public boolean apply(Object obj) {
}
}, 20, TimeUnit.SECONDS), equalTo(true));
}
+
+ @Test
+ public void testCanNotBringClusterDown() throws ExecutionException, InterruptedException {
+ int nodeCount = scaledRandomIntBetween(1, 1);
@martijnvg
martijnvg Nov 3, 2014 Member

Maybe the max should be more? Something like 5

@bleskes
bleskes Nov 3, 2014 Member

I had commit on my machine I forget to push changing this back to it's original value - 10

@martijnvg martijnvg commented on the diff Nov 3, 2014
...va/org/elasticsearch/cluster/ClusterServiceTests.java
assertThat(clusterService1.state().nodes().localNodeMaster(), is(true));
assertThat(testService1.master(), is(true));
+ // start another node and set min_master_node
@martijnvg
martijnvg Nov 3, 2014 Member

Why did this test needed to be changed as well?

@bleskes
bleskes Nov 3, 2014 Member

the test used to set the min_master_node to 2 in order to cause the single node to step down from being a master. This is not possible any more.

@martijnvg
martijnvg Nov 3, 2014 Member

ah got it!

@martijnvg
Member

@bleskes This looks good. I left two questions / comments.

@martijnvg martijnvg removed the review label Nov 3, 2014
@bleskes
Member
bleskes commented Nov 3, 2014

@martijnvg thx. responded.

@martijnvg
Member

@bleskes LGTM

@bleskes bleskes added a commit that closed this pull request Nov 3, 2014
@bleskes bleskes Discovery: don't accept a dynamic update to min_master_nodes which is…
… larger then current master node count

The discovery.zen.minimum_master_nodes setting can be updated dynamically. Settings it to a value higher then the current number of master nodes will cause the current master to step down. This is dangerous because if done by mistake (typo) there is no way to restore the settings (this requires an active master).

Closes #8321
f1f50ac
@bleskes bleskes closed this in f1f50ac Nov 3, 2014
@bleskes bleskes added a commit that referenced this pull request Nov 3, 2014
@bleskes bleskes Discovery: don't accept a dynamic update to min_master_nodes which is…
… larger then current master node count

The discovery.zen.minimum_master_nodes setting can be updated dynamically. Settings it to a value higher then the current number of master nodes will cause the current master to step down. This is dangerous because if done by mistake (typo) there is no way to restore the settings (this requires an active master).

Closes #8321
00932a5
@bleskes bleskes deleted the bleskes:min_master_nodes_validation branch Nov 3, 2014
@bleskes bleskes added the resiliency label Feb 2, 2015
@clintongormley clintongormley changed the title from Discovery: don't accept a dynamic update to min_master_nodes which s larger then current master node count to Don't accept a dynamic update to `min_master_nodes` which is larger then current master node count Jun 6, 2015
@clintongormley clintongormley added :Settings and removed :Discovery labels Jun 6, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment