Browse Source

Handle tombstone building entirely within ParsedDocument (#72251)

DocumentMapper contains some complicated logic to load
metadata fields so that it can build tombstone documents.
However, we only actually need three metadata mappers for
this purpose, and they are all stateless so this logic is
unnecessary. This commit adds two new static methods to
ParsedDocument to build no-op and delete tombstones,
and removes some ceremony elsewhere.
Alan Woodward 4 years ago
parent
commit
7d812b9f78

+ 0 - 33
server/src/main/java/org/elasticsearch/index/mapper/DocumentMapper.java

@@ -8,26 +8,17 @@
 
 package org.elasticsearch.index.mapper;
 
-import org.apache.lucene.document.StoredField;
-import org.apache.lucene.util.BytesRef;
-import org.elasticsearch.common.bytes.BytesArray;
 import org.elasticsearch.common.compress.CompressedXContent;
-import org.elasticsearch.common.xcontent.XContentType;
 import org.elasticsearch.index.IndexSettings;
 import org.elasticsearch.index.analysis.IndexAnalyzers;
 
-import java.util.Arrays;
-import java.util.Collection;
 import java.util.Collections;
-import java.util.stream.Stream;
 
 public class DocumentMapper {
     private final String type;
     private final CompressedXContent mappingSource;
     private final DocumentParser documentParser;
     private final MappingLookup mappingLookup;
-    private final MetadataFieldMapper[] deleteTombstoneMetadataFieldMappers;
-    private final MetadataFieldMapper[] noopTombstoneMetadataFieldMappers;
 
     public DocumentMapper(RootObjectMapper.Builder rootBuilder, MapperService mapperService) {
         this(
@@ -48,14 +39,6 @@ public class DocumentMapper {
         this.documentParser = documentParser;
         this.mappingLookup = MappingLookup.fromMapping(mapping, documentParser, indexSettings, indexAnalyzers);
         this.mappingSource = mapping.toCompressedXContent();
-        final Collection<String> deleteTombstoneMetadataFields = Arrays.asList(VersionFieldMapper.NAME, IdFieldMapper.NAME,
-            SeqNoFieldMapper.NAME, SeqNoFieldMapper.PRIMARY_TERM_NAME, SeqNoFieldMapper.TOMBSTONE_NAME);
-        this.deleteTombstoneMetadataFieldMappers = Stream.of(mapping.getSortedMetadataMappers())
-            .filter(field -> deleteTombstoneMetadataFields.contains(field.name())).toArray(MetadataFieldMapper[]::new);
-        final Collection<String> noopTombstoneMetadataFields = Arrays.asList(
-            VersionFieldMapper.NAME, SeqNoFieldMapper.NAME, SeqNoFieldMapper.PRIMARY_TERM_NAME, SeqNoFieldMapper.TOMBSTONE_NAME);
-        this.noopTombstoneMetadataFieldMappers = Stream.of(mapping.getSortedMetadataMappers())
-            .filter(field -> noopTombstoneMetadataFields.contains(field.name())).toArray(MetadataFieldMapper[]::new);
     }
 
     public Mapping mapping() {
@@ -98,22 +81,6 @@ public class DocumentMapper {
         return documentParser.parseDocument(source, mappingLookup);
     }
 
-    public ParsedDocument createDeleteTombstoneDoc(String index, String id) throws MapperParsingException {
-        final SourceToParse emptySource = new SourceToParse(index, id, new BytesArray("{}"), XContentType.JSON);
-        return documentParser.parseDocument(emptySource, deleteTombstoneMetadataFieldMappers, mappingLookup).toTombstone();
-    }
-
-    public ParsedDocument createNoopTombstoneDoc(String index, String reason) throws MapperParsingException {
-        final String id = ""; // _id won't be used.
-        final SourceToParse sourceToParse = new SourceToParse(index, id, new BytesArray("{}"), XContentType.JSON);
-        final ParsedDocument parsedDoc = documentParser.parseDocument(sourceToParse, noopTombstoneMetadataFieldMappers, mappingLookup)
-            .toTombstone();
-        // Store the reason of a noop as a raw string in the _source field
-        final BytesRef byteRef = new BytesRef(reason);
-        parsedDoc.rootDoc().add(new StoredField(SourceFieldMapper.NAME, byteRef.bytes, byteRef.offset, byteRef.length));
-        return parsedDoc;
-    }
-
     public void validate(IndexSettings settings, boolean checkLimits) {
         this.mapping().validate(this.mappingLookup);
         if (settings.getIndexMetadata().isRoutingPartitionedIndex()) {

+ 5 - 2
server/src/main/java/org/elasticsearch/index/mapper/IdFieldMapper.java

@@ -251,8 +251,11 @@ public class IdFieldMapper extends MetadataFieldMapper {
 
     @Override
     public void preParse(ParseContext context) {
-        BytesRef id = Uid.encodeId(context.sourceToParse().id());
-        context.doc().add(new Field(NAME, id, Defaults.FIELD_TYPE));
+        context.doc().add(idField(context.sourceToParse().id()));
+    }
+
+    public static Field idField(String id) {
+        return new Field(NAME, Uid.encodeId(id), Defaults.FIELD_TYPE);
     }
 
     @Override

+ 53 - 12
server/src/main/java/org/elasticsearch/index/mapper/ParsedDocument.java

@@ -9,11 +9,15 @@
 package org.elasticsearch.index.mapper;
 
 import org.apache.lucene.document.Field;
+import org.apache.lucene.document.StoredField;
+import org.apache.lucene.util.BytesRef;
+import org.elasticsearch.common.bytes.BytesArray;
 import org.elasticsearch.common.bytes.BytesReference;
 import org.elasticsearch.common.xcontent.XContentType;
-import org.elasticsearch.index.mapper.ParseContext.Document;
 import org.elasticsearch.index.mapper.MapperService.MergeReason;
+import org.elasticsearch.index.mapper.ParseContext.Document;
 
+import java.util.Collections;
 import java.util.List;
 
 /**
@@ -35,6 +39,54 @@ public class ParsedDocument {
 
     private Mapping dynamicMappingsUpdate;
 
+    /**
+     * Create a no-op tombstone document
+     * @param reason    the reason for the no-op
+     */
+    public static ParsedDocument noopTombstone(String reason) {
+        Document document = new Document();
+        SeqNoFieldMapper.SequenceIDFields seqIdFields = SeqNoFieldMapper.SequenceIDFields.tombstone();
+        seqIdFields.addFields(document);
+        Field versionField = VersionFieldMapper.versionField();
+        document.add(versionField);
+        // Store the reason of a noop as a raw string in the _source field
+        final BytesRef byteRef = new BytesRef(reason);
+        document.add(new StoredField(SourceFieldMapper.NAME, byteRef.bytes, byteRef.offset, byteRef.length));
+        return new ParsedDocument(
+            versionField,
+            seqIdFields,
+            "",
+            null,
+            Collections.singletonList(document),
+            new BytesArray("{}"),
+            XContentType.JSON,
+            null
+        );
+    }
+
+    /**
+     * Create a delete tombstone document
+     * @param id    the id of the deleted document
+     */
+    public static ParsedDocument deleteTombstone(String id) {
+        Document document = new Document();
+        SeqNoFieldMapper.SequenceIDFields seqIdFields = SeqNoFieldMapper.SequenceIDFields.tombstone();
+        seqIdFields.addFields(document);
+        Field versionField = VersionFieldMapper.versionField();
+        document.add(versionField);
+        document.add(IdFieldMapper.idField(id));
+        return new ParsedDocument(
+            versionField,
+            seqIdFields,
+            id,
+            null,
+            Collections.singletonList(document),
+            new BytesArray("{}"),
+            XContentType.JSON,
+            null
+        );
+    }
+
     public ParsedDocument(Field version,
                           SeqNoFieldMapper.SequenceIDFields seqID,
                           String id,
@@ -67,17 +119,6 @@ public class ParsedDocument {
         this.seqID.primaryTerm.setLongValue(primaryTerm);
     }
 
-    /**
-     * Makes the processing document as a tombstone document rather than a regular document.
-     * Tombstone documents are stored in Lucene index to represent delete operations or Noops.
-     */
-    ParsedDocument toTombstone() {
-        assert docs().size() == 1 : "Tombstone should have a single doc [" + docs() + "]";
-        this.seqID.tombstoneField.setLongValue(1);
-        rootDoc().add(this.seqID.tombstoneField);
-        return this;
-    }
-
     public String routing() {
         return this.routing;
     }

+ 24 - 7
server/src/main/java/org/elasticsearch/index/mapper/SeqNoFieldMapper.java

@@ -56,7 +56,7 @@ public class SeqNoFieldMapper extends MetadataFieldMapper {
         public final Field primaryTerm;
         public final Field tombstoneField;
 
-        public SequenceIDFields(Field seqNo, Field seqNoDocValue, Field primaryTerm, Field tombstoneField) {
+        private SequenceIDFields(Field seqNo, Field seqNoDocValue, Field primaryTerm, Field tombstoneField) {
             Objects.requireNonNull(seqNo, "sequence number field cannot be null");
             Objects.requireNonNull(seqNoDocValue, "sequence number dv field cannot be null");
             Objects.requireNonNull(primaryTerm, "primary term field cannot be null");
@@ -66,10 +66,27 @@ public class SeqNoFieldMapper extends MetadataFieldMapper {
             this.tombstoneField = tombstoneField;
         }
 
+        public void addFields(Document document) {
+            document.add(seqNo);
+            document.add(seqNoDocValue);
+            document.add(primaryTerm);
+            if (tombstoneField != null) {
+                document.add(tombstoneField);
+            }
+        }
+
         public static SequenceIDFields emptySeqID() {
+            return new SequenceIDFields(
+                new LongPoint(NAME, SequenceNumbers.UNASSIGNED_SEQ_NO),
+                new NumericDocValuesField(NAME, SequenceNumbers.UNASSIGNED_SEQ_NO),
+                new NumericDocValuesField(PRIMARY_TERM_NAME, 0),
+                null);
+        }
+
+        public static SequenceIDFields tombstone() {
             return new SequenceIDFields(new LongPoint(NAME, SequenceNumbers.UNASSIGNED_SEQ_NO),
-                    new NumericDocValuesField(NAME, SequenceNumbers.UNASSIGNED_SEQ_NO),
-                    new NumericDocValuesField(PRIMARY_TERM_NAME, 0), new NumericDocValuesField(TOMBSTONE_NAME, 0));
+                new NumericDocValuesField(NAME, SequenceNumbers.UNASSIGNED_SEQ_NO),
+                new NumericDocValuesField(PRIMARY_TERM_NAME, 0), new NumericDocValuesField(TOMBSTONE_NAME, 1));
         }
     }
 
@@ -78,7 +95,9 @@ public class SeqNoFieldMapper extends MetadataFieldMapper {
     public static final String PRIMARY_TERM_NAME = "_primary_term";
     public static final String TOMBSTONE_NAME = "_tombstone";
 
-    public static final TypeParser PARSER = new FixedTypeParser(c -> new SeqNoFieldMapper());
+    public static final SeqNoFieldMapper INSTANCE = new SeqNoFieldMapper();
+
+    public static final TypeParser PARSER = new FixedTypeParser(c -> INSTANCE);
 
     static final class SeqNoFieldType extends SimpleMappedFieldType {
 
@@ -170,9 +189,7 @@ public class SeqNoFieldMapper extends MetadataFieldMapper {
         // also see ParsedDocument.updateSeqID (called by innerIndex)
         SequenceIDFields seqID = SequenceIDFields.emptySeqID();
         context.seqID(seqID);
-        context.doc().add(seqID.seqNo);
-        context.doc().add(seqID.seqNoDocValue);
-        context.doc().add(seqID.primaryTerm);
+        seqID.addFields(context.doc());
     }
 
     @Override

+ 9 - 3
server/src/main/java/org/elasticsearch/index/mapper/VersionFieldMapper.java

@@ -23,7 +23,9 @@ public class VersionFieldMapper extends MetadataFieldMapper {
     public static final String NAME = "_version";
     public static final String CONTENT_TYPE = "_version";
 
-    public static final TypeParser PARSER = new FixedTypeParser(c -> new VersionFieldMapper());
+    public static final VersionFieldMapper INSTANCE = new VersionFieldMapper();
+
+    public static final TypeParser PARSER = new FixedTypeParser(c -> INSTANCE);
 
     static final class VersionFieldType extends MappedFieldType {
 
@@ -55,12 +57,16 @@ public class VersionFieldMapper extends MetadataFieldMapper {
 
     @Override
     public void preParse(ParseContext context) {
-        // see InternalEngine.updateVersion to see where the real version value is set
-        final Field version = new NumericDocValuesField(NAME, -1L);
+        final Field version = versionField();
         context.version(version);
         context.doc().add(version);
     }
 
+    public static Field versionField() {
+        // see InternalEngine.updateVersion to see where the real version value is set
+        return new NumericDocValuesField(NAME, -1L);
+    }
+
     @Override
     public void postParse(ParseContext context) {
         // In the case of nested docs, let's fill nested docs with version=1 so that Lucene doesn't write a Bitset for documents

+ 2 - 7
server/src/main/java/org/elasticsearch/index/shard/IndexShard.java

@@ -107,7 +107,6 @@ import org.elasticsearch.index.mapper.MappedFieldType;
 import org.elasticsearch.index.mapper.MapperService;
 import org.elasticsearch.index.mapper.Mapping;
 import org.elasticsearch.index.mapper.ParsedDocument;
-import org.elasticsearch.index.mapper.RootObjectMapper;
 import org.elasticsearch.index.mapper.SourceToParse;
 import org.elasticsearch.index.mapper.Uid;
 import org.elasticsearch.index.merge.MergeStats;
@@ -3495,18 +3494,14 @@ public class IndexShard extends AbstractIndexShardComponent implements IndicesCl
     }
 
     private EngineConfig.TombstoneDocSupplier tombstoneDocSupplier() {
-        final RootObjectMapper.Builder noopRootMapper = new RootObjectMapper.Builder("__noop", Version.CURRENT);
-        final DocumentMapper noopDocumentMapper = mapperService != null ?
-            new DocumentMapper(noopRootMapper, mapperService) :
-            null;
         return new EngineConfig.TombstoneDocSupplier() {
             @Override
             public ParsedDocument newDeleteTombstoneDoc(String id) {
-                return docMapper().getDocumentMapper().createDeleteTombstoneDoc(shardId.getIndexName(), id);
+                return ParsedDocument.deleteTombstone(id);
             }
             @Override
             public ParsedDocument newNoopTombstoneDoc(String reason) {
-                return noopDocumentMapper.createNoopTombstoneDoc(shardId.getIndexName(), reason);
+                return ParsedDocument.noopTombstone(reason);
             }
         };
     }

+ 2 - 26
test/framework/src/main/java/org/elasticsearch/index/engine/EngineTestCase.java

@@ -366,36 +366,12 @@ public abstract class EngineTestCase extends ESTestCase {
         return new EngineConfig.TombstoneDocSupplier() {
             @Override
             public ParsedDocument newDeleteTombstoneDoc(String id) {
-                final ParseContext.Document doc = new ParseContext.Document();
-                Field uidField = new Field(IdFieldMapper.NAME, Uid.encodeId(id), IdFieldMapper.Defaults.FIELD_TYPE);
-                doc.add(uidField);
-                Field versionField = new NumericDocValuesField(VersionFieldMapper.NAME, 0);
-                doc.add(versionField);
-                SeqNoFieldMapper.SequenceIDFields seqID = SeqNoFieldMapper.SequenceIDFields.emptySeqID();
-                doc.add(seqID.seqNo);
-                doc.add(seqID.seqNoDocValue);
-                doc.add(seqID.primaryTerm);
-                seqID.tombstoneField.setLongValue(1);
-                doc.add(seqID.tombstoneField);
-                return new ParsedDocument(versionField, seqID, id, null,
-                    Collections.singletonList(doc), new BytesArray("{}"), XContentType.JSON, null);
+                return ParsedDocument.deleteTombstone(id);
             }
 
             @Override
             public ParsedDocument newNoopTombstoneDoc(String reason) {
-                final ParseContext.Document doc = new ParseContext.Document();
-                SeqNoFieldMapper.SequenceIDFields seqID = SeqNoFieldMapper.SequenceIDFields.emptySeqID();
-                doc.add(seqID.seqNo);
-                doc.add(seqID.seqNoDocValue);
-                doc.add(seqID.primaryTerm);
-                seqID.tombstoneField.setLongValue(1);
-                doc.add(seqID.tombstoneField);
-                Field versionField = new NumericDocValuesField(VersionFieldMapper.NAME, 0);
-                doc.add(versionField);
-                BytesRef byteRef = new BytesRef(reason);
-                doc.add(new StoredField(SourceFieldMapper.NAME, byteRef.bytes, byteRef.offset, byteRef.length));
-                return new ParsedDocument(versionField, seqID, null, null,
-                    Collections.singletonList(doc), null, XContentType.JSON, null);
+                return ParsedDocument.noopTombstone(reason);
             }
         };
     }