From 0fb31d9e7a4c39ff4b45a1cca75e8c367e90b7ec Mon Sep 17 00:00:00 2001 From: David Turner Date: Mon, 23 Mar 2020 08:36:09 +0000 Subject: [PATCH] Allow static cluster.max_voting_config_exclusions (#53717) Today we only read `cluster.max_voting_config_exclusions` from the dynamic settings in the cluster metadata, ignoring any value set in `elasticsearch.yml`. This commit addresses this. Closes #53455 --- ...nsportAddVotingConfigExclusionsAction.java | 32 +++++++++---- ...tAddVotingConfigExclusionsActionTests.java | 45 ++++++++++++++----- 2 files changed, 58 insertions(+), 19 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/configuration/TransportAddVotingConfigExclusionsAction.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/configuration/TransportAddVotingConfigExclusionsAction.java index 51176ae989c..38d369e25bf 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/configuration/TransportAddVotingConfigExclusionsAction.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/configuration/TransportAddVotingConfigExclusionsAction.java @@ -39,8 +39,10 @@ import org.elasticsearch.cluster.service.ClusterService; import org.elasticsearch.common.Priority; import org.elasticsearch.common.inject.Inject; import org.elasticsearch.common.io.stream.StreamInput; +import org.elasticsearch.common.settings.ClusterSettings; import org.elasticsearch.common.settings.Setting; import org.elasticsearch.common.settings.Setting.Property; +import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.unit.TimeValue; import org.elasticsearch.threadpool.ThreadPool; import org.elasticsearch.threadpool.ThreadPool.Names; @@ -59,11 +61,21 @@ public class TransportAddVotingConfigExclusionsAction extends TransportMasterNod public static final Setting MAXIMUM_VOTING_CONFIG_EXCLUSIONS_SETTING = Setting.intSetting("cluster.max_voting_config_exclusions", 10, 1, Property.Dynamic, Property.NodeScope); + private volatile int maxVotingConfigExclusions; + @Inject - public TransportAddVotingConfigExclusionsAction(TransportService transportService, ClusterService clusterService, ThreadPool threadPool, - ActionFilters actionFilters, IndexNameExpressionResolver indexNameExpressionResolver) { + public TransportAddVotingConfigExclusionsAction(Settings settings, ClusterSettings clusterSettings, TransportService transportService, + ClusterService clusterService, ThreadPool threadPool, ActionFilters actionFilters, + IndexNameExpressionResolver indexNameExpressionResolver) { super(AddVotingConfigExclusionsAction.NAME, transportService, clusterService, threadPool, actionFilters, AddVotingConfigExclusionsRequest::new, indexNameExpressionResolver); + + maxVotingConfigExclusions = MAXIMUM_VOTING_CONFIG_EXCLUSIONS_SETTING.get(settings); + clusterSettings.addSettingsUpdateConsumer(MAXIMUM_VOTING_CONFIG_EXCLUSIONS_SETTING, this::setMaxVotingConfigExclusions); + } + + private void setMaxVotingConfigExclusions(int maxVotingConfigExclusions) { + this.maxVotingConfigExclusions = maxVotingConfigExclusions; } @Override @@ -80,7 +92,8 @@ public class TransportAddVotingConfigExclusionsAction extends TransportMasterNod protected void masterOperation(AddVotingConfigExclusionsRequest request, ClusterState state, ActionListener listener) throws Exception { - resolveVotingConfigExclusionsAndCheckMaximum(request, state); // throws IAE if no nodes matched or maximum exceeded + resolveVotingConfigExclusionsAndCheckMaximum(request, state, maxVotingConfigExclusions); + // throws IAE if no nodes matched or maximum exceeded clusterService.submitStateUpdateTask("add-voting-config-exclusions", new ClusterStateUpdateTask(Priority.URGENT) { @@ -89,14 +102,14 @@ public class TransportAddVotingConfigExclusionsAction extends TransportMasterNod @Override public ClusterState execute(ClusterState currentState) { assert resolvedExclusions == null : resolvedExclusions; - resolvedExclusions = resolveVotingConfigExclusionsAndCheckMaximum(request, currentState); + final int finalMaxVotingConfigExclusions = TransportAddVotingConfigExclusionsAction.this.maxVotingConfigExclusions; + resolvedExclusions = resolveVotingConfigExclusionsAndCheckMaximum(request, currentState, finalMaxVotingConfigExclusions); final CoordinationMetaData.Builder builder = CoordinationMetaData.builder(currentState.coordinationMetaData()); resolvedExclusions.forEach(builder::addVotingConfigExclusion); final MetaData newMetaData = MetaData.builder(currentState.metaData()).coordinationMetaData(builder.build()).build(); final ClusterState newState = ClusterState.builder(currentState).metaData(newMetaData).build(); - assert newState.getVotingConfigExclusions().size() <= MAXIMUM_VOTING_CONFIG_EXCLUSIONS_SETTING.get( - currentState.metaData().settings()); + assert newState.getVotingConfigExclusions().size() <= finalMaxVotingConfigExclusions; return newState; } @@ -148,9 +161,10 @@ public class TransportAddVotingConfigExclusionsAction extends TransportMasterNod } private static Set resolveVotingConfigExclusionsAndCheckMaximum(AddVotingConfigExclusionsRequest request, - ClusterState state) { - return request.resolveVotingConfigExclusionsAndCheckMaximum(state, - MAXIMUM_VOTING_CONFIG_EXCLUSIONS_SETTING.get(state.metaData().settings()), MAXIMUM_VOTING_CONFIG_EXCLUSIONS_SETTING.getKey()); + ClusterState state, + int maxVotingConfigExclusions) { + return request.resolveVotingConfigExclusionsAndCheckMaximum(state, maxVotingConfigExclusions, + MAXIMUM_VOTING_CONFIG_EXCLUSIONS_SETTING.getKey()); } @Override diff --git a/server/src/test/java/org/elasticsearch/action/admin/cluster/configuration/TransportAddVotingConfigExclusionsActionTests.java b/server/src/test/java/org/elasticsearch/action/admin/cluster/configuration/TransportAddVotingConfigExclusionsActionTests.java index 2a7ac4bb3ba..2cb8c834ba2 100644 --- a/server/src/test/java/org/elasticsearch/action/admin/cluster/configuration/TransportAddVotingConfigExclusionsActionTests.java +++ b/server/src/test/java/org/elasticsearch/action/admin/cluster/configuration/TransportAddVotingConfigExclusionsActionTests.java @@ -37,6 +37,7 @@ import org.elasticsearch.cluster.node.DiscoveryNodeRole; import org.elasticsearch.cluster.node.DiscoveryNodes.Builder; import org.elasticsearch.cluster.service.ClusterService; import org.elasticsearch.common.io.stream.StreamInput; +import org.elasticsearch.common.settings.ClusterSettings; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.unit.TimeValue; import org.elasticsearch.test.ESTestCase; @@ -81,6 +82,8 @@ public class TransportAddVotingConfigExclusionsActionTests extends ESTestCase { private TransportService transportService; private ClusterStateObserver clusterStateObserver; + private ClusterSettings clusterSettings; + private int staticMaximum; @BeforeClass public static void createThreadPoolAndClusterService() { @@ -117,8 +120,18 @@ public class TransportAddVotingConfigExclusionsActionTests extends ESTestCase { transportService = transport.createTransportService(Settings.EMPTY, threadPool, TransportService.NOOP_TRANSPORT_INTERCEPTOR, boundTransportAddress -> localNode, null, emptySet()); - new TransportAddVotingConfigExclusionsAction(transportService, clusterService, threadPool, new ActionFilters(emptySet()), - new IndexNameExpressionResolver()); // registers action + final Settings.Builder nodeSettingsBuilder = Settings.builder(); + if (randomBoolean()) { + staticMaximum = between(5, 15); + nodeSettingsBuilder.put(MAXIMUM_VOTING_CONFIG_EXCLUSIONS_SETTING.getKey(), staticMaximum); + } else { + staticMaximum = MAXIMUM_VOTING_CONFIG_EXCLUSIONS_SETTING.get(Settings.EMPTY); + } + final Settings nodeSettings = nodeSettingsBuilder.build(); + clusterSettings = new ClusterSettings(nodeSettings, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS); + + new TransportAddVotingConfigExclusionsAction(nodeSettings, clusterSettings, transportService, clusterService, threadPool, + new ActionFilters(emptySet()), new IndexNameExpressionResolver()); // registers action transportService.start(); transportService.acceptIncomingRequests(); @@ -308,20 +321,32 @@ public class TransportAddVotingConfigExclusionsActionTests extends ESTestCase { } public void testReturnsErrorIfMaximumExclusionCountExceeded() throws InterruptedException { - final MetaData.Builder metaDataBuilder = MetaData.builder(clusterService.state().metaData()).persistentSettings( - Settings.builder().put(clusterService.state().metaData().persistentSettings()) - .put(MAXIMUM_VOTING_CONFIG_EXCLUSIONS_SETTING.getKey(), 2).build()); + final MetaData.Builder metaDataBuilder = MetaData.builder(clusterService.state().metaData()); CoordinationMetaData.Builder coordinationMetaDataBuilder = - CoordinationMetaData.builder(clusterService.state().coordinationMetaData()) - .addVotingConfigExclusion(localNodeExclusion); + CoordinationMetaData.builder(clusterService.state().coordinationMetaData()) + .addVotingConfigExclusion(localNodeExclusion); + + final int actualMaximum; + if (randomBoolean()) { + actualMaximum = staticMaximum; + } else { + actualMaximum = between(2, 15); + clusterSettings.applySettings(Settings.builder().put(clusterService.state().metaData().persistentSettings()) + .put(MAXIMUM_VOTING_CONFIG_EXCLUSIONS_SETTING.getKey(), actualMaximum).build()); + } + + for (int i = 2; i < actualMaximum; i++) { + coordinationMetaDataBuilder.addVotingConfigExclusion( + new VotingConfigExclusion(randomAlphaOfLength(10), randomAlphaOfLength(10))); + } final int existingCount, newCount; if (randomBoolean()) { coordinationMetaDataBuilder.addVotingConfigExclusion(otherNode1Exclusion); - existingCount = 2; + existingCount = actualMaximum; newCount = 1; } else { - existingCount = 1; + existingCount = actualMaximum - 1; newCount = 2; } @@ -346,7 +371,7 @@ public class TransportAddVotingConfigExclusionsActionTests extends ESTestCase { assertThat(rootCause, instanceOf(IllegalArgumentException.class)); assertThat(rootCause.getMessage(), equalTo("add voting config exclusions request for [other*] would add [" + newCount + "] exclusions to the existing [" + existingCount + - "] which would exceed the maximum of [2] set by [cluster.max_voting_config_exclusions]")); + "] which would exceed the maximum of [" + actualMaximum + "] set by [cluster.max_voting_config_exclusions]")); } public void testTimesOut() throws InterruptedException {