diff --git a/server/src/main/java/org/elasticsearch/cluster/metadata/MetaDataCreateIndexService.java b/server/src/main/java/org/elasticsearch/cluster/metadata/MetaDataCreateIndexService.java index 56d8213c6ce87..3b500056dfbd9 100644 --- a/server/src/main/java/org/elasticsearch/cluster/metadata/MetaDataCreateIndexService.java +++ b/server/src/main/java/org/elasticsearch/cluster/metadata/MetaDataCreateIndexService.java @@ -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) { @@ -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) { @@ -607,9 +618,6 @@ public void validateIndexSettings(String indexName, final Settings settings, fin final boolean forbidPrivateIndexSettings) throws IndexCreationException { List validationErrors = getIndexSettingsValidationErrors(settings, forbidPrivateIndexSettings); - Optional shardAllocation = checkShardLimit(settings, clusterState); - shardAllocation.ifPresent(validationErrors::add); - if (validationErrors.isEmpty() == false) { ValidationException validationException = new ValidationException(); validationException.addValidationErrors(validationErrors); @@ -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 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 shardLimit = IndicesService.checkShardLimit(shardsToCreate, clusterState); + if (shardLimit.isPresent()) { + final ValidationException e = new ValidationException(); + e.addValidationError(shardLimit.get()); + throw e; + } } List getIndexSettingsValidationErrors(final Settings settings, final boolean forbidPrivateIndexSettings) { diff --git a/server/src/main/java/org/elasticsearch/snapshots/RestoreService.java b/server/src/main/java/org/elasticsearch/snapshots/RestoreService.java index 4d7ecf5b962a2..ad346426333c5 100644 --- a/server/src/main/java/org/elasticsearch/snapshots/RestoreService.java +++ b/server/src/main/java/org/elasticsearch/snapshots/RestoreService.java @@ -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(); diff --git a/server/src/test/java/org/elasticsearch/cluster/metadata/IndexCreationTaskTests.java b/server/src/test/java/org/elasticsearch/cluster/metadata/IndexCreationTaskTests.java index f2a87d09eb1f2..691e23bb87ad1 100644 --- a/server/src/test/java/org/elasticsearch/cluster/metadata/IndexCreationTaskTests.java +++ b/server/src/test/java/org/elasticsearch/cluster/metadata/IndexCreationTaskTests.java @@ -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) { + // 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); } diff --git a/server/src/test/java/org/elasticsearch/cluster/metadata/MetaDataCreateIndexServiceTests.java b/server/src/test/java/org/elasticsearch/cluster/metadata/MetaDataCreateIndexServiceTests.java index 667909d644beb..cfbc7895a1cce 100644 --- a/server/src/test/java/org/elasticsearch/cluster/metadata/MetaDataCreateIndexServiceTests.java +++ b/server/src/test/java/org/elasticsearch/cluster/metadata/MetaDataCreateIndexServiceTests.java @@ -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; @@ -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; @@ -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 { @@ -466,14 +468,19 @@ public void testShardLimit() { .put(SETTING_NUMBER_OF_REPLICAS, counts.getFailingIndexReplicas()) .build(); - DeprecationLogger deprecationLogger = new DeprecationLogger(logger); - Optional 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))); } } diff --git a/server/src/test/java/org/elasticsearch/cluster/shards/ClusterShardLimitIT.java b/server/src/test/java/org/elasticsearch/cluster/shards/ClusterShardLimitIT.java index e79bf35dda335..849cc1ad4efb8 100644 --- a/server/src/test/java/org/elasticsearch/cluster/shards/ClusterShardLimitIT.java +++ b/server/src/test/java/org/elasticsearch/cluster/shards/ClusterShardLimitIT.java @@ -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; @@ -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();