diff --git a/core/src/main/java/org/elasticsearch/index/mapper/MapperService.java b/core/src/main/java/org/elasticsearch/index/mapper/MapperService.java index aeab1e5c0cf..68983bcf63f 100755 --- a/core/src/main/java/org/elasticsearch/index/mapper/MapperService.java +++ b/core/src/main/java/org/elasticsearch/index/mapper/MapperService.java @@ -110,7 +110,7 @@ public class MapperService extends AbstractIndexComponent implements Closeable { private volatile Map mappers = emptyMap(); private volatile FieldTypeLookup fieldTypes; - private volatile Map fullPathObjectMappers = new HashMap<>(); + private volatile Map fullPathObjectMappers = emptyMap(); private boolean hasNested = false; // updated dynamically to true when a nested object is added private boolean allEnabled = false; // updated dynamically to true when _all is enabled @@ -394,6 +394,7 @@ public class MapperService extends AbstractIndexComponent implements Closeable { for (ObjectMapper objectMapper : objectMappers) { if (fullPathObjectMappers == this.fullPathObjectMappers) { + // first time through the loops fullPathObjectMappers = new HashMap<>(this.fullPathObjectMappers); } fullPathObjectMappers.put(objectMapper.fullPath(), objectMapper); @@ -414,6 +415,7 @@ public class MapperService extends AbstractIndexComponent implements Closeable { if (oldMapper == null && newMapper.parentFieldMapper().active()) { if (parentTypes == this.parentTypes) { + // first time through the loop parentTypes = new HashSet<>(this.parentTypes); } parentTypes.add(mapper.parentFieldMapper().type()); @@ -456,8 +458,15 @@ public class MapperService extends AbstractIndexComponent implements Closeable { // make structures immutable mappers = Collections.unmodifiableMap(mappers); results = Collections.unmodifiableMap(results); - parentTypes = Collections.unmodifiableSet(parentTypes); - fullPathObjectMappers = Collections.unmodifiableMap(fullPathObjectMappers); + + // only need to immutably rewrap these if the previous reference was changed. + // if not then they are already implicitly immutable. + if (fullPathObjectMappers != this.fullPathObjectMappers) { + fullPathObjectMappers = Collections.unmodifiableMap(fullPathObjectMappers); + } + if (parentTypes != this.parentTypes) { + parentTypes = Collections.unmodifiableSet(parentTypes); + } // commit the change if (defaultMappingSource != null) { diff --git a/core/src/test/java/org/elasticsearch/index/mapper/MapperServiceTests.java b/core/src/test/java/org/elasticsearch/index/mapper/MapperServiceTests.java index 5c6ffb70c73..0a6a8f8d469 100644 --- a/core/src/test/java/org/elasticsearch/index/mapper/MapperServiceTests.java +++ b/core/src/test/java/org/elasticsearch/index/mapper/MapperServiceTests.java @@ -38,6 +38,7 @@ import java.util.Collections; import java.util.HashMap; import java.util.HashSet; import java.util.Map; +import java.util.Set; import java.util.concurrent.ExecutionException; import java.util.function.Function; @@ -189,6 +190,22 @@ public class MapperServiceTests extends ESSingleNodeTestCase { assertThat(e.getMessage(), startsWith("Failed to parse mapping [type1]: ")); } + public void testMergeParentTypesSame() { + // Verifies that a merge (absent a DocumentMapper change) + // doesn't change the parentTypes reference. + // The collection was being rewrapped with each merge + // in v5.2 resulting in eventual StackOverflowErrors. + // https://github.com/elastic/elasticsearch/issues/23604 + + IndexService indexService1 = createIndex("index1"); + MapperService mapperService = indexService1.mapperService(); + Set parentTypes = mapperService.getParentTypes(); + + Map> mappings = new HashMap<>(); + mapperService.merge(mappings, MergeReason.MAPPING_UPDATE, false); + assertSame(parentTypes, mapperService.getParentTypes()); + } + public void testOtherDocumentMappersOnlyUpdatedWhenChangingFieldType() throws IOException { IndexService indexService = createIndex("test");