Don't count metadata fields towards index.mapping.total_fields.limit (#33386)

The maximum number of fields per index is limited to 1000 by default by the
`index.mapping.total_fields.limit` setting to prevent accidental mapping
explosions due to too many fields. Currently all metadata fields also count
towards this limit, which can lead to some confusion when using lower limits.
It is not obvious for users that they cannot actually add as many fields as
are specified by the limit in this case.

This change takes the number of metadata fields out of the field count that we
check against the field limit. It also adds tests that check that we can add
fields up to the specified limit, but throw an exception for any additional field added.

Closes #24096

docs/reference/mapping.asciidoc

@@ -77,7 +77,8 @@ can be created manually or dynamically, in order to prevent bad documents from
 causing a mapping explosion:
 
 `index.mapping.total_fields.limit`::
-    The maximum number of fields in an index. The default value is `1000`.
+    The maximum number of fields in an index. Field and object mappings, as well as
+    field aliases count towards this limit. The default value is `1000`.
 
 `index.mapping.depth.limit`::
     The maximum depth for a field, which is measured as the number of inner

server/src/main/java/org/elasticsearch/index/mapper/MapperService.java

@@ -21,6 +21,7 @@ package org.elasticsearch.index.mapper;
 
 import com.carrotsearch.hppc.ObjectHashSet;
 import com.carrotsearch.hppc.cursors.ObjectCursor;
+
 import org.apache.logging.log4j.message.ParameterizedMessage;
 import org.apache.lucene.analysis.Analyzer;
 import org.apache.lucene.analysis.DelegatingAnalyzerWrapper;
