Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove DocumentMapperForType #72158

Closed
wants to merge 12 commits into from
Original file line number Diff line number Diff line change
Expand Up @@ -46,9 +46,9 @@
import org.elasticsearch.common.unit.TimeValue;
import org.elasticsearch.common.util.concurrent.ReleasableLock;
import org.elasticsearch.index.VersionType;
import org.elasticsearch.index.mapper.DocumentMapper;
import org.elasticsearch.index.mapper.IdFieldMapper;
import org.elasticsearch.index.mapper.Mapping;
import org.elasticsearch.index.mapper.MappingLookup;
import org.elasticsearch.index.mapper.ParseContext.Document;
import org.elasticsearch.index.mapper.ParsedDocument;
import org.elasticsearch.index.mapper.Uid;
Expand Down Expand Up @@ -581,7 +581,7 @@ protected final GetResult getFromSearcher(Get get, Engine.Searcher searcher) thr
}
}

public abstract GetResult get(Get get, DocumentMapper mapper, Function<Engine.Searcher, Engine.Searcher> searcherWrapper);
public abstract GetResult get(Get get, MappingLookup mappingLookup, Function<Engine.Searcher, Engine.Searcher> searcherWrapper);

/**
* Acquires a point-in-time reader that can be used to create {@link Engine.Searcher}s on demand.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,8 @@
import org.elasticsearch.core.internal.io.IOUtils;
import org.elasticsearch.index.IndexSettings;
import org.elasticsearch.index.VersionType;
import org.elasticsearch.index.mapper.DocumentMapper;
import org.elasticsearch.index.mapper.IdFieldMapper;
import org.elasticsearch.index.mapper.MappingLookup;
import org.elasticsearch.index.mapper.ParseContext;
import org.elasticsearch.index.mapper.ParsedDocument;
import org.elasticsearch.index.mapper.SeqNoFieldMapper;
Expand Down Expand Up @@ -606,10 +606,10 @@ private ExternalReaderManager createReaderManager(RefreshWarmerListener external
}
}

private GetResult getFromTranslog(Get get, Translog.Index index, DocumentMapper mapper,
private GetResult getFromTranslog(Get get, Translog.Index index, MappingLookup mappingLookup,
Function<Searcher, Searcher> searcherWrapper) throws IOException {
assert get.isReadFromTranslog();
final SingleDocDirectoryReader inMemoryReader = new SingleDocDirectoryReader(shardId, index, mapper, config().getAnalyzer());
final SingleDocDirectoryReader inMemoryReader = new SingleDocDirectoryReader(shardId, index, mappingLookup, config().getAnalyzer());
final Engine.Searcher searcher = new Engine.Searcher("realtime_get", ElasticsearchDirectoryReader.wrap(inMemoryReader, shardId),
config().getSimilarity(), config().getQueryCache(), config().getQueryCachingPolicy(), inMemoryReader);
final Searcher wrappedSearcher = searcherWrapper.apply(searcher);
Expand All @@ -628,7 +628,7 @@ private GetResult getFromTranslog(Get get, Translog.Index index, DocumentMapper
}

@Override
public GetResult get(Get get, DocumentMapper mapper, Function<Engine.Searcher, Engine.Searcher> searcherWrapper) {
public GetResult get(Get get, MappingLookup mappingLookup, Function<Engine.Searcher, Engine.Searcher> searcherWrapper) {
assert Objects.equals(get.uid().field(), IdFieldMapper.NAME) : get.uid().field();
try (ReleasableLock ignored = readLock.acquire()) {
ensureOpen();
Expand Down Expand Up @@ -659,7 +659,7 @@ public GetResult get(Get get, DocumentMapper mapper, Function<Engine.Searcher, E
try {
final Translog.Operation operation = translog.readOperation(versionValue.getLocation());
if (operation != null) {
return getFromTranslog(get, (Translog.Index) operation, mapper, searcherWrapper);
return getFromTranslog(get, (Translog.Index) operation, mappingLookup, searcherWrapper);
}
} catch (IOException e) {
maybeFailEngine("realtime_get", e); // lets check if the translog has failed with a tragic event
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
import org.elasticsearch.common.lucene.index.ElasticsearchDirectoryReader;
import org.elasticsearch.common.util.concurrent.ReleasableLock;
import org.elasticsearch.core.internal.io.IOUtils;
import org.elasticsearch.index.mapper.DocumentMapper;
import org.elasticsearch.index.mapper.MappingLookup;
import org.elasticsearch.index.seqno.SeqNoStats;
import org.elasticsearch.index.seqno.SequenceNumbers;
import org.elasticsearch.index.shard.ShardLongFieldRange;
Expand Down Expand Up @@ -246,7 +246,7 @@ private static TranslogStats translogStats(final EngineConfig config, final Segm
}

@Override
public GetResult get(Get get, DocumentMapper mapper, Function<Searcher, Searcher> searcherWrapper) {
public GetResult get(Get get, MappingLookup mappingLookup, Function<Searcher, Searcher> searcherWrapper) {
return getFromSearcher(get, acquireSearcher("get", SearcherScope.EXTERNAL, searcherWrapper));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
import org.apache.lucene.util.Bits;
import org.elasticsearch.common.xcontent.XContentHelper;
import org.elasticsearch.core.internal.io.IOUtils;
import org.elasticsearch.index.mapper.DocumentMapper;
import org.elasticsearch.index.mapper.MappingLookup;
import org.elasticsearch.index.mapper.ParsedDocument;
import org.elasticsearch.index.mapper.SourceToParse;
import org.elasticsearch.index.shard.ShardId;
Expand All @@ -47,8 +47,8 @@
final class SingleDocDirectoryReader extends DirectoryReader {
private final SingleDocLeafReader leafReader;

SingleDocDirectoryReader(ShardId shardId, Translog.Index operation, DocumentMapper mapper, Analyzer analyzer) throws IOException {
this(new SingleDocLeafReader(shardId, operation, mapper, analyzer));
SingleDocDirectoryReader(ShardId shardId, Translog.Index operation, MappingLookup mappingLookup, Analyzer analyzer) throws IOException {
this(new SingleDocLeafReader(shardId, operation, mappingLookup, analyzer));
}

private SingleDocDirectoryReader(SingleDocLeafReader leafReader) throws IOException {
Expand Down Expand Up @@ -109,15 +109,15 @@ private static class SingleDocLeafReader extends LeafReader {

private final ShardId shardId;
private final Translog.Index operation;
private final DocumentMapper mapper;
private final MappingLookup mappingLookup;
private final Analyzer analyzer;
private final Directory directory;
private final AtomicReference<LeafReader> delegate = new AtomicReference<>();

SingleDocLeafReader(ShardId shardId, Translog.Index operation, DocumentMapper mapper, Analyzer analyzer) {
SingleDocLeafReader(ShardId shardId, Translog.Index operation, MappingLookup mappingLookup, Analyzer analyzer) {
this.shardId = shardId;
this.operation = operation;
this.mapper = mapper;
this.mappingLookup = mappingLookup;
this.analyzer = analyzer;
this.directory = new ByteBuffersDirectory();
}
Expand All @@ -140,7 +140,7 @@ private LeafReader getDelegate() {

private LeafReader createInMemoryLeafReader() {
assert Thread.holdsLock(this);
final ParsedDocument parsedDocs = mapper.parse(new SourceToParse(shardId.getIndexName(), operation.id(),
final ParsedDocument parsedDocs = mappingLookup.parseDocument(new SourceToParse(shardId.getIndexName(), operation.id(),
operation.source(), XContentHelper.xContentType(operation.source()), operation.routing(), Map.of()));

parsedDocs.updateSeqID(operation.seqNo(), operation.primaryTerm());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,9 @@
import org.elasticsearch.index.engine.TranslogLeafReader;
import org.elasticsearch.index.fieldvisitor.CustomFieldsVisitor;
import org.elasticsearch.index.fieldvisitor.FieldsVisitor;
import org.elasticsearch.index.mapper.DocumentMapper;
import org.elasticsearch.index.mapper.Mapper;
import org.elasticsearch.index.mapper.MapperService;
import org.elasticsearch.index.mapper.MappingLookup;
import org.elasticsearch.index.mapper.ParsedDocument;
import org.elasticsearch.index.mapper.RoutingFieldMapper;
import org.elasticsearch.index.mapper.SourceFieldMapper;
Expand Down Expand Up @@ -122,7 +122,7 @@ public GetResult get(Engine.GetResult engineGetResult, String id,
try {
long now = System.nanoTime();
fetchSourceContext = normalizeFetchSourceContent(fetchSourceContext, fields);
GetResult getResult = innerGetLoadFromStoredFields(id, fields, fetchSourceContext, engineGetResult, mapperService);
GetResult getResult = innerGetLoadFromStoredFields(id, fields, fetchSourceContext, engineGetResult);
if (getResult.isExists()) {
existsMetric.inc(System.nanoTime() - now);
} else {
Expand Down Expand Up @@ -169,23 +169,23 @@ private GetResult innerGet(String id, String[] gFields, boolean realtime, long v

try {
// break between having loaded it from translog (so we only have _source), and having a document to load
return innerGetLoadFromStoredFields(id, gFields, fetchSourceContext, get, mapperService);
return innerGetLoadFromStoredFields(id, gFields, fetchSourceContext, get);
} finally {
get.close();
}
}

private GetResult innerGetLoadFromStoredFields(String id, String[] storedFields, FetchSourceContext fetchSourceContext,
Engine.GetResult get, MapperService mapperService) {
Engine.GetResult get) {
assert get.exists() : "method should only be called if document could be retrieved";

// check first if stored fields to be loaded don't contain an object field
DocumentMapper docMapper = mapperService.documentMapper();
MappingLookup mappingLookup = mapperService.mappingLookup();
if (storedFields != null) {
for (String field : storedFields) {
Mapper fieldMapper = docMapper.mappers().getMapper(field);
Mapper fieldMapper = mappingLookup.getMapper(field);
if (fieldMapper == null) {
if (docMapper.mappers().objectMappers().get(field) != null) {
if (mappingLookup.objectMappers().get(field) != null) {
// Only fail if we know it is a object field, missing paths / fields shouldn't fail.
throw new IllegalArgumentException("field [" + field + "] isn't a leaf field");
}
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ public enum MergeReason {
private final Version indexVersionCreated;
private final MapperRegistry mapperRegistry;
private final Supplier<Mapper.TypeParser.ParserContext> parserContextSupplier;
private final MappingLookup emptyMappingLookup;

private volatile DocumentMapper mapper;

Expand All @@ -117,6 +118,10 @@ public MapperService(IndexSettings indexSettings, IndexAnalyzers indexAnalyzers,
this.parserContextSupplier = () -> parserContextFunction.apply(null);
this.mappingParser = new MappingParser(parserContextSupplier, metadataMapperParsers,
this::getMetadataMappers, this::resolveDocumentType);
//pre-compute the empty mapping lookup for this mapper service so it can be returned whenever document mapper is null
MetadataFieldMapper[] metadataMappers = getMetadataMappers().values().toArray(new MetadataFieldMapper[0]);
Mapping mapping = Mapping.empty(MapperService.SINGLE_MAPPING_NAME, indexSettings.getIndexVersionCreated(), metadataMappers);
this.emptyMappingLookup = MappingLookup.fromMapping(mapping, documentParser, indexSettings, indexAnalyzers);
}

public boolean hasNested() {
Expand Down Expand Up @@ -149,7 +154,6 @@ Map<Class<? extends MetadataFieldMapper>, MetadataFieldMapper> getMetadataMapper
MetadataFieldMapper metadataFieldMapper = parser.getDefault(parserContext());
metadataMappers.put(metadataFieldMapper.getClass(), metadataFieldMapper);
}

} else {
metadataMappers.putAll(existingMapper.mapping().getMetadataMappersMap());
}
Expand Down Expand Up @@ -343,20 +347,6 @@ private String resolveDocumentType(String type) {
return type;
}

/**
* Returns the document mapper for this MapperService. If no mapper exists,
* creates one and returns that.
*/
public DocumentMapperForType documentMapperWithAutoCreate() {
DocumentMapper mapper = documentMapper();
if (mapper != null) {
return new DocumentMapperForType(mapper, null);
}
Mapping mapping = mappingParser.parse(SINGLE_MAPPING_NAME, null);
mapper = new DocumentMapper(indexSettings, indexAnalyzers, documentParser, mapping);
return new DocumentMapperForType(mapper, mapper.mapping());
}

