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
This commit is contained in:
David Turner 2020-03-23 08:36:09 +00:00
parent efd1838206
commit 0fb31d9e7a
2 changed files with 58 additions and 19 deletions

View File

@ -39,8 +39,10 @@ import org.elasticsearch.cluster.service.ClusterService;
import org.elasticsearch.common.Priority; import org.elasticsearch.common.Priority;
import org.elasticsearch.common.inject.Inject; import org.elasticsearch.common.inject.Inject;
import org.elasticsearch.common.io.stream.StreamInput; 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;
import org.elasticsearch.common.settings.Setting.Property; import org.elasticsearch.common.settings.Setting.Property;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.unit.TimeValue; import org.elasticsearch.common.unit.TimeValue;
import org.elasticsearch.threadpool.ThreadPool; import org.elasticsearch.threadpool.ThreadPool;
import org.elasticsearch.threadpool.ThreadPool.Names; import org.elasticsearch.threadpool.ThreadPool.Names;
@ -59,11 +61,21 @@ public class TransportAddVotingConfigExclusionsAction extends TransportMasterNod
public static final Setting<Integer> MAXIMUM_VOTING_CONFIG_EXCLUSIONS_SETTING public static final Setting<Integer> MAXIMUM_VOTING_CONFIG_EXCLUSIONS_SETTING
= Setting.intSetting("cluster.max_voting_config_exclusions", 10, 1, Property.Dynamic, Property.NodeScope); = Setting.intSetting("cluster.max_voting_config_exclusions", 10, 1, Property.Dynamic, Property.NodeScope);
private volatile int maxVotingConfigExclusions;
@Inject @Inject
public TransportAddVotingConfigExclusionsAction(TransportService transportService, ClusterService clusterService, ThreadPool threadPool, public TransportAddVotingConfigExclusionsAction(Settings settings, ClusterSettings clusterSettings, TransportService transportService,
ActionFilters actionFilters, IndexNameExpressionResolver indexNameExpressionResolver) { ClusterService clusterService, ThreadPool threadPool, ActionFilters actionFilters,
IndexNameExpressionResolver indexNameExpressionResolver) {
super(AddVotingConfigExclusionsAction.NAME, transportService, clusterService, threadPool, actionFilters, super(AddVotingConfigExclusionsAction.NAME, transportService, clusterService, threadPool, actionFilters,
AddVotingConfigExclusionsRequest::new, indexNameExpressionResolver); 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 @Override
@ -80,7 +92,8 @@ public class TransportAddVotingConfigExclusionsAction extends TransportMasterNod
protected void masterOperation(AddVotingConfigExclusionsRequest request, ClusterState state, protected void masterOperation(AddVotingConfigExclusionsRequest request, ClusterState state,
ActionListener<AddVotingConfigExclusionsResponse> listener) throws Exception { ActionListener<AddVotingConfigExclusionsResponse> 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) { clusterService.submitStateUpdateTask("add-voting-config-exclusions", new ClusterStateUpdateTask(Priority.URGENT) {
@ -89,14 +102,14 @@ public class TransportAddVotingConfigExclusionsAction extends TransportMasterNod
@Override @Override
public ClusterState execute(ClusterState currentState) { public ClusterState execute(ClusterState currentState) {
assert resolvedExclusions == null : resolvedExclusions; 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()); final CoordinationMetaData.Builder builder = CoordinationMetaData.builder(currentState.coordinationMetaData());
resolvedExclusions.forEach(builder::addVotingConfigExclusion); resolvedExclusions.forEach(builder::addVotingConfigExclusion);
final MetaData newMetaData = MetaData.builder(currentState.metaData()).coordinationMetaData(builder.build()).build(); final MetaData newMetaData = MetaData.builder(currentState.metaData()).coordinationMetaData(builder.build()).build();
final ClusterState newState = ClusterState.builder(currentState).metaData(newMetaData).build(); final ClusterState newState = ClusterState.builder(currentState).metaData(newMetaData).build();
assert newState.getVotingConfigExclusions().size() <= MAXIMUM_VOTING_CONFIG_EXCLUSIONS_SETTING.get( assert newState.getVotingConfigExclusions().size() <= finalMaxVotingConfigExclusions;
currentState.metaData().settings());
return newState; return newState;
} }
@ -148,9 +161,10 @@ public class TransportAddVotingConfigExclusionsAction extends TransportMasterNod
} }
private static Set<VotingConfigExclusion> resolveVotingConfigExclusionsAndCheckMaximum(AddVotingConfigExclusionsRequest request, private static Set<VotingConfigExclusion> resolveVotingConfigExclusionsAndCheckMaximum(AddVotingConfigExclusionsRequest request,
ClusterState state) { ClusterState state,
return request.resolveVotingConfigExclusionsAndCheckMaximum(state, int maxVotingConfigExclusions) {
MAXIMUM_VOTING_CONFIG_EXCLUSIONS_SETTING.get(state.metaData().settings()), MAXIMUM_VOTING_CONFIG_EXCLUSIONS_SETTING.getKey()); return request.resolveVotingConfigExclusionsAndCheckMaximum(state, maxVotingConfigExclusions,
MAXIMUM_VOTING_CONFIG_EXCLUSIONS_SETTING.getKey());
} }
@Override @Override

View File

@ -37,6 +37,7 @@ import org.elasticsearch.cluster.node.DiscoveryNodeRole;
import org.elasticsearch.cluster.node.DiscoveryNodes.Builder; import org.elasticsearch.cluster.node.DiscoveryNodes.Builder;
import org.elasticsearch.cluster.service.ClusterService; import org.elasticsearch.cluster.service.ClusterService;
import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamInput;
import org.elasticsearch.common.settings.ClusterSettings;
import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.unit.TimeValue; import org.elasticsearch.common.unit.TimeValue;
import org.elasticsearch.test.ESTestCase; import org.elasticsearch.test.ESTestCase;
@ -81,6 +82,8 @@ public class TransportAddVotingConfigExclusionsActionTests extends ESTestCase {
private TransportService transportService; private TransportService transportService;
private ClusterStateObserver clusterStateObserver; private ClusterStateObserver clusterStateObserver;
private ClusterSettings clusterSettings;
private int staticMaximum;
@BeforeClass @BeforeClass
public static void createThreadPoolAndClusterService() { public static void createThreadPoolAndClusterService() {
@ -117,8 +120,18 @@ public class TransportAddVotingConfigExclusionsActionTests extends ESTestCase {
transportService = transport.createTransportService(Settings.EMPTY, threadPool, transportService = transport.createTransportService(Settings.EMPTY, threadPool,
TransportService.NOOP_TRANSPORT_INTERCEPTOR, boundTransportAddress -> localNode, null, emptySet()); TransportService.NOOP_TRANSPORT_INTERCEPTOR, boundTransportAddress -> localNode, null, emptySet());
new TransportAddVotingConfigExclusionsAction(transportService, clusterService, threadPool, new ActionFilters(emptySet()), final Settings.Builder nodeSettingsBuilder = Settings.builder();
new IndexNameExpressionResolver()); // registers action 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.start();
transportService.acceptIncomingRequests(); transportService.acceptIncomingRequests();
@ -308,20 +321,32 @@ public class TransportAddVotingConfigExclusionsActionTests extends ESTestCase {
} }
public void testReturnsErrorIfMaximumExclusionCountExceeded() throws InterruptedException { public void testReturnsErrorIfMaximumExclusionCountExceeded() throws InterruptedException {
final MetaData.Builder metaDataBuilder = MetaData.builder(clusterService.state().metaData()).persistentSettings( final MetaData.Builder metaDataBuilder = MetaData.builder(clusterService.state().metaData());
Settings.builder().put(clusterService.state().metaData().persistentSettings())
.put(MAXIMUM_VOTING_CONFIG_EXCLUSIONS_SETTING.getKey(), 2).build());
CoordinationMetaData.Builder coordinationMetaDataBuilder = CoordinationMetaData.Builder coordinationMetaDataBuilder =
CoordinationMetaData.builder(clusterService.state().coordinationMetaData()) CoordinationMetaData.builder(clusterService.state().coordinationMetaData())
.addVotingConfigExclusion(localNodeExclusion); .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; final int existingCount, newCount;
if (randomBoolean()) { if (randomBoolean()) {
coordinationMetaDataBuilder.addVotingConfigExclusion(otherNode1Exclusion); coordinationMetaDataBuilder.addVotingConfigExclusion(otherNode1Exclusion);
existingCount = 2; existingCount = actualMaximum;
newCount = 1; newCount = 1;
} else { } else {
existingCount = 1; existingCount = actualMaximum - 1;
newCount = 2; newCount = 2;
} }
@ -346,7 +371,7 @@ public class TransportAddVotingConfigExclusionsActionTests extends ESTestCase {
assertThat(rootCause, instanceOf(IllegalArgumentException.class)); assertThat(rootCause, instanceOf(IllegalArgumentException.class));
assertThat(rootCause.getMessage(), equalTo("add voting config exclusions request for [other*] would add [" + newCount + assertThat(rootCause.getMessage(), equalTo("add voting config exclusions request for [other*] would add [" + newCount +
"] exclusions to the existing [" + existingCount + "] 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 { public void testTimesOut() throws InterruptedException {