@@ -438,7 +439,8 @@ public class MapperService extends AbstractIndexComponent implements Closeable {
             List<ObjectMapper> objectMappers = new ArrayList<>();
             List<FieldMapper> fieldMappers = new ArrayList<>();
             List<FieldAliasMapper> fieldAliasMappers = new ArrayList<>();
-            Collections.addAll(fieldMappers, newMapper.mapping().metadataMappers);
+            MetadataFieldMapper[] metadataMappers = newMapper.mapping().metadataMappers;
+            Collections.addAll(fieldMappers, metadataMappers);
             MapperUtils.collect(newMapper.mapping().root(), objectMappers, fieldMappers, fieldAliasMappers);
 
             MapperMergeValidator.validateMapperStructure(newMapper.type(), objectMappers, fieldMappers,
@@ -472,7 +474,8 @@ public class MapperService extends AbstractIndexComponent implements Closeable {
                 // the master node restoring mappings from disk or data nodes
                 // deserializing cluster state that was sent by the master node,
                 // this check will be skipped.
-                checkTotalFieldsLimit(objectMappers.size() + fieldMappers.size() + fieldAliasMappers.size());
+                // Also, don't take metadata mappers into account for the field limit check
+                checkTotalFieldsLimit(objectMappers.size() + fieldMappers.size() - metadataMappers.length + fieldAliasMappers.size() );
             }
 
             results.put(newMapper.type(), newMapper);

server/src/test/java/org/elasticsearch/index/mapper/MapperServiceTests.java

@@ -24,6 +24,7 @@ import org.elasticsearch.common.Strings;
 import org.elasticsearch.common.bytes.BytesReference;
 import org.elasticsearch.common.compress.CompressedXContent;
 import org.elasticsearch.common.settings.Settings;
+import org.elasticsearch.common.xcontent.XContentBuilder;
 import org.elasticsearch.common.xcontent.XContentFactory;
 import org.elasticsearch.common.xcontent.XContentType;
 import org.elasticsearch.index.IndexService;
@@ -37,13 +38,11 @@ import org.elasticsearch.test.InternalSettingsPlugin;
 import org.hamcrest.Matchers;
 
 import java.io.IOException;
-import java.io.UncheckedIOException;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.Map;
 import java.util.concurrent.ExecutionException;
-import java.util.function.Function;
 
 import static org.hamcrest.CoreMatchers.containsString;
 import static org.hamcrest.Matchers.instanceOf;
@@ -117,35 +116,41 @@ public class MapperServiceTests extends ESSingleNodeTestCase {
         assertNull(indexService.mapperService().documentMapper(MapperService.DEFAULT_MAPPING));
     }
 
-    public void testTotalFieldsExceedsLimit() throws Throwable {
-        Function<String, String> mapping = type -> {
-            try {
-                return Strings.toString(XContentFactory.jsonBuilder().startObject().startObject(type).startObject("properties")
-                    .startObject("field1").field("type", "keyword")
-                    .endObject().endObject().endObject().endObject());
-            } catch (IOException e) {
-                throw new UncheckedIOException(e);
-            }
-        };
-        createIndex("test1").mapperService().merge("type", new CompressedXContent(mapping.apply("type")), MergeReason.MAPPING_UPDATE);
-        //set total number of fields to 1 to trigger an exception
+    /**
+     * Test that we can have at least the number of fields in new mappings that are defined by "index.mapping.total_fields.limit".
+     * Any additional field should trigger an IllegalArgumentException.
+     */
+    public void testTotalFieldsLimit() throws Throwable {
+        int totalFieldsLimit = randomIntBetween(1, 10);
+        Settings settings = Settings.builder().put(MapperService.INDEX_MAPPING_TOTAL_FIELDS_LIMIT_SETTING.getKey(), totalFieldsLimit).build();
+        createIndex("test1", settings).mapperService().merge("type", createMappingSpecifyingNumberOfFields(totalFieldsLimit),
+                MergeReason.MAPPING_UPDATE);
+
+        // adding one more field should trigger exception
         IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> {
-            createIndex("test2", Settings.builder().put(MapperService.INDEX_MAPPING_TOTAL_FIELDS_LIMIT_SETTING.getKey(), 1).build())
-                .mapperService().merge("type", new CompressedXContent(mapping.apply("type")), MergeReason.MAPPING_UPDATE);
+            createIndex("test2", settings).mapperService().merge("type", createMappingSpecifyingNumberOfFields(totalFieldsLimit + 1),
+                    MergeReason.MAPPING_UPDATE);
         });
-        assertTrue(e.getMessage(), e.getMessage().contains("Limit of total fields [1] in index [test2] has been exceeded"));
+        assertTrue(e.getMessage(),
+                e.getMessage().contains("Limit of total fields [" + totalFieldsLimit + "] in index [test2] has been exceeded"));
+    }
+
+    private CompressedXContent createMappingSpecifyingNumberOfFields(int numberOfFields) throws IOException {
+        XContentBuilder mappingBuilder = XContentFactory.jsonBuilder().startObject()
+                .startObject("properties");
+        for (int i = 0; i < numberOfFields; i++) {
+            mappingBuilder.startObject("field" + i);
+            mappingBuilder.field("type", randomFrom("long", "integer", "date", "keyword", "text"));
+            mappingBuilder.endObject();
+        }
+        mappingBuilder.endObject().endObject();
+        return new CompressedXContent(BytesReference.bytes(mappingBuilder));
     }
 
     public void testMappingDepthExceedsLimit() throws Throwable {
-        CompressedXContent simpleMapping = new CompressedXContent(BytesReference.bytes(XContentFactory.jsonBuilder().startObject()
-                .startObject("properties")
-                    .startObject("field")
-                        .field("type", "text")
-                    .endObject()
-                .endObject().endObject()));
         IndexService indexService1 = createIndex("test1", Settings.builder().put(MapperService.INDEX_MAPPING_DEPTH_LIMIT_SETTING.getKey(), 1).build());
         // no exception
-        indexService1.mapperService().merge("type", simpleMapping, MergeReason.MAPPING_UPDATE);
+        indexService1.mapperService().merge("type", createMappingSpecifyingNumberOfFields(1), MergeReason.MAPPING_UPDATE);
 
         CompressedXContent objectMapping = new CompressedXContent(BytesReference.bytes(XContentFactory.jsonBuilder().startObject()
                 .startObject("properties")
@@ -281,22 +286,20 @@ public class MapperServiceTests extends ESSingleNodeTestCase {
             .endObject()
         .endObject().endObject());
 
-        DocumentMapper documentMapper = createIndex("test1").mapperService()
-            .merge("type", new CompressedXContent(mapping), MergeReason.MAPPING_UPDATE);
+        int numberOfFieldsIncludingAlias = 2;
+        createIndex("test1", Settings.builder()
+                .put(MapperService.INDEX_MAPPING_TOTAL_FIELDS_LIMIT_SETTING.getKey(), numberOfFieldsIncludingAlias).build()).mapperService()
+                        .merge("type", new CompressedXContent(mapping), MergeReason.MAPPING_UPDATE);
 
         // Set the total fields limit to the number of non-alias fields, to verify that adding
         // a field alias pushes the mapping over the limit.
-        int numFields = documentMapper.mapping().metadataMappers.length + 2;
-        int numNonAliasFields = numFields - 1;
-
+        int numberOfNonAliasFields = 1;
         IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> {
-            Settings settings = Settings.builder()
-                .put(MapperService.INDEX_MAPPING_TOTAL_FIELDS_LIMIT_SETTING.getKey(), numNonAliasFields)
-                .build();
-            createIndex("test2", settings).mapperService()
-                .merge("type", new CompressedXContent(mapping), MergeReason.MAPPING_UPDATE);
+            createIndex("test2",
+                    Settings.builder().put(MapperService.INDEX_MAPPING_TOTAL_FIELDS_LIMIT_SETTING.getKey(), numberOfNonAliasFields).build())
+                            .mapperService().merge("type", new CompressedXContent(mapping), MergeReason.MAPPING_UPDATE);
         });
-        assertEquals("Limit of total fields [" + numNonAliasFields + "] in index [test2] has been exceeded", e.getMessage());
+        assertEquals("Limit of total fields [" + numberOfNonAliasFields + "] in index [test2] has been exceeded", e.getMessage());
     }
 
     public void testForbidMultipleTypes() throws IOException {