Skip to content
5 changes: 5 additions & 0 deletions docs/changelog/102885.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
pr: 102885
summary: Make field limit more predictable
area: Mapping
type: enhancement
issues: []
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,12 @@ protected boolean supportsStoredFields() {
return false;
}

@Override
public void testTotalFieldsCount() throws IOException {
super.testTotalFieldsCount();
assertWarnings("Parameter [strategy] is deprecated and will be removed in a future version");
}

@Override
protected void registerParameters(ParameterChecker checker) throws IOException {

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -330,7 +330,7 @@ public NodeMappingStats getNodeMappingStats() {
if (mapperService == null) {
return null;
}
long totalCount = mapperService().mappingLookup().getTotalFieldsCount();
long totalCount = mapperService().mappingLookup().getTotalMapperCount();
long totalEstimatedOverhead = totalCount * 1024L; // 1KiB estimated per mapping
NodeMappingStats indexNodeMappingStats = new NodeMappingStats(totalCount, totalEstimatedOverhead);
return indexNodeMappingStats;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -335,7 +335,7 @@ public final boolean addDynamicMapper(Mapper mapper) {
if (mappingLookup.getMapper(mapper.name()) == null
&& mappingLookup.objectMappers().containsKey(mapper.name()) == false
&& dynamicMappers.containsKey(mapper.name()) == false) {
int mapperSize = mapper.mapperSize();
int mapperSize = mapper.getTotalFieldsCount();
int additionalFieldsToAdd = getNewFieldsSize() + mapperSize;
if (indexSettings().isIgnoreDynamicFieldsBeyondLimit()) {
if (mappingLookup.exceedsLimit(indexSettings().getMappingTotalFieldsLimit(), additionalFieldsToAdd)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,11 @@ public void validate(MappingLookup mappers) {
}
}

@Override
public int getTotalFieldsCount() {
return 1;
Copy link
Copy Markdown
Contributor

@salvatore-campagna salvatore-campagna Feb 1, 2024

Choose a reason for hiding this comment

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

I see here that we are counting aliases as one...wouldn't it make sense to skip aliases not counting them?
Considering also that we might use (extensively) passthrough fields #103648
probably it might make sense not to count them.

}

public static class TypeParser implements Mapper.TypeParser {
@Override
public Mapper.Builder parse(String name, Map<String, Object> node, MappingParserContext parserContext)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@
import java.util.function.Consumer;
import java.util.function.Function;
import java.util.function.Supplier;
import java.util.stream.Stream;

import static org.elasticsearch.core.Strings.format;

Expand Down Expand Up @@ -428,6 +429,11 @@ protected void doXContentBody(XContentBuilder builder, Params params) throws IOE

protected abstract String contentType();

@Override
public int getTotalFieldsCount() {
return 1 + Stream.of(multiFields.mappers).mapToInt(FieldMapper::getTotalFieldsCount).sum();
}

public Map<String, NamedAnalyzer> indexAnalyzers() {
return Map.of();
}
Expand Down Expand Up @@ -455,7 +461,7 @@ private void add(FieldMapper mapper) {

private void update(FieldMapper toMerge, MapperMergeContext context) {
if (mapperBuilders.containsKey(toMerge.simpleName()) == false) {
if (context.decrementFieldBudgetIfPossible(toMerge.mapperSize())) {
if (context.decrementFieldBudgetIfPossible(toMerge.getTotalFieldsCount())) {
add(toMerge);
}
} else {
Expand Down
14 changes: 3 additions & 11 deletions server/src/main/java/org/elasticsearch/index/mapper/Mapper.java
Original file line number Diff line number Diff line change
Expand Up @@ -137,16 +137,8 @@ public static FieldType freezeAndDeduplicateFieldType(FieldType fieldType) {
}

/**
* Returns the size this mapper counts against the {@linkplain MapperService#INDEX_MAPPING_TOTAL_FIELDS_LIMIT_SETTING field limit}.
* <p>
* Needs to be in sync with {@link MappingLookup#getTotalFieldsCount()}.
* The total number of fields as defined in the mapping.
* Defines how this mapper counts towards {@link MapperService#INDEX_MAPPING_TOTAL_FIELDS_LIMIT_SETTING}.
*/
public int mapperSize() {
int size = 1;
for (Mapper mapper : this) {
size += mapper.mapperSize();
}
return size;
}

public abstract int getTotalFieldsCount();
}
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ private CacheKey() {}
private final List<FieldMapper> indexTimeScriptMappers;
private final Mapping mapping;
private final Set<String> completionFields;
private final int totalFieldsCount;

/**
* Creates a new {@link MappingLookup} instance by parsing the provided mapping and extracting its field definitions.
Expand Down Expand Up @@ -127,6 +128,7 @@ private MappingLookup(
Collection<ObjectMapper> objectMappers,
Collection<FieldAliasMapper> aliasMappers
) {
this.totalFieldsCount = mapping.getRoot().getTotalFieldsCount();
this.mapping = mapping;
Map<String, Mapper> fieldMappers = new HashMap<>();
Map<String, ObjectMapper> objects = new HashMap<>();
Expand Down Expand Up @@ -223,6 +225,14 @@ FieldTypeLookup fieldTypesLookup() {
* Returns the total number of fields defined in the mappings, including field mappers, object mappers as well as runtime fields.
*/
public long getTotalFieldsCount() {
return totalFieldsCount;
}

/**
* Returns the total number of mappers defined in the mappings, including field mappers and their sub-fields
* (which are not explicitly defined in the mappings), multi-fields, object mappers, runtime fields and metadata field mappers.
*/
public long getTotalMapperCount() {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think I can no longer tell the difference between getTotalFieldsCount and getTotalMapperCount . It's very subtle, or is there any difference at all?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

return fieldMappers.size() + objectMappers.size() + runtimeFieldMappersCount;
}

Expand Down Expand Up @@ -286,7 +296,7 @@ boolean exceedsLimit(long limit, int additionalFieldsToAdd) {
}

long remainingFieldsUntilLimit(long mappingTotalFieldsLimit) {
return mappingTotalFieldsLimit - getTotalFieldsCount() + mapping.getSortedMetadataMappers().length;
return mappingTotalFieldsLimit - totalFieldsCount;
}

private void checkDimensionFieldLimit(long limit) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,11 @@ public ObjectMapper build(MapperBuilderContext context) {
}
}

@Override
public int getTotalFieldsCount() {
return 1 + mappers.values().stream().mapToInt(Mapper::getTotalFieldsCount).sum();
}

public static class TypeParser implements Mapper.TypeParser {
@Override
public boolean supportsVersion(IndexVersion indexCreatedVersion) {
Expand Down Expand Up @@ -547,7 +552,7 @@ private static Map<String, Mapper> buildMergedMappers(
Mapper mergeIntoMapper = mergedMappers.get(mergeWithMapper.simpleName());
Mapper merged = null;
if (mergeIntoMapper == null) {
if (objectMergeContext.decrementFieldBudgetIfPossible(mergeWithMapper.mapperSize())) {
if (objectMergeContext.decrementFieldBudgetIfPossible(mergeWithMapper.getTotalFieldsCount())) {
merged = mergeWithMapper;
} else if (mergeWithMapper instanceof ObjectMapper om) {
merged = truncateObjectMapper(reason, objectMergeContext, om);
Expand Down Expand Up @@ -581,7 +586,7 @@ private static ObjectMapper truncateObjectMapper(MergeReason reason, MapperMerge
// there's not enough capacity for the whole object mapper,
// so we're just trying to add the shallow object, without it's sub-fields
ObjectMapper shallowObjectMapper = objectMapper.withoutMappers();
if (context.decrementFieldBudgetIfPossible(shallowObjectMapper.mapperSize())) {
if (context.decrementFieldBudgetIfPossible(shallowObjectMapper.getTotalFieldsCount())) {
// now trying to add the sub-fields one by one via a merge, until we hit the limit
return shallowObjectMapper.merge(objectMapper, reason, context);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -576,11 +576,7 @@ private static boolean processField(
}

@Override
public int mapperSize() {
int size = runtimeFields().size();
for (Mapper mapper : this) {
size += mapper.mapperSize();
}
return size;
public int getTotalFieldsCount() {
return mappers.values().stream().mapToInt(Mapper::getTotalFieldsCount).sum() + runtimeFields.size();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ public void testParsing() throws IOException {
);
DocumentMapper mapper = createDocumentMapper(mapping);
assertEquals(mapping, mapper.mappingSource().toString());
assertEquals(2, mapper.mapping().getRoot().mapperSize());
assertEquals(2, mapper.mapping().getRoot().getTotalFieldsCount());
}

public void testParsingWithMissingPath() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -213,10 +213,10 @@ public void testMergeWithLimit() {
final ObjectMapper mergedAdd1 = rootObjectMapper.merge(mergeWith, MapperMergeContext.root(false, false, 1));

// THEN "baz" new field is added to merged mapping
assertEquals(3, rootObjectMapper.mapperSize());
assertEquals(4, mergeWith.mapperSize());
assertEquals(3, mergedAdd0.mapperSize());
assertEquals(4, mergedAdd1.mapperSize());
assertEquals(3, rootObjectMapper.getTotalFieldsCount());
assertEquals(4, mergeWith.getTotalFieldsCount());
assertEquals(3, mergedAdd0.getTotalFieldsCount());
assertEquals(4, mergedAdd1.getTotalFieldsCount());
}

public void testMergeWithLimitTruncatedObjectField() {
Expand All @@ -231,11 +231,11 @@ public void testMergeWithLimitTruncatedObjectField() {
ObjectMapper mergedAdd1 = root.merge(mergeWith, MapperMergeContext.root(false, false, 1));
ObjectMapper mergedAdd2 = root.merge(mergeWith, MapperMergeContext.root(false, false, 2));
ObjectMapper mergedAdd3 = root.merge(mergeWith, MapperMergeContext.root(false, false, 3));
assertEquals(0, root.mapperSize());
assertEquals(0, mergedAdd0.mapperSize());
assertEquals(1, mergedAdd1.mapperSize());
assertEquals(2, mergedAdd2.mapperSize());
assertEquals(3, mergedAdd3.mapperSize());
assertEquals(0, root.getTotalFieldsCount());
assertEquals(0, mergedAdd0.getTotalFieldsCount());
assertEquals(1, mergedAdd1.getTotalFieldsCount());
assertEquals(2, mergedAdd2.getTotalFieldsCount());
assertEquals(3, mergedAdd3.getTotalFieldsCount());

ObjectMapper parent1 = (ObjectMapper) mergedAdd1.getMapper("parent");
assertNull(parent1.getMapper("child1"));
Expand All @@ -262,9 +262,9 @@ public void testMergeSameObjectDifferentFields() {

ObjectMapper mergedAdd0 = root.merge(mergeWith, MapperMergeContext.root(false, false, 0));
ObjectMapper mergedAdd1 = root.merge(mergeWith, MapperMergeContext.root(false, false, 1));
assertEquals(2, root.mapperSize());
assertEquals(2, mergedAdd0.mapperSize());
assertEquals(3, mergedAdd1.mapperSize());
assertEquals(2, root.getTotalFieldsCount());
assertEquals(2, mergedAdd0.getTotalFieldsCount());
assertEquals(3, mergedAdd1.getTotalFieldsCount());

ObjectMapper parent0 = (ObjectMapper) mergedAdd0.getMapper("parent");
assertNotNull(parent0.getMapper("child1"));
Expand All @@ -285,13 +285,13 @@ public void testMergeWithLimitMultiField() {
createTextKeywordMultiField("text", "keyword2")
).build(MapperBuilderContext.root(false, false));

assertEquals(2, mergeInto.mapperSize());
assertEquals(2, mergeWith.mapperSize());
assertEquals(2, mergeInto.getTotalFieldsCount());
assertEquals(2, mergeWith.getTotalFieldsCount());

ObjectMapper mergedAdd0 = mergeInto.merge(mergeWith, MapperMergeContext.root(false, false, 0));
ObjectMapper mergedAdd1 = mergeInto.merge(mergeWith, MapperMergeContext.root(false, false, 1));
assertEquals(2, mergedAdd0.mapperSize());
assertEquals(3, mergedAdd1.mapperSize());
assertEquals(2, mergedAdd0.getTotalFieldsCount());
assertEquals(3, mergedAdd1.getTotalFieldsCount());
}

public void testMergeWithLimitRuntimeField() {
Expand All @@ -302,13 +302,13 @@ public void testMergeWithLimitRuntimeField() {
new TestRuntimeField("existing_runtime_field", "keyword")
).addRuntimeField(new TestRuntimeField("new_runtime_field", "keyword")).build(MapperBuilderContext.root(false, false));

assertEquals(3, mergeInto.mapperSize());
assertEquals(2, mergeWith.mapperSize());
assertEquals(3, mergeInto.getTotalFieldsCount());
assertEquals(2, mergeWith.getTotalFieldsCount());

ObjectMapper mergedAdd0 = mergeInto.merge(mergeWith, MapperMergeContext.root(false, false, 0));
ObjectMapper mergedAdd1 = mergeInto.merge(mergeWith, MapperMergeContext.root(false, false, 1));
assertEquals(3, mergedAdd0.mapperSize());
assertEquals(4, mergedAdd1.mapperSize());
assertEquals(3, mergedAdd0.getTotalFieldsCount());
assertEquals(4, mergedAdd1.getTotalFieldsCount());
}

private static RootObjectMapper createRootSubobjectFalseLeafWithDots() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -530,15 +530,20 @@ public void testSyntheticSourceDocValuesFieldWithout() throws IOException {
assertThat(mapper.mapping().getRoot().syntheticFieldLoader().docValuesLoader(null, null), nullValue());
}

public void testNestedObjectWithMultiFieldsMapperSize() throws IOException {
public void testNestedObjectWithMultiFieldsgetTotalFieldsCount() {
ObjectMapper.Builder mapperBuilder = new ObjectMapper.Builder("parent_size_1", Explicit.IMPLICIT_TRUE).add(
new ObjectMapper.Builder("child_size_2", Explicit.IMPLICIT_TRUE).add(
new TextFieldMapper.Builder("grand_child_size_3", createDefaultIndexAnalyzers()).addMultiField(
new KeywordFieldMapper.Builder("multi_field_size_4", IndexVersion.current())
).addMultiField(new KeywordFieldMapper.Builder("multi_field_size_5", IndexVersion.current()))
)
.addMultiField(
new TextFieldMapper.Builder("grand_child_size_5", createDefaultIndexAnalyzers()).addMultiField(
new KeywordFieldMapper.Builder("multi_field_of_multi_field_size_6", IndexVersion.current())
)
)
)
);
assertThat(mapperBuilder.build(MapperBuilderContext.root(false, false)).mapperSize(), equalTo(5));
assertThat(mapperBuilder.build(MapperBuilderContext.root(false, false)).getTotalFieldsCount(), equalTo(6));
}

public void testWithoutMappers() throws IOException {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ public void testRuntimeSection() throws IOException {
}));
MapperService mapperService = createMapperService(mapping);
assertEquals(mapping, mapperService.documentMapper().mappingSource().toString());
assertEquals(3, mapperService.documentMapper().mapping().getRoot().mapperSize());
assertEquals(3, mapperService.documentMapper().mapping().getRoot().getTotalFieldsCount());
}

public void testRuntimeSectionRejectedUpdate() throws IOException {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -427,6 +427,11 @@ public final void testMinimalToMaximal() throws IOException {
assertParseMaximalWarnings();
}

public void testTotalFieldsCount() throws IOException {
MapperService mapperService = createMapperService(fieldMapping(this::minimalMapping));
assertEquals(1, mapperService.documentMapper().mapping().getRoot().getTotalFieldsCount());
}

protected final void assertParseMinimalWarnings() {
String[] warnings = getParseMinimalWarnings();
if (warnings.length > 0) {
Expand Down