/**
* Given the full name of a field, returns its {@link MappedFieldType}.
*/
Expand All @@ -369,7 +359,7 @@ public MappedFieldType fieldType(String fullName) {
*/
public MappingLookup mappingLookup() {
DocumentMapper mapper = this.mapper;
return mapper == null ? MappingLookup.EMPTY : mapper.mappers();
return mapper == null ? emptyMappingLookup : mapper.mappers();
}

/**
Expand Down
22 changes: 17 additions & 5 deletions server/src/main/java/org/elasticsearch/index/mapper/Mapping.java
Original file line number Diff line number Diff line change
Expand Up @@ -36,10 +36,19 @@
*/
public final class Mapping implements ToXContentFragment {

public static final Mapping EMPTY = new Mapping(
new RootObjectMapper.Builder("_doc", Version.CURRENT).build(new ContentPath()),
new MetadataFieldMapper[0],
Collections.emptyMap());
/**
* Creates a new instance of empty mapping that have no fields explicitly mapped
* @param type the type
* @param indexVersionCreated the version which the current index has been created on
* @param metadataMappers the metadata mappers registered
* @return the empty mapping instance
*/
public static Mapping empty(String type, Version indexVersionCreated, MetadataFieldMapper[] metadataMappers) {
return new Mapping(
new RootObjectMapper.Builder(type, indexVersionCreated).build(new ContentPath()),
metadataMappers,
Collections.emptyMap());
}

private final RootObjectMapper root;
private final Map<String, Object> meta;
Expand All @@ -66,7 +75,6 @@ public int compare(Mapper o1, Mapper o2) {
this.metadataMappersMap = unmodifiableMap(metadataMappersMap);
this.metadataMappersByName = unmodifiableMap(metadataMappersByName);
this.meta = meta;

}

CompressedXContent toCompressedXContent() {
Expand All @@ -77,6 +85,10 @@ CompressedXContent toCompressedXContent() {
}
}

boolean hasMappings() {
return root.iterator().hasNext();
}

/**
* Returns the root object for the current mapping
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,19 +34,6 @@ public static class CacheKey {
private CacheKey() {}
}

/**
* A lookup representing an empty mapping.
*/
public static final MappingLookup EMPTY = new MappingLookup(
Mapping.EMPTY,
List.of(),
List.of(),
List.of(),
null,
null,
null
);

private final CacheKey cacheKey = new CacheKey();

/** Full field name to mapper */
Expand Down Expand Up @@ -324,7 +311,7 @@ public ParsedDocument parseDocument(SourceToParse source) {
}

public boolean hasMappings() {
return this != EMPTY;
return mapping.hasMappings();
}

public boolean isSourceEnabled() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -138,10 +138,11 @@ protected QueryBuilder doRewrite(QueryRewriteContext queryRewriteContext) throws

@Override
protected Query doToQuery(SearchExecutionContext context) throws IOException {
MappedFieldType idField = context.getFieldType(IdFieldMapper.NAME);
if (idField == null || ids.isEmpty()) {
if (context.hasMappings() == false || ids.isEmpty()) {
throw new IllegalStateException("Rewrite first");
}
MappedFieldType idField = context.getFieldType(IdFieldMapper.NAME);
assert idField != null;
return idField.termsQuery(new ArrayList<>(ids), context);
}

Expand Down
Loading