Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use settings infrastructure for shards and replicas #56801

Merged
merged 6 commits into from
May 15, 2020
Merged

Use settings infrastructure for shards and replicas #56801

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
7841641
Use settings infrastructure for shards and replicas
jasontedor May 15, 2020
c7bbe78
Iteration
jasontedor May 15, 2020
812ce7f
Test iteration
jasontedor May 15, 2020
cd830be
Remove import
jasontedor May 15, 2020
82d4dd7
Remove the need to build
jasontedor May 15, 2020
8f451f8
Merge branch 'master' into index-metadata-use-settings-infrastructure
elasticmachine May 15, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 11 additions & 14 deletions server/src/main/java/org/elasticsearch/cluster/metadata/IndexMetadata.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1080,29 +1080,26 @@ public IndexMetadata build() {
ImmutableOpenMap.Builder<String, AliasMetadata> tmpAliases = aliases;
Settings tmpSettings = settings;

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++) {
Expand Down
26 changes: 14 additions & 12 deletions server/src/main/java/org/elasticsearch/cluster/metadata/MetadataCreateIndexService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,8 @@
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;
Expand Down Expand Up @@ -762,11 +764,11 @@ static Settings aggregateIndexSettings(ClusterState currentState, CreateIndexClu
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) {
indexSettingsBuilder.put(SETTING_NUMBER_OF_SHARDS, settings.getAsInt(SETTING_NUMBER_OF_SHARDS, 1));
if (INDEX_NUMBER_OF_SHARDS_SETTING.exists(indexSettingsBuilder.build()) == false) {
indexSettingsBuilder.put(SETTING_NUMBER_OF_SHARDS, INDEX_NUMBER_OF_SHARDS_SETTING.get(settings));
}
if (indexSettingsBuilder.get(SETTING_NUMBER_OF_REPLICAS) == null) {
indexSettingsBuilder.put(SETTING_NUMBER_OF_REPLICAS, settings.getAsInt(SETTING_NUMBER_OF_REPLICAS, 1));
if (INDEX_NUMBER_OF_REPLICAS_SETTING.exists(indexSettingsBuilder.build()) == 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));
Expand Down Expand Up @@ -811,7 +813,7 @@ static Settings aggregateIndexSettings(ClusterState currentState, CreateIndexClu
* 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) {
Expand Down Expand Up @@ -1034,7 +1036,7 @@ public void validateIndexSettings(String indexName, final Settings settings, fin
* @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);

Expand Down Expand Up @@ -1100,8 +1102,8 @@ static List<String> validateShrinkIndex(ClusterState state, String sourceIndex,
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");
Expand Down Expand Up @@ -1133,14 +1135,14 @@ static void validateSplitIndex(ClusterState state, String sourceIndex,
Set<String> targetIndexMappingsTypes, String targetIndexName,
Settings targetIndexSettings) {
IndexMetadata sourceMetadata = validateResize(state, sourceIndex, targetIndexMappingsTypes, targetIndexName, targetIndexSettings);
IndexMetadata.selectSplitShard(0, sourceMetadata, IndexMetadata.INDEX_NUMBER_OF_SHARDS_SETTING.get(targetIndexSettings));
IndexMetadata.selectSplitShard(0, sourceMetadata, INDEX_NUMBER_OF_SHARDS_SETTING.get(targetIndexSettings));
}

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,
Expand All @@ -1163,11 +1165,11 @@ static IndexMetadata validateResize(ClusterState state, String sourceIndex,
", 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;
}
Expand Down
12 changes: 8 additions & 4 deletions server/src/main/java/org/elasticsearch/cluster/metadata/MetadataUpdateSettingsService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -201,8 +201,10 @@ public ClusterState execute(ClusterState currentState) {
* 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.build()) == false) {
indexSettings.put(
IndexMetadata.SETTING_NUMBER_OF_REPLICAS,
IndexMetadata.INDEX_NUMBER_OF_REPLICAS_SETTING.get(Settings.EMPTY));
}
Settings finalSettings = indexSettings.build();
indexScopedSettings.validate(
Expand All @@ -228,8 +230,10 @@ public ClusterState execute(ClusterState currentState) {
* 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.build()) == false) {
indexSettings.put(
IndexMetadata.SETTING_NUMBER_OF_REPLICAS,
IndexMetadata.INDEX_NUMBER_OF_REPLICAS_SETTING.get(Settings.EMPTY));
}
Settings finalSettings = indexSettings.build();
indexScopedSettings.validate(
Expand Down
48 changes: 48 additions & 0 deletions server/src/test/java/org/elasticsearch/cluster/metadata/IndexMetadataTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,10 @@
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;
import static org.hamcrest.Matchers.matchesPattern;

public class IndexMetadataTests extends ESTestCase {

Expand Down Expand Up @@ -284,4 +287,49 @@ public void testNumberOfRoutingShards() {
assertEquals("the number of source shards [2] must be a factor of [3]", iae.getMessage());
}

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"));
}

}