Skip to content

Commit 7d135a2

Browse files
Don't synchronize MapperService.doMerge for preflight checks (#109934)
We don't need to synchronize if we are not actually updating the mapping field. This is needlessly slowing down dynamic mapping updates and blocking the write pool.
1 parent ab55833 commit 7d135a2

File tree

1 file changed

+14
-7
lines changed

1 file changed

+14
-7
lines changed

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

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -560,18 +560,25 @@ public DocumentMapper merge(String type, CompressedXContent mappingSource, Merge
560560
return doMerge(type, reason, mappingSourceAsMap);
561561
}
562562

563-
private synchronized DocumentMapper doMerge(String type, MergeReason reason, Map<String, Object> mappingSourceAsMap) {
563+
private DocumentMapper doMerge(String type, MergeReason reason, Map<String, Object> mappingSourceAsMap) {
564564
Mapping incomingMapping = parseMapping(type, reason, mappingSourceAsMap);
565-
Mapping mapping = mergeMappings(this.mapper, incomingMapping, reason, this.indexSettings);
566565
// TODO: In many cases the source here is equal to mappingSource so we need not serialize again.
567566
// We should identify these cases reliably and save expensive serialization here
568-
DocumentMapper newMapper = newDocumentMapper(mapping, reason, mapping.toCompressedXContent());
569567
if (reason == MergeReason.MAPPING_AUTO_UPDATE_PREFLIGHT) {
570-
return newMapper;
568+
// only doing a merge without updating the actual #mapper field, no need to synchronize
569+
Mapping mapping = mergeMappings(this.mapper, incomingMapping, MergeReason.MAPPING_AUTO_UPDATE_PREFLIGHT, this.indexSettings);
570+
return newDocumentMapper(mapping, MergeReason.MAPPING_AUTO_UPDATE_PREFLIGHT, mapping.toCompressedXContent());
571+
} else {
572+
// synchronized concurrent mapper updates are guaranteed to set merged mappers derived from the mapper value previously read
573+
// TODO: can we even have concurrent updates here?
574+
synchronized (this) {
575+
Mapping mapping = mergeMappings(this.mapper, incomingMapping, reason, this.indexSettings);
576+
DocumentMapper newMapper = newDocumentMapper(mapping, reason, mapping.toCompressedXContent());
577+
this.mapper = newMapper;
578+
assert assertSerialization(newMapper, reason);
579+
return newMapper;
580+
}
571581
}
572-
this.mapper = newMapper;
573-
assert assertSerialization(newMapper, reason);
574-
return newMapper;
575582
}
576583

577584
private DocumentMapper newDocumentMapper(Mapping mapping, MergeReason reason, CompressedXContent mappingSource) {

0 commit comments

Comments
 (0)