Use settings infrastructure for shards and replicas (#56801)

We get the number of shards and replicas with our bare hands in index
metadata, rather than letting the settings infrastructure do the work
for us. This commit switches to using the settings infrastructure.
This commit is contained in:
Jason Tedor 2020-05-15 15:44:03 -04:00
parent a3e845cbad
commit da833d6cd3
No known key found for this signature in database
GPG Key ID: FA89F05560F16BC5
5 changed files with 95 additions and 32 deletions

View File

@ -1173,29 +1173,26 @@ public class IndexMetadata implements Diffable<IndexMetadata>, ToXContentFragmen
}
}
Integer maybeNumberOfShards = settings.getAsInt(SETTING_NUMBER_OF_SHARDS, null);
if (maybeNumberOfShards == null) {
throw new IllegalArgumentException("must specify numberOfShards for index [" + index + "]");
}
int numberOfShards = maybeNumberOfShards;
if (numberOfShards <= 0) {
throw new IllegalArgumentException("must specify positive number of shards for index [" + index + "]");
/*
* We expect that the metadata has been properly built to set the number of shards and the number of replicas, and do not rely
* on the default values here. Those must have been set upstream.
*/
if (INDEX_NUMBER_OF_SHARDS_SETTING.exists(settings) == false) {
throw new IllegalArgumentException("must specify number of shards for index [" + index + "]");
}
final int numberOfShards = INDEX_NUMBER_OF_SHARDS_SETTING.get(settings);
Integer maybeNumberOfReplicas = settings.getAsInt(SETTING_NUMBER_OF_REPLICAS, null);
if (maybeNumberOfReplicas == null) {
throw new IllegalArgumentException("must specify numberOfReplicas for index [" + index + "]");
}
int numberOfReplicas = maybeNumberOfReplicas;
if (numberOfReplicas < 0) {
throw new IllegalArgumentException("must specify non-negative number of replicas for index [" + index + "]");
if (INDEX_NUMBER_OF_REPLICAS_SETTING.exists(settings) == false) {
throw new IllegalArgumentException("must specify number of replicas for index [" + index + "]");
}
final int numberOfReplicas = INDEX_NUMBER_OF_REPLICAS_SETTING.get(settings);
int routingPartitionSize = INDEX_ROUTING_PARTITION_SIZE_SETTING.get(settings);
if (routingPartitionSize != 1 && routingPartitionSize >= getRoutingNumShards()) {
throw new IllegalArgumentException("routing partition size [" + routingPartitionSize + "] should be a positive number"
+ " less than the number of shards [" + getRoutingNumShards() + "] for [" + index + "]");
}
// fill missing slots in inSyncAllocationIds with empty set if needed and make all entries immutable
ImmutableOpenIntMap.Builder<Set<String>> filledInSyncAllocationIds = ImmutableOpenIntMap.builder();
for (int i = 0; i < numberOfShards; i++) {

View File

@ -101,6 +101,8 @@ import java.util.stream.Collectors;
import java.util.stream.IntStream;
import static java.util.stream.Collectors.toList;
import static org.elasticsearch.cluster.metadata.IndexMetadata.INDEX_NUMBER_OF_REPLICAS_SETTING;
import static org.elasticsearch.cluster.metadata.IndexMetadata.INDEX_NUMBER_OF_SHARDS_SETTING;
import static org.elasticsearch.cluster.metadata.IndexMetadata.SETTING_AUTO_EXPAND_REPLICAS;
import static org.elasticsearch.cluster.metadata.IndexMetadata.SETTING_CREATION_DATE;
import static org.elasticsearch.cluster.metadata.IndexMetadata.SETTING_INDEX_UUID;
@ -720,12 +722,17 @@ public class MetadataCreateIndexService {
final Version createdVersion = Version.min(Version.CURRENT, nodes.getSmallestNonClientNodeVersion());
indexSettingsBuilder.put(IndexMetadata.SETTING_INDEX_VERSION_CREATED.getKey(), createdVersion);
}
if (indexSettingsBuilder.get(SETTING_NUMBER_OF_SHARDS) == null) {
final int numberOfShards = getNumberOfShards(indexSettingsBuilder);
indexSettingsBuilder.put(SETTING_NUMBER_OF_SHARDS, settings.getAsInt(SETTING_NUMBER_OF_SHARDS, numberOfShards));
if (INDEX_NUMBER_OF_SHARDS_SETTING.exists(indexSettingsBuilder) == false) {
final int numberOfShards;
if (INDEX_NUMBER_OF_SHARDS_SETTING.exists(settings)) {
numberOfShards = INDEX_NUMBER_OF_SHARDS_SETTING.get(settings);
} else {
numberOfShards = getNumberOfShards(indexSettingsBuilder);
}
if (indexSettingsBuilder.get(SETTING_NUMBER_OF_REPLICAS) == null) {
indexSettingsBuilder.put(SETTING_NUMBER_OF_REPLICAS, settings.getAsInt(SETTING_NUMBER_OF_REPLICAS, 1));
indexSettingsBuilder.put(SETTING_NUMBER_OF_SHARDS, numberOfShards);
}
if (INDEX_NUMBER_OF_REPLICAS_SETTING.exists(indexSettingsBuilder) == false) {
indexSettingsBuilder.put(SETTING_NUMBER_OF_REPLICAS, INDEX_NUMBER_OF_REPLICAS_SETTING.get(settings));
}
if (settings.get(SETTING_AUTO_EXPAND_REPLICAS) != null && indexSettingsBuilder.get(SETTING_AUTO_EXPAND_REPLICAS) == null) {
indexSettingsBuilder.put(SETTING_AUTO_EXPAND_REPLICAS, settings.get(SETTING_AUTO_EXPAND_REPLICAS));
@ -784,7 +791,7 @@ public class MetadataCreateIndexService {
* it will return the value configured for that index.
*/
static int getIndexNumberOfRoutingShards(Settings indexSettings, @Nullable IndexMetadata sourceMetadata) {
final int numTargetShards = IndexMetadata.INDEX_NUMBER_OF_SHARDS_SETTING.get(indexSettings);
final int numTargetShards = INDEX_NUMBER_OF_SHARDS_SETTING.get(indexSettings);
final Version indexVersionCreated = IndexMetadata.SETTING_INDEX_VERSION_CREATED.get(indexSettings);
final int routingNumShards;
if (sourceMetadata == null || sourceMetadata.getNumberOfShards() == 1) {
@ -1010,7 +1017,7 @@ public class MetadataCreateIndexService {
* @throws ValidationException if creating this index would put the cluster over the cluster shard limit
*/
public static void checkShardLimit(final Settings settings, final ClusterState clusterState) {
final int numberOfShards = IndexMetadata.INDEX_NUMBER_OF_SHARDS_SETTING.get(settings);
final int numberOfShards = INDEX_NUMBER_OF_SHARDS_SETTING.get(settings);
final int numberOfReplicas = IndexMetadata.INDEX_NUMBER_OF_REPLICAS_SETTING.get(settings);
final int shardsToCreate = numberOfShards * (1 + numberOfReplicas);
@ -1076,8 +1083,8 @@ public class MetadataCreateIndexService {
Set<String> targetIndexMappingsTypes, String targetIndexName,
Settings targetIndexSettings) {
IndexMetadata sourceMetadata = validateResize(state, sourceIndex, targetIndexMappingsTypes, targetIndexName, targetIndexSettings);
assert IndexMetadata.INDEX_NUMBER_OF_SHARDS_SETTING.exists(targetIndexSettings);
IndexMetadata.selectShrinkShards(0, sourceMetadata, IndexMetadata.INDEX_NUMBER_OF_SHARDS_SETTING.get(targetIndexSettings));
assert INDEX_NUMBER_OF_SHARDS_SETTING.exists(targetIndexSettings);
IndexMetadata.selectShrinkShards(0, sourceMetadata, INDEX_NUMBER_OF_SHARDS_SETTING.get(targetIndexSettings));
if (sourceMetadata.getNumberOfShards() == 1) {
throw new IllegalArgumentException("can't shrink an index with only one shard");
@ -1116,14 +1123,13 @@ public class MetadataCreateIndexService {
// since in 5.x we don't have a setting to artificially set the number of routing shards
throw new IllegalStateException("source index created version is too old to apply a split operation");
}
}
static void validateCloneIndex(ClusterState state, String sourceIndex,
Set<String> targetIndexMappingsTypes, String targetIndexName,
Settings targetIndexSettings) {
IndexMetadata sourceMetadata = validateResize(state, sourceIndex, targetIndexMappingsTypes, targetIndexName, targetIndexSettings);
IndexMetadata.selectCloneShard(0, sourceMetadata, IndexMetadata.INDEX_NUMBER_OF_SHARDS_SETTING.get(targetIndexSettings));
IndexMetadata.selectCloneShard(0, sourceMetadata, INDEX_NUMBER_OF_SHARDS_SETTING.get(targetIndexSettings));
}
static IndexMetadata validateResize(ClusterState state, String sourceIndex,
@ -1147,11 +1153,11 @@ public class MetadataCreateIndexService {
", all mappings are copied from the source index");
}
if (IndexMetadata.INDEX_NUMBER_OF_SHARDS_SETTING.exists(targetIndexSettings)) {
if (INDEX_NUMBER_OF_SHARDS_SETTING.exists(targetIndexSettings)) {
// this method applies all necessary checks ie. if the target shards are less than the source shards
// of if the source shards are divisible by the number of target shards
IndexMetadata.getRoutingFactor(sourceMetadata.getNumberOfShards(),
IndexMetadata.INDEX_NUMBER_OF_SHARDS_SETTING.get(targetIndexSettings));
INDEX_NUMBER_OF_SHARDS_SETTING.get(targetIndexSettings));
}
return sourceMetadata;
}

View File

@ -201,8 +201,10 @@ public class MetadataUpdateSettingsService {
* including updating it to null, indicating that they want to use the default value. In this case, we again
* have to provide an explicit value for the setting to the default (one).
*/
if (indexSettings.get(IndexMetadata.SETTING_NUMBER_OF_REPLICAS) == null) {
indexSettings.put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, 1);
if (IndexMetadata.INDEX_NUMBER_OF_REPLICAS_SETTING.exists(indexSettings) == false) {
indexSettings.put(
IndexMetadata.SETTING_NUMBER_OF_REPLICAS,
IndexMetadata.INDEX_NUMBER_OF_REPLICAS_SETTING.get(Settings.EMPTY));
}
Settings finalSettings = indexSettings.build();
indexScopedSettings.validate(
@ -228,8 +230,10 @@ public class MetadataUpdateSettingsService {
* including updating it to null, indicating that they want to use the default value. In this case, we again
* have to provide an explicit value for the setting to the default (one).
*/
if (indexSettings.get(IndexMetadata.SETTING_NUMBER_OF_REPLICAS) == null) {
indexSettings.put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, 1);
if (IndexMetadata.INDEX_NUMBER_OF_REPLICAS_SETTING.exists(indexSettings) == false) {
indexSettings.put(
IndexMetadata.SETTING_NUMBER_OF_REPLICAS,
IndexMetadata.INDEX_NUMBER_OF_REPLICAS_SETTING.get(Settings.EMPTY));
}
Settings finalSettings = indexSettings.build();
indexScopedSettings.validate(

View File

@ -406,7 +406,15 @@ public class Setting<T> implements ToXContentObject {
* @return true if the setting is present in the given settings instance, otherwise false
*/
public boolean exists(final Settings settings) {
return settings.keySet().contains(getKey());
return exists(settings.keySet());
}
public boolean exists(final Settings.Builder builder) {
return exists(builder.keys());
}
private boolean exists(final Set<String> keys) {
return keys.contains(getKey());
}
/**

View File

@ -52,6 +52,8 @@ import java.util.HashMap;
import java.util.Map;
import java.util.Set;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.is;
public class IndexMetadataTests extends ESTestCase {
@ -321,4 +323,50 @@ public class IndexMetadataTests extends ESTestCase {
assertNotNull(meta.mappingOrDefault());
assertEquals("type", meta.mappingOrDefault().type());
}
public void testMissingNumberOfShards() {
final IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> IndexMetadata.builder("test").build());
assertThat(e.getMessage(), containsString("must specify number of shards for index [test]"));
}
public void testNumberOfShardsIsNotZero() {
runTestNumberOfShardsIsPositive(0);
}
public void testNumberOfShardsIsNotNegative() {
runTestNumberOfShardsIsPositive(-randomIntBetween(1, Integer.MAX_VALUE));
}
private void runTestNumberOfShardsIsPositive(final int numberOfShards) {
final Settings settings =
Settings.builder().put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, numberOfShards).build();
final IllegalArgumentException e =
expectThrows(IllegalArgumentException.class, () -> IndexMetadata.builder("test").settings(settings).build());
assertThat(
e.getMessage(),
equalTo("Failed to parse value [" + numberOfShards + "] for setting [index.number_of_shards] must be >= 1"));
}
public void testMissingNumberOfReplicas() {
final Settings settings =
Settings.builder().put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, randomIntBetween(1, 8)).build();
final IllegalArgumentException e =
expectThrows(IllegalArgumentException.class, () -> IndexMetadata.builder("test").settings(settings).build());
assertThat(e.getMessage(), containsString("must specify number of replicas for index [test]"));
}
public void testNumberOfReplicasIsNonNegative() {
final int numberOfReplicas = -randomIntBetween(1, Integer.MAX_VALUE);
final Settings settings = Settings.builder()
.put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, randomIntBetween(1, 8))
.put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, numberOfReplicas)
.build();
final IllegalArgumentException e =
expectThrows(IllegalArgumentException.class, () -> IndexMetadata.builder("test").settings(settings).build());
assertThat(
e.getMessage(),
equalTo(
"Failed to parse value [" + numberOfReplicas + "] for setting [index.number_of_replicas] must be >= 0"));
}
}