Removing null from syntheticSourceFallbackFieldName

[Kubik42]

There is a problem with how isSynthetic is set here. More specifically, older indices which still enable synthetic source via

    "_source": {
      "mode": "synthetic"
    },

will inevitably invoke syntheticSourceSupport() inside of various field mapper. This is a problem since here we check the index.mapping.source.mode and not _source.mode. What ends up happening is isSyntheticSource is set to false, meanwhile syntheticSourceSupport() is still invoked. This causes syntheticSourceFallbackFieldName() to return null, which results in a NPE:

java.lang.NullPointerException
	at java.base/java.util.Objects.requireNonNull(Objects.java:220)
	at java.base/java.util.KeyValueHolder.<init>(KeyValueHolder.java:61)
	at java.base/java.util.Map.entry(Map.java:1719)
	at [email protected]/org.elasticsearch.index.mapper.CompositeSyntheticFieldLoader$StoredFieldLayer.storedFieldLoaders(CompositeSyntheticFieldLoader.java:213)
	at java.base/java.util.stream.ReferencePipeline$7$1FlatMap.accept(ReferencePipeline.java:288)
	at java.base/java.util.ArrayList$ArrayListSpliterator.forEachRemaining(ArrayList.java:1716)
	at java.base/java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:570)
	at java.base/java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:560)
	at java.base/java.util.stream.ForEachOps$ForEachOp.evaluateSequential(ForEachOps.java:153)
	at java.base/java.util.stream.ForEachOps$ForEachOp$OfRef.evaluateSequential(ForEachOps.java:176)
	at java.base/java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:265)
	at java.base/java.util.stream.ReferencePipeline.forEach(ReferencePipeline.java:632)
	at java.base/java.util.stream.ReferencePipeline$7$1FlatMap.accept(ReferencePipeline.java:293)
	at java.base/java.util.AbstractList$RandomAccessSpliterator.forEachRemaining(AbstractList.java:722)
	at java.base/java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:570)
	at java.base/java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:560)
	at java.base/java.util.stream.ForEachOps$ForEachOp.evaluateSequential(ForEachOps.java:153)
	at java.base/java.util.stream.ForEachOps$ForEachOp$OfRef.evaluateSequential(ForEachOps.java:176)
	at java.base/java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:265)
	at java.base/java.util.stream.ReferencePipeline.forEach(ReferencePipeline.java:632)
	at java.base/java.util.stream.ReferencePipeline$7$1FlatMap.accept(ReferencePipeline.java:293)
	at java.base/java.util.AbstractList$RandomAccessSpliterator.forEachRemaining(AbstractList.java:722)
	at java.base/java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:570)
	at java.base/java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:560)
	at java.base/java.util.stream.ForEachOps$ForEachOp.evaluateSequential(ForEachOps.java:153)
	at java.base/java.util.stream.ForEachOps$ForEachOp$OfRef.evaluateSequential(ForEachOps.java:176)
	at java.base/java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:265)
	at java.base/java.util.stream.ReferencePipeline.forEach(ReferencePipeline.java:632)
	at java.base/java.util.stream.ReferencePipeline$7$1FlatMap.accept(ReferencePipeline.java:293)
	at java.base/java.util.AbstractList$RandomAccessSpliterator.forEachRemaining(AbstractList.java:722)
	at java.base/java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:570)
	at java.base/java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:560)
	at java.base/java.util.stream.ReduceOps$ReduceOp.evaluateSequential(ReduceOps.java:921)
	at java.base/java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:265)
	at java.base/java.util.stream.ReferencePipeline.collect(ReferencePipeline.java:723)
	at [email protected]/org.elasticsearch.index.mapper.SourceLoader$Synthetic.<init>(SourceLoader.java:140)
	at [email protected]/org.elasticsearch.index.mapper.MappingLookup.newSourceLoader(MappingLookup.java:497)
	at [email protected]/org.elasticsearch.index.mapper.DocumentMapper.validate(DocumentMapper.java:156)
	at [email protected]/org.elasticsearch.index.mapper.MapperService.newDocumentMapper(MapperService.java:611)
	at [email protected]/org.elasticsearch.index.mapper.MapperService.updateMapping(MapperService.java:361)
	at [email protected]/org.elasticsearch.index.IndexService.updateMapping(IndexService.java:885)
	at [email protected]/org.elasticsearch.indices.cluster.IndicesClusterStateService.createIndicesAndUpdateShards(IndicesClusterStateService.java:644)
	at [email protected]/org.elasticsearch.indices.cluster.IndicesClusterStateService.doApplyClusterState(IndicesClusterStateService.java:359)
	at [email protected]/org.elasticsearch.indices.cluster.IndicesClusterStateService.applyClusterState(IndicesClusterStateService.java:304)
	at [email protected]/org.elasticsearch.cluster.service.ClusterApplierService.callClusterStateAppliers(ClusterApplierService.java:605)
	at [email protected]/org.elasticsearch.cluster.service.ClusterApplierService.callClusterStateAppliers(ClusterApplierService.java:591)
	at [email protected]/org.elasticsearch.cluster.service.ClusterApplierService.applyChanges(ClusterApplierService.java:564)
	at [email protected]/org.elasticsearch.cluster.service.ClusterApplierService.runTask(ClusterApplierService.java:493)
	at [email protected]/org.elasticsearch.cluster.service.ClusterApplierService$UpdateTask.run(ClusterApplierService.java:183)
	at [email protected]/org.elasticsearch.common.util.concurrent.ThreadContext$ContextPreservingRunnable.run(ThreadContext.java:1000)
	at [email protected]/org.elasticsearch.common.util.concurrent.PrioritizedEsThreadPoolExecutor$TieBreakingPrioritizedRunnable.runAndClean(PrioritizedEsThreadPoolExecutor.java:218)
	at [email protected]/org.elasticsearch.common.util.concurrent.PrioritizedEsThreadPoolExecutor$TieBreakingPrioritizedRunnable.run(PrioritizedEsThreadPoolExecutor.java:184)
	at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1090)
	at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:614)
	at java.base/java.lang.Thread.run(Thread.java:1474)

[elasticsearchmachine]

Hi @Kubik42, I've created a changelog YAML for you.

[elasticsearchmachine]

Pinging @elastic/es-storage-engine (Team:StorageEngine)

[Kubik42]

My two cents on this:

• it makes no sense to return null and then never perform a null check, so removing null is natural
• the inconsistency between source modes may result in slower query times since some fields will end up in ignored_source as opposed to in stored fields. Although, the recent improvements to ingored_source should minimize this slowness signifcantly

Ultimately, we need to fix isSyntheticSource checks.

[Kubik42]

follow up: #136345

[martijnvg]

Can we add a test (yaml test with reproduction is fine) for this bug? I think we need to create a rolling upgrade test for this and use an old version (8.19.0?). Otherwise this looks good. EDIT: I'm unable to reproduce with a rolling upgrade test, but it does reproduce with setting force_synthetic_source to true on get api request. So I think at least a yaml test that reproduces the problem with force_synthetic_source should be added.