Skip to content

Commit

Permalink
Deprecate updating replica count with auto-expand
Browse files Browse the repository at this point in the history
Today if you update `index.number_of_replicas` on indices that have
`index.auto_expand_replicas` set then the update is accepted but
silently ignored. Lenience like this is surprising. This commit
deprecates this behaviour so that it can be removed in a future release.

Relates elastic#27835
  • Loading branch information
DaveCTurner committed May 19, 2021
1 parent 4b2c3ab commit 89670b9
Show file tree
Hide file tree
Showing 4 changed files with 172 additions and 2 deletions.
Expand Up @@ -32,7 +32,7 @@ public final class AutoExpandReplicas {
// the value we recognize in the "max" position to mean all the nodes
private static final String ALL_NODES_VALUE = "all";

private static final AutoExpandReplicas FALSE_INSTANCE = new AutoExpandReplicas(0, 0, false);
public static final AutoExpandReplicas FALSE_INSTANCE = new AutoExpandReplicas(0, 0, false);

public static final Setting<AutoExpandReplicas> SETTING = new Setting<>(IndexMetadata.SETTING_AUTO_EXPAND_REPLICAS, "false",
AutoExpandReplicas::parse, Property.Dynamic, Property.IndexScope);
Expand Down
Expand Up @@ -23,6 +23,8 @@
import org.elasticsearch.cluster.service.ClusterService;
import org.elasticsearch.common.Priority;
import org.elasticsearch.common.inject.Inject;
import org.elasticsearch.common.logging.DeprecationCategory;
import org.elasticsearch.common.logging.DeprecationLogger;
import org.elasticsearch.common.regex.Regex;
import org.elasticsearch.common.settings.IndexScopedSettings;
import org.elasticsearch.common.settings.Setting;
Expand All @@ -38,6 +40,7 @@
import java.util.HashSet;
import java.util.Locale;
import java.util.Set;
import java.util.stream.Stream;

import static org.elasticsearch.action.support.ContextPreservingActionListener.wrapPreservingContext;
import static org.elasticsearch.index.IndexSettings.same;
Expand All @@ -46,7 +49,9 @@
* Service responsible for submitting update index settings requests
*/
public class MetadataUpdateSettingsService {

private static final Logger logger = LogManager.getLogger(MetadataUpdateSettingsService.class);
private static final DeprecationLogger deprecationLogger = DeprecationLogger.getLogger(MetadataUpdateSettingsService.class);

private final ClusterService clusterService;

Expand Down Expand Up @@ -130,6 +135,31 @@ public ClusterState execute(ClusterState currentState) {
}

if (IndexMetadata.INDEX_NUMBER_OF_REPLICAS_SETTING.exists(openSettings)) {

final boolean updatingReplicasHasEffect;
if (IndexMetadata.INDEX_AUTO_EXPAND_REPLICAS_SETTING.exists(openSettings)) {
// we are setting auto-expand replicas on all these indices, so setting the number of replicas is meaningful iff
// it's being disabled
updatingReplicasHasEffect
= IndexMetadata.INDEX_AUTO_EXPAND_REPLICAS_SETTING.get(openSettings) == AutoExpandReplicas.FALSE_INSTANCE;
} else {
// we are leaving the auto-expand replicas config alone on these indices, so setting the number of replicas is
// meaningful iff there are some indices that are not using auto-expand replicas.
updatingReplicasHasEffect = Stream.concat(openIndices.stream(), closeIndices.stream()).anyMatch(index ->
IndexMetadata.INDEX_AUTO_EXPAND_REPLICAS_SETTING.get(metadataBuilder.getSafe(index).getSettings())
== AutoExpandReplicas.FALSE_INSTANCE);
}
if (updatingReplicasHasEffect == false) {
deprecationLogger.deprecate(
DeprecationCategory.INDICES,
"ignored_number_of_replicas",
"setting [{}] on indices using [{}] has no effect so it is deprecated and will be forbidden in a future " +
"version",
IndexMetadata.SETTING_NUMBER_OF_REPLICAS,
IndexMetadata.SETTING_AUTO_EXPAND_REPLICAS
);
}

final int updatedNumberOfReplicas = IndexMetadata.INDEX_NUMBER_OF_REPLICAS_SETTING.get(openSettings);
if (preserveExisting == false) {
// Verify that this won't take us over the cluster shard limit.
Expand Down
Expand Up @@ -756,7 +756,6 @@ public synchronized void updateMetadata(final IndexMetadata currentIndexMetadata
if (currentSettingsVersion == newSettingsVersion) {
assert updateIndexSettings == false;
} else {
assert updateIndexSettings;
assert currentSettingsVersion < newSettingsVersion :
"expected current settings version [" + currentSettingsVersion + "] "
+ "to be less than new settings version [" + newSettingsVersion + "]";
Expand Down
@@ -0,0 +1,141 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0 and the Server Side Public License, v 1; you may not use this file except
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/

package org.elasticsearch.cluster.metadata;

import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import org.elasticsearch.action.admin.indices.create.CreateIndexRequest;
import org.elasticsearch.action.admin.indices.settings.put.UpdateSettingsRequest;
import org.elasticsearch.cluster.ClusterState;
import org.elasticsearch.common.logging.DeprecationLogger;
import org.elasticsearch.common.logging.Loggers;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.xcontent.NamedXContentRegistry;
import org.elasticsearch.indices.cluster.ClusterStateChanges;
import org.elasticsearch.test.ESTestCase;
import org.elasticsearch.test.MockLogAppender;
import org.elasticsearch.threadpool.TestThreadPool;
import org.elasticsearch.threadpool.ThreadPool;
import org.junit.AfterClass;
import org.junit.BeforeClass;

import java.util.concurrent.TimeUnit;

public class NumberOfReplicasUpdateTests extends ESTestCase {

private static ThreadPool threadPool;
private static ClusterStateChanges clusterStateChanges;
private static ClusterState clusterState;
private static Logger deprecationLogger;

@BeforeClass
public static void setup() {
threadPool = new TestThreadPool("test");
clusterStateChanges = new ClusterStateChanges(NamedXContentRegistry.EMPTY, threadPool);

final Settings.Builder regularSettings = Settings.builder();
if (randomBoolean()) {
regularSettings.put(IndexMetadata.SETTING_AUTO_EXPAND_REPLICAS, "false");
}

final Settings.Builder autoExpandSettings = Settings.builder();
autoExpandSettings.put(IndexMetadata.SETTING_AUTO_EXPAND_REPLICAS, "0-all");

ClusterState clusterState = ClusterState.EMPTY_STATE;
clusterState = clusterStateChanges.createIndex(clusterState, new CreateIndexRequest("regular", regularSettings.build()));
clusterState = clusterStateChanges.createIndex(clusterState, new CreateIndexRequest("auto-expands", autoExpandSettings.build()));
NumberOfReplicasUpdateTests.clusterState = clusterState;

deprecationLogger = LogManager.getLogger("org.elasticsearch.deprecation.cluster.metadata.MetadataUpdateSettingsService");
}

@AfterClass
public static void terminateThreadpool() {
ThreadPool.terminate(threadPool, 10, TimeUnit.SECONDS);
threadPool = null;
clusterStateChanges = null;
clusterState = null;
deprecationLogger = null;
}

public void testDeprecatedIfAutoExpandReplicasSet() {
assertDeprecated(numberOfReplicasUpdate(), "auto-expands");
}

public void testNotDeprecatedIfAutoExpandReplicasUnset() {
assertNotDeprecated(numberOfReplicasUpdate(), "regular");
}

public void testNotDeprecatedIfAutoExpandReplicasUnsetOnSomeIndices() {
assertNotDeprecated(numberOfReplicasUpdate(), "regular", "auto-expands");
}

public void testNotDeprecatedIfDisablingAutoExpandReplicas() {
assertNotDeprecated(numberOfReplicasUpdate().put(IndexMetadata.SETTING_AUTO_EXPAND_REPLICAS, "false"), randomIndices());
}

public void testNotDeprecatedIfClearingAutoExpandReplicas() {
assertNotDeprecated(numberOfReplicasUpdate().putNull(IndexMetadata.SETTING_AUTO_EXPAND_REPLICAS), randomIndices());
}

public void testDeprecatedIfSettingAutoExpandReplicas() {
assertDeprecated(numberOfReplicasUpdate().put(IndexMetadata.SETTING_AUTO_EXPAND_REPLICAS, "0-all"), randomIndices());
}

private static Settings.Builder numberOfReplicasUpdate() {
return Settings.builder().put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, Integer.toString(between(0, 5)));
}

private static String[] randomIndices() {
return randomSubsetOf(between(1, 2), "regular", "auto-expands").toArray(new String[0]);
}

private static void assertDeprecated(Settings.Builder settings, String... indices) {
runTest(true, settings, indices);
}

private static void assertNotDeprecated(Settings.Builder settings, String... indices) {
runTest(false, settings, indices);
}

private static void runTest(boolean expectDeprecated, Settings.Builder settings, String[] indices) {
try {
final MockLogAppender appender = new MockLogAppender();
try {
appender.start();
Loggers.addAppender(deprecationLogger, appender);

if (expectDeprecated) {
appender.addExpectation(new MockLogAppender.SeenEventExpectation(
"deprecation",
deprecationLogger.getName(),
DeprecationLogger.DEPRECATION,
"setting [index.number_of_replicas] on indices using [index.auto_expand_replicas] has no effect so it is " +
"deprecated and will be forbidden in a future version"));
} else {
appender.addExpectation(new MockLogAppender.UnseenEventExpectation(
"deprecation",
deprecationLogger.getName(),
DeprecationLogger.DEPRECATION,
"*"));
}

clusterStateChanges.updateSettings(clusterState, new UpdateSettingsRequest(settings.build(), indices));

appender.assertAllExpectationsMatched();
} finally {
Loggers.removeAppender(deprecationLogger, appender);
appender.stop();
}
} catch (IllegalAccessException e) {
throw new AssertionError("unexpected", e);
}
}

}

0 comments on commit 89670b9

Please sign in to comment.