Check shard limit after applying index templates (#44619)

Today when creating an index and checking cluster shard limits, we check
the number of shards before applying index templates. At this point, we
do not know the actual number of shards that will be used to create the
index. In a case when the defaults are used and a template would
override, we could be grossly underestimating the number of shards that
would be created, and thus incorrectly applying the limits. This commit
addresses this by checking the shard limits after applying index
templates.

Author: jasontedor

server/src/main/java/org/elasticsearch/cluster/metadata/MetaDataCreateIndexService.java

@@ -20,8 +20,8 @@
 package org.elasticsearch.cluster.metadata;
 
 import com.carrotsearch.hppc.cursors.ObjectObjectCursor;
-import org.apache.logging.log4j.Logger;
 import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
 import org.apache.logging.log4j.message.ParameterizedMessage;
 import org.elasticsearch.ElasticsearchException;
 import org.elasticsearch.ResourceAlreadyExistsException;
@@ -435,6 +435,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) {
@@ -583,6 +590,10 @@ public ClusterState execute(ClusterState currentState) throws Exception {
             }
         }
 
+        protected void checkShardLimit(final Settings settings, final ClusterState clusterState) {
+            MetaDataCreateIndexService.checkShardLimit(settings, clusterState, deprecationLogger);
+        }
+
         @Override
         public void onFailure(String source, Exception e) {
             if (e instanceof ResourceAlreadyExistsException) {
@@ -603,9 +614,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, deprecationLogger);
-        shardAllocation.ifPresent(validationErrors::add);
-
         if (validationErrors.isEmpty() == false) {
             ValidationException validationException = new ValidationException();
             validationException.addValidationErrors(validationErrors);
@@ -616,16 +624,22 @@ 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.
-     * @param deprecationLogger The logger to use to emit a deprecation warning, if appropriate.
-     * @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
+     * @param deprecationLogger the logger to use to emit a deprecation warning, if appropriate
+     * @throws ValidationException if creating this index would put the cluster over the cluster shard limit
      */
-    static Optional<String> checkShardLimit(Settings settings, ClusterState clusterState, DeprecationLogger deprecationLogger) {
-        int shardsToCreate = IndexMetaData.INDEX_NUMBER_OF_SHARDS_SETTING.get(settings)
-            * (1 + IndexMetaData.INDEX_NUMBER_OF_REPLICAS_SETTING.get(settings));
-
-        return IndicesService.checkShardLimit(shardsToCreate, clusterState, deprecationLogger);
+    public static void checkShardLimit(Settings settings, ClusterState clusterState, DeprecationLogger deprecationLogger) {
+        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, deprecationLogger);
+        if (shardLimit.isPresent()) {
+            final ValidationException e = new ValidationException();
+            e.addValidationError(shardLimit.get());
+            throw e;
+        }
     }
 
     List<String> getIndexSettingsValidationErrors(final Settings settings, final boolean forbidPrivateIndexSettings) {

server/src/main/java/org/elasticsearch/snapshots/RestoreService.java

@@ -58,6 +58,7 @@
 import org.elasticsearch.common.UUIDs;
 import org.elasticsearch.common.collect.ImmutableOpenMap;
 import org.elasticsearch.common.inject.Inject;
+import org.elasticsearch.common.logging.DeprecationLogger;
 import org.elasticsearch.common.lucene.Lucene;
 import org.elasticsearch.common.regex.Regex;
 import org.elasticsearch.common.settings.ClusterSettings;
@@ -117,6 +118,7 @@
 public class RestoreService implements ClusterStateApplier {
 
     private static final Logger logger = LogManager.getLogger(RestoreService.class);
+    private static final DeprecationLogger deprecationLogger = new DeprecationLogger(logger);
 
     private static final Set<String> UNMODIFIABLE_SETTINGS = unmodifiableSet(newHashSet(
             SETTING_NUMBER_OF_SHARDS,
@@ -288,6 +290,10 @@ public ClusterState execute(ClusterState currentState) {
                                 indexMdBuilder.settings(Settings.builder()
                                                                 .put(snapshotIndexMetaData.getSettings())
                                                                 .put(IndexMetaData.SETTING_INDEX_UUID, UUIDs.randomBase64UUID()));
+                                MetaDataCreateIndexService.checkShardLimit(
+                                    snapshotIndexMetaData.getSettings(),
+                                    currentState,
+                                    deprecationLogger);
                                 if (!request.includeAliases() && !snapshotIndexMetaData.getAliases().isEmpty()) {
                                     // Remove all aliases - they shouldn't be restored
                                     indexMdBuilder.removeAllAliases();

server/src/test/java/org/elasticsearch/cluster/metadata/IndexCreationTaskTests.java

@@ -471,7 +471,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);
     }
 

server/src/test/java/org/elasticsearch/cluster/shards/ClusterShardLimitIT.java

@@ -17,7 +17,6 @@
  * under the License.
  */
 
-
 package org.elasticsearch.cluster.shards;
 
 import org.elasticsearch.action.admin.cluster.settings.ClusterUpdateSettingsResponse;