Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -437,6 +437,13 @@ public ClusterState execute(ClusterState currentState) throws Exception {
indexScopedSettings);
}
final Settings actualIndexSettings = indexSettingsBuilder.build();

/*
* We can not check the shard limit until we have applied templates, otherwise we do not know the actual number of shards
* that will be used to create this index.
*/
checkShardLimit(actualIndexSettings, currentState);

tmpImdBuilder.settings(actualIndexSettings);

if (recoverFromIndex != null) {
Expand Down Expand Up @@ -587,6 +594,10 @@ public ClusterState execute(ClusterState currentState) throws Exception {
}
}

protected void checkShardLimit(final Settings settings, final ClusterState clusterState) {
MetaDataCreateIndexService.checkShardLimit(settings, clusterState);
}

@Override
public void onFailure(String source, Exception e) {
if (e instanceof ResourceAlreadyExistsException) {
Expand All @@ -607,9 +618,6 @@ public void validateIndexSettings(String indexName, final Settings settings, fin
final boolean forbidPrivateIndexSettings) throws IndexCreationException {
List<String> validationErrors = getIndexSettingsValidationErrors(settings, forbidPrivateIndexSettings);

Optional<String> shardAllocation = checkShardLimit(settings, clusterState);
shardAllocation.ifPresent(validationErrors::add);

if (validationErrors.isEmpty() == false) {
ValidationException validationException = new ValidationException();
validationException.addValidationErrors(validationErrors);
Expand All @@ -620,15 +628,21 @@ public void validateIndexSettings(String indexName, final Settings settings, fin
/**
* Checks whether an index can be created without going over the cluster shard limit.
*
* @param settings The settings of the index to be created.
* @param clusterState The current cluster state.
* @return If present, an error message to be used to reject index creation. If empty, a signal that this operation may be carried out.
* @param settings the settings of the index to be created
* @param clusterState the current cluster state
* @throws ValidationException if creating this index would put the cluster over the cluster shard limit
*/
static Optional<String> checkShardLimit(Settings settings, ClusterState clusterState) {
int shardsToCreate = IndexMetaData.INDEX_NUMBER_OF_SHARDS_SETTING.get(settings)
* (1 + IndexMetaData.INDEX_NUMBER_OF_REPLICAS_SETTING.get(settings));

return IndicesService.checkShardLimit(shardsToCreate, clusterState);
public static void checkShardLimit(final Settings settings, final ClusterState clusterState) {
final int numberOfShards = IndexMetaData.INDEX_NUMBER_OF_SHARDS_SETTING.get(settings);
final int numberOfReplicas = IndexMetaData.INDEX_NUMBER_OF_REPLICAS_SETTING.get(settings);
final int shardsToCreate = numberOfShards * (1 + numberOfReplicas);

final Optional<String> shardLimit = IndicesService.checkShardLimit(shardsToCreate, clusterState);
if (shardLimit.isPresent()) {
final ValidationException e = new ValidationException();
e.addValidationError(shardLimit.get());
throw e;
}
}

List<String> getIndexSettingsValidationErrors(final Settings settings, final boolean forbidPrivateIndexSettings) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -276,6 +276,7 @@ public ClusterState execute(ClusterState currentState) {
indexMdBuilder.settings(Settings.builder()
.put(snapshotIndexMetaData.getSettings())
.put(IndexMetaData.SETTING_INDEX_UUID, UUIDs.randomBase64UUID()));
MetaDataCreateIndexService.checkShardLimit(snapshotIndexMetaData.getSettings(), currentState);
if (!request.includeAliases() && !snapshotIndexMetaData.getAliases().isEmpty()) {
// Remove all aliases - they shouldn't be restored
indexMdBuilder.removeAllAliases();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -429,7 +429,14 @@ private ClusterState executeTask() throws Exception {
setupRequest();
final MetaDataCreateIndexService.IndexCreationTask task = new MetaDataCreateIndexService.IndexCreationTask(
logger, allocationService, request, listener, indicesService, aliasValidator, xContentRegistry, clusterStateSettings.build(),
validator, IndexScopedSettings.DEFAULT_SCOPED_SETTINGS);
validator, IndexScopedSettings.DEFAULT_SCOPED_SETTINGS) {

@Override
protected void checkShardLimit(final Settings settings, final ClusterState clusterState) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we add a short comment here explaining why this is here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I pushed f1ea945.

// we have to make this a no-op since we are not mocking enough for this method to be able to execute
}

};
return task.execute(state);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@
import org.elasticsearch.cluster.routing.allocation.decider.MaxRetryAllocationDecider;
import org.elasticsearch.cluster.shards.ClusterShardLimitIT;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.logging.DeprecationLogger;
import org.elasticsearch.common.ValidationException;
import org.elasticsearch.common.settings.IndexScopedSettings;
import org.elasticsearch.common.settings.Setting;
import org.elasticsearch.common.settings.Settings;
Expand All @@ -52,7 +52,7 @@
import java.util.Collections;
import java.util.Comparator;
import java.util.List;
import java.util.Optional;
import java.util.Locale;
import java.util.Set;
import java.util.function.Consumer;
import java.util.stream.Collectors;
Expand All @@ -64,8 +64,10 @@
import static org.elasticsearch.cluster.metadata.IndexMetaData.SETTING_VERSION_CREATED;
import static org.elasticsearch.cluster.shards.ClusterShardLimitIT.ShardCounts.forDataNodeCount;
import static org.elasticsearch.indices.IndicesServiceTests.createClusterForShardLimitTest;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.endsWith;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.hasToString;

public class MetaDataCreateIndexServiceTests extends ESTestCase {

Expand Down Expand Up @@ -466,14 +468,19 @@ public void testShardLimit() {
.put(SETTING_NUMBER_OF_REPLICAS, counts.getFailingIndexReplicas())
.build();

DeprecationLogger deprecationLogger = new DeprecationLogger(logger);
Optional<String> errorMessage = MetaDataCreateIndexService.checkShardLimit(indexSettings, state);
final ValidationException e = expectThrows(
ValidationException.class,
() -> MetaDataCreateIndexService.checkShardLimit(indexSettings, state));
int totalShards = counts.getFailingIndexShards() * (1 + counts.getFailingIndexReplicas());
int currentShards = counts.getFirstIndexShards() * (1 + counts.getFirstIndexReplicas());
int maxShards = counts.getShardsPerNode() * nodesInCluster;
assertTrue(errorMessage.isPresent());
assertEquals("this action would add [" + totalShards + "] total shards, but this cluster currently has [" + currentShards
+ "]/[" + maxShards + "] maximum shards open", errorMessage.get());
final String expectedMessage = String.format(
Locale.ROOT,
"this action would add [%d] total shards, but this cluster currently has [%d]/[%d] maximum shards open",
totalShards,
currentShards,
maxShards);
assertThat(e, hasToString(containsString(expectedMessage)));
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
import org.elasticsearch.snapshots.SnapshotState;
import org.elasticsearch.test.ESIntegTestCase;

import java.util.Collections;
import java.util.List;

import static org.elasticsearch.cluster.metadata.IndexMetaData.SETTING_NUMBER_OF_REPLICAS;
Expand Down Expand Up @@ -101,6 +102,51 @@ public void testIndexCreationOverLimit() {
assertFalse(clusterState.getMetaData().hasIndex("should-fail"));
}

public void testIndexCreationOverLimitFromTemplate() {
int dataNodes = client().admin().cluster().prepareState().get().getState().getNodes().getDataNodes().size();

final ShardCounts counts;
{
final ShardCounts temporaryCounts = ShardCounts.forDataNodeCount(dataNodes);
/*
* We are going to create an index that will bring us up to one below the limit; we go one below the limit to ensure the
* template is used instead of one shard.
*/
counts = new ShardCounts(
temporaryCounts.shardsPerNode,
temporaryCounts.firstIndexShards - 1,
temporaryCounts.firstIndexReplicas,
temporaryCounts.failingIndexShards + 1,
temporaryCounts.failingIndexReplicas);
}
setShardsPerNode(counts.getShardsPerNode());

if (counts.firstIndexShards > 0) {
createIndex(
"test",
Settings.builder()
.put(indexSettings())
.put(SETTING_NUMBER_OF_SHARDS, counts.getFirstIndexShards())
.put(SETTING_NUMBER_OF_REPLICAS, counts.getFirstIndexReplicas()).build());
}

assertAcked(client().admin()
.indices()
.preparePutTemplate("should-fail*")
.setPatterns(Collections.singletonList("should-fail"))
.setOrder(1)
.setSettings(Settings.builder()
.put(SETTING_NUMBER_OF_SHARDS, counts.getFailingIndexShards())
.put(SETTING_NUMBER_OF_REPLICAS, counts.getFailingIndexReplicas()))
.get());

final IllegalArgumentException e =
expectThrows(IllegalArgumentException.class, () -> client().admin().indices().prepareCreate("should-fail").get());
verifyException(dataNodes, counts, e);
ClusterState clusterState = client().admin().cluster().prepareState().get().getState();
assertFalse(clusterState.getMetaData().hasIndex("should-fail"));
}

public void testIncreaseReplicasOverLimit() {
int dataNodes = client().admin().cluster().prepareState().get().getState().getNodes().getDataNodes().size();

Expand Down