diff --git a/benchmarks/src/main/java/org/elasticsearch/benchmark/index/mapper/DynamicMapperBenchmark.java b/benchmarks/src/main/java/org/elasticsearch/benchmark/index/mapper/DynamicMapperBenchmark.java index eae233e276038..2d042977cc4e7 100644 --- a/benchmarks/src/main/java/org/elasticsearch/benchmark/index/mapper/DynamicMapperBenchmark.java +++ b/benchmarks/src/main/java/org/elasticsearch/benchmark/index/mapper/DynamicMapperBenchmark.java @@ -108,7 +108,8 @@ private SourceToParse generateRandomDocument() { if (random.nextBoolean()) { continue; } - String objFieldPrefix = Stream.generate(() -> "obj_field_" + idx).limit(objFieldDepth).collect(Collectors.joining(".")); + int objFieldDepthActual = random.nextInt(1, objFieldDepth); + String objFieldPrefix = Stream.generate(() -> "obj_field_" + idx).limit(objFieldDepthActual).collect(Collectors.joining(".")); for (int j = 0; j < textFields; j++) { if (random.nextBoolean()) { StringBuilder fieldValueBuilder = generateTextField(fieldValueCountMax); diff --git a/benchmarks/src/main/java/org/elasticsearch/benchmark/index/mapper/MapperServiceFactory.java b/benchmarks/src/main/java/org/elasticsearch/benchmark/index/mapper/MapperServiceFactory.java index aabd1ac3f7a1b..9858b124f0e73 100644 --- a/benchmarks/src/main/java/org/elasticsearch/benchmark/index/mapper/MapperServiceFactory.java +++ b/benchmarks/src/main/java/org/elasticsearch/benchmark/index/mapper/MapperServiceFactory.java @@ -45,7 +45,7 @@ public static MapperService create(String mappings) { .put("index.number_of_replicas", 0) .put("index.number_of_shards", 1) .put(IndexMetadata.SETTING_VERSION_CREATED, IndexVersion.current()) - .put("index.mapping.total_fields.limit", 10000) + .put("index.mapping.total_fields.limit", 100000) .build(); IndexMetadata meta = IndexMetadata.builder("index").settings(settings).build(); IndexSettings indexSettings = new IndexSettings(meta, settings); diff --git a/docs/changelog/103858.yaml b/docs/changelog/103858.yaml new file mode 100644 index 0000000000000..3b89e3d4371d3 --- /dev/null +++ b/docs/changelog/103858.yaml @@ -0,0 +1,6 @@ +pr: 103858 +summary: Cache `ObjectMappers` to prevent continual recursion during dynamic mapper + building +area: Mapping +type: bug +issues: [] diff --git a/server/src/main/java/org/elasticsearch/index/mapper/DocumentMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/DocumentMapper.java index f7dc09cdbb370..ca3844c6d03b9 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/DocumentMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/DocumentMapper.java @@ -27,9 +27,10 @@ public class DocumentMapper { * @return the newly created document mapper */ public static DocumentMapper createEmpty(MapperService mapperService) { - RootObjectMapper root = new RootObjectMapper.Builder(MapperService.SINGLE_MAPPING_NAME, ObjectMapper.Defaults.SUBOBJECTS).build( - MapperBuilderContext.root(false, false) - ); + RootObjectMapper root = (RootObjectMapper) new RootObjectMapper.Builder( + MapperService.SINGLE_MAPPING_NAME, + ObjectMapper.Defaults.SUBOBJECTS + ).build(MapperBuilderContext.root(false, false)); MetadataFieldMapper[] metadata = mapperService.getMetadataMappers().values().toArray(new MetadataFieldMapper[0]); Mapping mapping = new Mapping(root, metadata, null); return new DocumentMapper(mapperService.documentParser(), mapping, mapping.toCompressedXContent(), IndexVersion.current()); diff --git a/server/src/main/java/org/elasticsearch/index/mapper/DocumentParser.java b/server/src/main/java/org/elasticsearch/index/mapper/DocumentParser.java index 17af6259ca27c..bec8a34f6e62e 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/DocumentParser.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/DocumentParser.java @@ -256,7 +256,9 @@ static Mapping createDynamicUpdate(DocumentParserContext context) { for (RuntimeField runtimeField : context.getDynamicRuntimeFields()) { rootBuilder.addRuntimeField(runtimeField); } - RootObjectMapper root = rootBuilder.build(MapperBuilderContext.root(context.mappingLookup().isSourceSynthetic(), false)); + RootObjectMapper root = (RootObjectMapper) rootBuilder.build( + MapperBuilderContext.root(context.mappingLookup().isSourceSynthetic(), false) + ); return context.mappingLookup().getMapping().mappingUpdate(root); } diff --git a/server/src/main/java/org/elasticsearch/index/mapper/Mapping.java b/server/src/main/java/org/elasticsearch/index/mapper/Mapping.java index fb07ddbc56d83..abb394af86ccc 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/Mapping.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/Mapping.java @@ -29,7 +29,7 @@ public final class Mapping implements ToXContentFragment { public static final Mapping EMPTY = new Mapping( - new RootObjectMapper.Builder(MapperService.SINGLE_MAPPING_NAME, ObjectMapper.Defaults.SUBOBJECTS).build( + (RootObjectMapper) new RootObjectMapper.Builder(MapperService.SINGLE_MAPPING_NAME, ObjectMapper.Defaults.SUBOBJECTS).build( MapperBuilderContext.root(false, false) ), new MetadataFieldMapper[0], diff --git a/server/src/main/java/org/elasticsearch/index/mapper/MappingParser.java b/server/src/main/java/org/elasticsearch/index/mapper/MappingParser.java index 8b30915ca4d3c..4daf4d0d7eef6 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/MappingParser.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/MappingParser.java @@ -178,7 +178,7 @@ Mapping parse(@Nullable String type, Map mappingSource) throws M } return new Mapping( - rootObjectMapper.build(MapperBuilderContext.root(isSourceSynthetic, isDataStream)), + (RootObjectMapper) rootObjectMapper.build(MapperBuilderContext.root(isSourceSynthetic, isDataStream)), metadataMappers.values().toArray(new MetadataFieldMapper[0]), meta ); diff --git a/server/src/main/java/org/elasticsearch/index/mapper/NestedObjectMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/NestedObjectMapper.java index 257b2270176bc..af3b60b2810c0 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/NestedObjectMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/NestedObjectMapper.java @@ -38,17 +38,19 @@ public Builder(String name, IndexVersion indexCreatedVersion) { } Builder includeInRoot(boolean includeInRoot) { + clearObjectMapperCache(); this.includeInRoot = Explicit.explicitBoolean(includeInRoot); return this; } Builder includeInParent(boolean includeInParent) { + clearObjectMapperCache(); this.includeInParent = Explicit.explicitBoolean(includeInParent); return this; } @Override - public NestedObjectMapper build(MapperBuilderContext context) { + protected NestedObjectMapper doBuild(MapperBuilderContext context) { boolean parentIncludedInRoot = this.includeInRoot.value(); if (context instanceof NestedMapperBuilderContext nc) { // we're already inside a nested mapper, so adjust our includes diff --git a/server/src/main/java/org/elasticsearch/index/mapper/ObjectMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/ObjectMapper.java index d67763879433f..4d0cdf69749c3 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/ObjectMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/ObjectMapper.java @@ -81,25 +81,31 @@ public static class Builder extends Mapper.Builder { protected Dynamic dynamic; protected final List mappersBuilders = new ArrayList<>(); private final Set subMapperNames = new HashSet<>(); // keeps track of dynamically added subfields + // caches the ObjectMapper instance created by build() + // Without the cache, we run into the risk of increased runtime complexity due to the recursive nature of the build() method. + private ObjectMapper objectMapperCache; public Builder(String name, Explicit subobjects) { super(name); this.subobjects = subobjects; } - public Builder enabled(boolean enabled) { + public final Builder enabled(boolean enabled) { this.enabled = Explicit.explicitBoolean(enabled); + clearObjectMapperCache(); return this; } - public Builder dynamic(Dynamic dynamic) { + public final Builder dynamic(Dynamic dynamic) { this.dynamic = dynamic; + clearObjectMapperCache(); return this; } public Builder add(Mapper.Builder builder) { mappersBuilders.add(builder); subMapperNames.add(builder.name); + clearObjectMapperCache(); return this; } @@ -174,8 +180,7 @@ protected final Map buildMappers(MapperBuilderContext mapperBuil return mappers; } - @Override - public ObjectMapper build(MapperBuilderContext context) { + protected ObjectMapper doBuild(MapperBuilderContext context) { return new ObjectMapper( name, context.buildFullName(name), @@ -185,6 +190,19 @@ public ObjectMapper build(MapperBuilderContext context) { buildMappers(context.createChildContext(name)) ); } + + @Override + public final ObjectMapper build(MapperBuilderContext context) { + if (objectMapperCache != null) { + return objectMapperCache; + } + objectMapperCache = doBuild(context); + return objectMapperCache; + } + + protected void clearObjectMapperCache() { + objectMapperCache = null; + } } public static class TypeParser implements Mapper.TypeParser { diff --git a/server/src/main/java/org/elasticsearch/index/mapper/RootObjectMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/RootObjectMapper.java index 65fce1b69b8cc..7bbf8be3ba939 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/RootObjectMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/RootObjectMapper.java @@ -79,22 +79,26 @@ public Builder(String name, Explicit subobjects) { } public Builder dynamicDateTimeFormatter(Collection dateTimeFormatters) { + clearObjectMapperCache(); this.dynamicDateTimeFormatters = new Explicit<>(dateTimeFormatters.toArray(new DateFormatter[0]), true); return this; } public Builder dynamicTemplates(Collection templates) { + clearObjectMapperCache(); this.dynamicTemplates = new Explicit<>(templates.toArray(new DynamicTemplate[0]), true); return this; } @Override public RootObjectMapper.Builder add(Mapper.Builder builder) { + clearObjectMapperCache(); super.add(builder); return this; } public RootObjectMapper.Builder addRuntimeField(RuntimeField runtimeField) { + clearObjectMapperCache(); this.runtimeFields.put(runtimeField.name(), runtimeField); return this; } @@ -105,7 +109,7 @@ public RootObjectMapper.Builder addRuntimeFields(Map runti } @Override - public RootObjectMapper build(MapperBuilderContext context) { + protected RootObjectMapper doBuild(MapperBuilderContext context) { return new RootObjectMapper( name, enabled,