From 84a2c58e7474624281789381f65e39f45b4737ba Mon Sep 17 00:00:00 2001 From: Nhat Nguyen Date: Sat, 12 May 2018 21:49:06 -0400 Subject: [PATCH 1/3] Store the reason of noop in its document tombstone Relates # --- .../java/org/elasticsearch/index/engine/Engine.java | 1 + .../org/elasticsearch/index/engine/EngineConfig.java | 4 +++- .../elasticsearch/index/engine/InternalEngine.java | 6 +++++- .../index/engine/LuceneChangesSnapshot.java | 6 +++++- .../elasticsearch/index/mapper/DocumentMapper.java | 11 ++++++----- .../org/elasticsearch/index/shard/IndexShard.java | 7 ++++--- .../index/engine/InternalEngineTests.java | 4 ++-- .../elasticsearch/index/shard/IndexShardTests.java | 12 ++++++++---- .../elasticsearch/index/engine/EngineTestCase.java | 6 ++++-- 9 files changed, 38 insertions(+), 19 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/index/engine/Engine.java b/server/src/main/java/org/elasticsearch/index/engine/Engine.java index f3a4c8b13bd3e..a599e92368cb2 100644 --- a/server/src/main/java/org/elasticsearch/index/engine/Engine.java +++ b/server/src/main/java/org/elasticsearch/index/engine/Engine.java @@ -1289,6 +1289,7 @@ public int estimatedSizeInBytes() { } public static class NoOp extends Operation { + public static final String REASON_FIELD_NAME = "_reason"; private final String reason; diff --git a/server/src/main/java/org/elasticsearch/index/engine/EngineConfig.java b/server/src/main/java/org/elasticsearch/index/engine/EngineConfig.java index ad6ff12c1d1e9..b3ed57e215cae 100644 --- a/server/src/main/java/org/elasticsearch/index/engine/EngineConfig.java +++ b/server/src/main/java/org/elasticsearch/index/engine/EngineConfig.java @@ -27,6 +27,7 @@ import org.apache.lucene.search.Sort; import org.apache.lucene.search.similarities.Similarity; import org.elasticsearch.common.Nullable; +import org.elasticsearch.common.bytes.BytesReference; import org.elasticsearch.common.settings.Setting; import org.elasticsearch.common.settings.Setting.Property; import org.elasticsearch.common.unit.ByteSizeUnit; @@ -380,8 +381,9 @@ public interface TombstoneDocSupplier { /** * Creates a tombstone document for a noop operation. + * @param source a source in JSON representing the reason of a Noop */ - ParsedDocument newNoopTombstoneDoc(); + ParsedDocument newNoopTombstoneDoc(BytesReference source); } public TombstoneDocSupplier getTombstoneDocSupplier() { diff --git a/server/src/main/java/org/elasticsearch/index/engine/InternalEngine.java b/server/src/main/java/org/elasticsearch/index/engine/InternalEngine.java index e14ecc47c877f..4ac94d504fd02 100644 --- a/server/src/main/java/org/elasticsearch/index/engine/InternalEngine.java +++ b/server/src/main/java/org/elasticsearch/index/engine/InternalEngine.java @@ -53,6 +53,7 @@ import org.elasticsearch.action.index.IndexRequest; import org.elasticsearch.common.Nullable; import org.elasticsearch.common.SuppressForbidden; +import org.elasticsearch.common.bytes.BytesReference; import org.elasticsearch.common.lease.Releasable; import org.elasticsearch.common.lease.Releasables; import org.elasticsearch.common.lucene.LoggerInfoStream; @@ -64,6 +65,7 @@ import org.elasticsearch.common.metrics.CounterMetric; import org.elasticsearch.common.util.concurrent.AbstractRunnable; import org.elasticsearch.common.util.concurrent.ReleasableLock; +import org.elasticsearch.common.xcontent.json.JsonXContent; import org.elasticsearch.core.internal.io.IOUtils; import org.elasticsearch.index.IndexSettings; import org.elasticsearch.index.VersionType; @@ -1349,7 +1351,9 @@ private NoOpResult innerNoOp(final NoOp noOp) throws IOException { Exception failure = null; if (softDeleteEnabled) { try { - final ParsedDocument tombstone = engineConfig.getTombstoneDocSupplier().newNoopTombstoneDoc(); + final BytesReference reason = BytesReference.bytes( + JsonXContent.contentBuilder().startObject().field(NoOp.REASON_FIELD_NAME, noOp.reason()).endObject()); + final ParsedDocument tombstone = engineConfig.getTombstoneDocSupplier().newNoopTombstoneDoc(reason); tombstone.updateSeqID(noOp.seqNo(), noOp.primaryTerm()); // A noop tombstone does not require a _version but it's added to have a fully dense docvalues for the version field. // 1L is selected to optimize the compression because it might probably be the most common value in version field. diff --git a/server/src/main/java/org/elasticsearch/index/engine/LuceneChangesSnapshot.java b/server/src/main/java/org/elasticsearch/index/engine/LuceneChangesSnapshot.java index 127787c7224bd..612068a4a1af2 100644 --- a/server/src/main/java/org/elasticsearch/index/engine/LuceneChangesSnapshot.java +++ b/server/src/main/java/org/elasticsearch/index/engine/LuceneChangesSnapshot.java @@ -34,6 +34,8 @@ import org.apache.lucene.search.TopDocs; import org.elasticsearch.common.bytes.BytesReference; import org.elasticsearch.common.lucene.Lucene; +import org.elasticsearch.common.xcontent.XContentHelper; +import org.elasticsearch.common.xcontent.XContentType; import org.elasticsearch.index.VersionType; import org.elasticsearch.index.fieldvisitor.FieldsVisitor; import org.elasticsearch.index.mapper.IdFieldMapper; @@ -186,7 +188,9 @@ private Translog.Operation readDocAsOp(int docID) throws IOException { final Translog.Operation op; final boolean isTombstone = docValues[leaf.ord].isTombstone(segmentDocID); if (isTombstone && fields.uid() == null) { - op = new Translog.NoOp(seqNo, primaryTerm, ""); // TODO: store reason in ignored fields? + final String reason = (String) XContentHelper.convertToMap(fields.source(), false, XContentType.JSON).v2() + .get(Engine.NoOp.REASON_FIELD_NAME); + op = new Translog.NoOp(seqNo, primaryTerm, reason); assert version == 1L : "Noop tombstone should have version 1L; actual version [" + version + "]"; assert assertDocSoftDeleted(leaf.reader(), segmentDocID) : "Noop but soft_deletes field is not set [" + op + "]"; } else { diff --git a/server/src/main/java/org/elasticsearch/index/mapper/DocumentMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/DocumentMapper.java index df02041ecc446..a4333b6b4c15b 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/DocumentMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/DocumentMapper.java @@ -25,6 +25,7 @@ import org.apache.lucene.search.Weight; import org.elasticsearch.ElasticsearchGenerationException; import org.elasticsearch.common.bytes.BytesArray; +import org.elasticsearch.common.bytes.BytesReference; import org.elasticsearch.common.compress.CompressedXContent; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.text.Text; @@ -178,8 +179,8 @@ public DocumentMapper(MapperService mapperService, Mapping mapping) { TypeFieldMapper.NAME, SeqNoFieldMapper.NAME, SeqNoFieldMapper.PRIMARY_TERM_NAME, SeqNoFieldMapper.TOMBSTONE_NAME); this.deleteTombstoneMetadataFieldMappers = Stream.of(mapping.metadataMappers) .filter(field -> deleteTombstoneMetadataFields.contains(field.name())).toArray(MetadataFieldMapper[]::new); - final Collection noopTombstoneMetadataFields = Arrays.asList( - VersionFieldMapper.NAME, SeqNoFieldMapper.NAME, SeqNoFieldMapper.PRIMARY_TERM_NAME, SeqNoFieldMapper.TOMBSTONE_NAME); + final Collection noopTombstoneMetadataFields = Arrays.asList(VersionFieldMapper.NAME, SourceFieldMapper.NAME, + SeqNoFieldMapper.NAME, SeqNoFieldMapper.PRIMARY_TERM_NAME, SeqNoFieldMapper.TOMBSTONE_NAME); this.noopTombstoneMetadataFieldMappers = Stream.of(mapping.metadataMappers) .filter(field -> noopTombstoneMetadataFields.contains(field.name())).toArray(MetadataFieldMapper[]::new); } @@ -262,10 +263,10 @@ public ParsedDocument createDeleteTombstoneDoc(String index, String type, String return documentParser.parseDocument(emptySource, deleteTombstoneMetadataFieldMappers).toTombstone(); } - public ParsedDocument createNoopTombstoneDoc(String index) throws MapperParsingException { + public ParsedDocument createNoopTombstoneDoc(String index, BytesReference source) throws MapperParsingException { final String id = ""; // _id won't be used. - final SourceToParse emptySource = SourceToParse.source(index, type, id, new BytesArray("{}"), XContentType.JSON); - return documentParser.parseDocument(emptySource, noopTombstoneMetadataFieldMappers).toTombstone(); + final SourceToParse sourceToParse = SourceToParse.source(index, type, id, source, XContentType.JSON); + return documentParser.parseDocument(sourceToParse, noopTombstoneMetadataFieldMappers).toTombstone(); } /** diff --git a/server/src/main/java/org/elasticsearch/index/shard/IndexShard.java b/server/src/main/java/org/elasticsearch/index/shard/IndexShard.java index 1b6cabb948de0..7e5049d69e236 100644 --- a/server/src/main/java/org/elasticsearch/index/shard/IndexShard.java +++ b/server/src/main/java/org/elasticsearch/index/shard/IndexShard.java @@ -52,6 +52,7 @@ import org.elasticsearch.cluster.routing.ShardRouting; import org.elasticsearch.common.Booleans; import org.elasticsearch.common.Nullable; +import org.elasticsearch.common.bytes.BytesReference; import org.elasticsearch.common.collect.Tuple; import org.elasticsearch.common.io.stream.BytesStreamOutput; import org.elasticsearch.common.lease.Releasable; @@ -2606,7 +2607,7 @@ public void afterRefresh(boolean didRefresh) throws IOException { } private EngineConfig.TombstoneDocSupplier tombstoneDocSupplier() { - final RootObjectMapper.Builder noopRootMapper = new RootObjectMapper.Builder("__noop"); + final RootObjectMapper.Builder noopRootMapper = new RootObjectMapper.Builder("__noop").enabled(false); // Don't parse _source final DocumentMapper noopDocumentMapper = new DocumentMapper.Builder(noopRootMapper, mapperService).build(mapperService); return new EngineConfig.TombstoneDocSupplier() { @Override @@ -2614,8 +2615,8 @@ public ParsedDocument newDeleteTombstoneDoc(String type, String id) { return docMapper(type).getDocumentMapper().createDeleteTombstoneDoc(shardId.getIndexName(), type, id); } @Override - public ParsedDocument newNoopTombstoneDoc() { - return noopDocumentMapper.createNoopTombstoneDoc(shardId.getIndexName()); + public ParsedDocument newNoopTombstoneDoc(BytesReference source) { + return noopDocumentMapper.createNoopTombstoneDoc(shardId.getIndexName(), source); } }; } diff --git a/server/src/test/java/org/elasticsearch/index/engine/InternalEngineTests.java b/server/src/test/java/org/elasticsearch/index/engine/InternalEngineTests.java index ce0c3df9b3497..67b0e12940b29 100644 --- a/server/src/test/java/org/elasticsearch/index/engine/InternalEngineTests.java +++ b/server/src/test/java/org/elasticsearch/index/engine/InternalEngineTests.java @@ -3711,7 +3711,7 @@ protected long doGenerateSeqNoForOperation(Operation operation) { }; noOpEngine.recoverFromTranslog(); final int gapsFilled = noOpEngine.fillSeqNoGaps(primaryTerm.get()); - final String reason = randomAlphaOfLength(16); + final String reason = "filling gaps"; noOpEngine.noOp(new Engine.NoOp(maxSeqNo + 1, primaryTerm.get(), LOCAL_TRANSLOG_RECOVERY, System.nanoTime(), reason)); assertThat(noOpEngine.getLocalCheckpointTracker().getCheckpoint(), equalTo((long) (maxSeqNo + 1))); assertThat(noOpEngine.getTranslog().stats().getUncommittedOperations(), equalTo(gapsFilled)); @@ -3737,7 +3737,7 @@ protected long doGenerateSeqNoForOperation(Operation operation) { List operationsFromLucene = readAllOperationsInLucene(noOpEngine, mapperService); assertThat(operationsFromLucene, hasSize(maxSeqNo + 2 - localCheckpoint)); // fills n gap and 2 manual noop. for (int i = 0; i < operationsFromLucene.size(); i++) { - assertThat(operationsFromLucene.get(i), equalTo(new Translog.NoOp(localCheckpoint + 1 + i, primaryTerm.get(), ""))); + assertThat(operationsFromLucene.get(i), equalTo(new Translog.NoOp(localCheckpoint + 1 + i, primaryTerm.get(), "filling gaps"))); } assertConsistentHistoryBetweenTranslogAndLuceneIndex(noOpEngine, mapperService); } finally { diff --git a/server/src/test/java/org/elasticsearch/index/shard/IndexShardTests.java b/server/src/test/java/org/elasticsearch/index/shard/IndexShardTests.java index 7bc4096741073..24de373ecc996 100644 --- a/server/src/test/java/org/elasticsearch/index/shard/IndexShardTests.java +++ b/server/src/test/java/org/elasticsearch/index/shard/IndexShardTests.java @@ -56,6 +56,7 @@ import org.elasticsearch.common.UUIDs; import org.elasticsearch.common.breaker.CircuitBreaker; import org.elasticsearch.common.bytes.BytesArray; +import org.elasticsearch.common.bytes.BytesReference; import org.elasticsearch.common.component.AbstractLifecycleComponent; import org.elasticsearch.common.io.stream.BytesStreamOutput; import org.elasticsearch.common.io.stream.StreamInput; @@ -71,13 +72,13 @@ import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentFactory; import org.elasticsearch.common.xcontent.XContentType; +import org.elasticsearch.common.xcontent.json.JsonXContent; import org.elasticsearch.core.internal.io.IOUtils; import org.elasticsearch.env.NodeEnvironment; import org.elasticsearch.index.IndexSettings; import org.elasticsearch.index.VersionType; import org.elasticsearch.index.engine.Engine; import org.elasticsearch.index.engine.EngineException; -import org.elasticsearch.index.engine.EngineTestCase; import org.elasticsearch.index.engine.InternalEngine; import org.elasticsearch.index.engine.InternalEngineFactory; import org.elasticsearch.index.engine.Segment; @@ -88,10 +89,10 @@ import org.elasticsearch.index.fielddata.IndexFieldDataService; import org.elasticsearch.index.mapper.IdFieldMapper; import org.elasticsearch.index.mapper.MappedFieldType; -import org.elasticsearch.index.mapper.Mapping; import org.elasticsearch.index.mapper.ParseContext; import org.elasticsearch.index.mapper.ParsedDocument; import org.elasticsearch.index.mapper.SeqNoFieldMapper; +import org.elasticsearch.index.mapper.SourceFieldMapper; import org.elasticsearch.index.mapper.SourceToParse; import org.elasticsearch.index.mapper.Uid; import org.elasticsearch.index.mapper.VersionFieldMapper; @@ -3109,13 +3110,16 @@ public void testSupplyTombstoneDoc() throws Exception { assertThat(deleteDoc.getField(IdFieldMapper.NAME).binaryValue(), equalTo(Uid.encodeId(id))); assertThat(deleteDoc.getField(SeqNoFieldMapper.TOMBSTONE_NAME).numericValue().longValue(), equalTo(1L)); - ParsedDocument noopTombstone = shard.getEngine().config().getTombstoneDocSupplier().newNoopTombstoneDoc(); + BytesReference reason = BytesReference.bytes( + JsonXContent.contentBuilder().startObject().field(Engine.NoOp.REASON_FIELD_NAME, randomUnicodeOfLength(200)).endObject()); + ParsedDocument noopTombstone = shard.getEngine().config().getTombstoneDocSupplier().newNoopTombstoneDoc(reason); assertThat(noopTombstone.docs(), hasSize(1)); ParseContext.Document noopDoc = noopTombstone.docs().get(0); assertThat(noopDoc.getFields().stream().map(IndexableField::name).collect(Collectors.toList()), - containsInAnyOrder(VersionFieldMapper.NAME, SeqNoFieldMapper.TOMBSTONE_NAME, + containsInAnyOrder(VersionFieldMapper.NAME, SourceFieldMapper.NAME, SeqNoFieldMapper.TOMBSTONE_NAME, SeqNoFieldMapper.NAME, SeqNoFieldMapper.NAME, SeqNoFieldMapper.PRIMARY_TERM_NAME)); assertThat(noopDoc.getField(SeqNoFieldMapper.TOMBSTONE_NAME).numericValue().longValue(), equalTo(1L)); + assertThat(noopDoc.getField(SourceFieldMapper.NAME).binaryValue(), equalTo(reason.toBytesRef())); closeShards(shard); } diff --git a/test/framework/src/main/java/org/elasticsearch/index/engine/EngineTestCase.java b/test/framework/src/main/java/org/elasticsearch/index/engine/EngineTestCase.java index da972d9224b16..c0ccdd21e6d57 100644 --- a/test/framework/src/main/java/org/elasticsearch/index/engine/EngineTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/index/engine/EngineTestCase.java @@ -306,7 +306,7 @@ public ParsedDocument newDeleteTombstoneDoc(String type, String id) { } @Override - public ParsedDocument newNoopTombstoneDoc() { + public ParsedDocument newNoopTombstoneDoc(BytesReference source) { final ParseContext.Document doc = new ParseContext.Document(); SeqNoFieldMapper.SequenceIDFields seqID = SeqNoFieldMapper.SequenceIDFields.emptySeqID(); doc.add(seqID.seqNo); @@ -316,8 +316,10 @@ public ParsedDocument newNoopTombstoneDoc() { doc.add(seqID.tombstoneField); Field versionField = new NumericDocValuesField(VersionFieldMapper.NAME, 0); doc.add(versionField); + BytesRef bytesRef = source.toBytesRef(); + doc.add(new StoredField(SourceFieldMapper.NAME, bytesRef.bytes, bytesRef.offset, bytesRef.length)); return new ParsedDocument(versionField, seqID, null, null, null, - Collections.singletonList(doc), null, XContentType.JSON, null); + Collections.singletonList(doc), source, XContentType.JSON, null); } }; } From caf32cc41f33a430f63a8e8380795287c88730ad Mon Sep 17 00:00:00 2001 From: Nhat Nguyen Date: Mon, 14 May 2018 12:04:02 -0400 Subject: [PATCH 2/3] Store reason in a raw string --- .../org/elasticsearch/index/engine/Engine.java | 1 - .../index/engine/EngineConfig.java | 5 ++--- .../index/engine/InternalEngine.java | 6 +----- .../index/engine/LuceneChangesSnapshot.java | 6 +----- .../index/mapper/DocumentMapper.java | 17 +++++++++++------ .../elasticsearch/index/shard/IndexShard.java | 7 +++---- .../index/shard/IndexShardTests.java | 7 ++----- .../index/engine/EngineTestCase.java | 8 ++++---- 8 files changed, 24 insertions(+), 33 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/index/engine/Engine.java b/server/src/main/java/org/elasticsearch/index/engine/Engine.java index a599e92368cb2..f3a4c8b13bd3e 100644 --- a/server/src/main/java/org/elasticsearch/index/engine/Engine.java +++ b/server/src/main/java/org/elasticsearch/index/engine/Engine.java @@ -1289,7 +1289,6 @@ public int estimatedSizeInBytes() { } public static class NoOp extends Operation { - public static final String REASON_FIELD_NAME = "_reason"; private final String reason; diff --git a/server/src/main/java/org/elasticsearch/index/engine/EngineConfig.java b/server/src/main/java/org/elasticsearch/index/engine/EngineConfig.java index b3ed57e215cae..57e896bffc5ba 100644 --- a/server/src/main/java/org/elasticsearch/index/engine/EngineConfig.java +++ b/server/src/main/java/org/elasticsearch/index/engine/EngineConfig.java @@ -27,7 +27,6 @@ import org.apache.lucene.search.Sort; import org.apache.lucene.search.similarities.Similarity; import org.elasticsearch.common.Nullable; -import org.elasticsearch.common.bytes.BytesReference; import org.elasticsearch.common.settings.Setting; import org.elasticsearch.common.settings.Setting.Property; import org.elasticsearch.common.unit.ByteSizeUnit; @@ -381,9 +380,9 @@ public interface TombstoneDocSupplier { /** * Creates a tombstone document for a noop operation. - * @param source a source in JSON representing the reason of a Noop + * @param reason the reason of an a noop */ - ParsedDocument newNoopTombstoneDoc(BytesReference source); + ParsedDocument newNoopTombstoneDoc(String reason); } public TombstoneDocSupplier getTombstoneDocSupplier() { diff --git a/server/src/main/java/org/elasticsearch/index/engine/InternalEngine.java b/server/src/main/java/org/elasticsearch/index/engine/InternalEngine.java index 4ac94d504fd02..f43b2c191167a 100644 --- a/server/src/main/java/org/elasticsearch/index/engine/InternalEngine.java +++ b/server/src/main/java/org/elasticsearch/index/engine/InternalEngine.java @@ -53,7 +53,6 @@ import org.elasticsearch.action.index.IndexRequest; import org.elasticsearch.common.Nullable; import org.elasticsearch.common.SuppressForbidden; -import org.elasticsearch.common.bytes.BytesReference; import org.elasticsearch.common.lease.Releasable; import org.elasticsearch.common.lease.Releasables; import org.elasticsearch.common.lucene.LoggerInfoStream; @@ -65,7 +64,6 @@ import org.elasticsearch.common.metrics.CounterMetric; import org.elasticsearch.common.util.concurrent.AbstractRunnable; import org.elasticsearch.common.util.concurrent.ReleasableLock; -import org.elasticsearch.common.xcontent.json.JsonXContent; import org.elasticsearch.core.internal.io.IOUtils; import org.elasticsearch.index.IndexSettings; import org.elasticsearch.index.VersionType; @@ -1351,9 +1349,7 @@ private NoOpResult innerNoOp(final NoOp noOp) throws IOException { Exception failure = null; if (softDeleteEnabled) { try { - final BytesReference reason = BytesReference.bytes( - JsonXContent.contentBuilder().startObject().field(NoOp.REASON_FIELD_NAME, noOp.reason()).endObject()); - final ParsedDocument tombstone = engineConfig.getTombstoneDocSupplier().newNoopTombstoneDoc(reason); + final ParsedDocument tombstone = engineConfig.getTombstoneDocSupplier().newNoopTombstoneDoc(noOp.reason()); tombstone.updateSeqID(noOp.seqNo(), noOp.primaryTerm()); // A noop tombstone does not require a _version but it's added to have a fully dense docvalues for the version field. // 1L is selected to optimize the compression because it might probably be the most common value in version field. diff --git a/server/src/main/java/org/elasticsearch/index/engine/LuceneChangesSnapshot.java b/server/src/main/java/org/elasticsearch/index/engine/LuceneChangesSnapshot.java index 612068a4a1af2..7e4cac542a214 100644 --- a/server/src/main/java/org/elasticsearch/index/engine/LuceneChangesSnapshot.java +++ b/server/src/main/java/org/elasticsearch/index/engine/LuceneChangesSnapshot.java @@ -34,8 +34,6 @@ import org.apache.lucene.search.TopDocs; import org.elasticsearch.common.bytes.BytesReference; import org.elasticsearch.common.lucene.Lucene; -import org.elasticsearch.common.xcontent.XContentHelper; -import org.elasticsearch.common.xcontent.XContentType; import org.elasticsearch.index.VersionType; import org.elasticsearch.index.fieldvisitor.FieldsVisitor; import org.elasticsearch.index.mapper.IdFieldMapper; @@ -188,9 +186,7 @@ private Translog.Operation readDocAsOp(int docID) throws IOException { final Translog.Operation op; final boolean isTombstone = docValues[leaf.ord].isTombstone(segmentDocID); if (isTombstone && fields.uid() == null) { - final String reason = (String) XContentHelper.convertToMap(fields.source(), false, XContentType.JSON).v2() - .get(Engine.NoOp.REASON_FIELD_NAME); - op = new Translog.NoOp(seqNo, primaryTerm, reason); + op = new Translog.NoOp(seqNo, primaryTerm, fields.source().utf8ToString()); assert version == 1L : "Noop tombstone should have version 1L; actual version [" + version + "]"; assert assertDocSoftDeleted(leaf.reader(), segmentDocID) : "Noop but soft_deletes field is not set [" + op + "]"; } else { diff --git a/server/src/main/java/org/elasticsearch/index/mapper/DocumentMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/DocumentMapper.java index a4333b6b4c15b..85cbebb84b77c 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/DocumentMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/DocumentMapper.java @@ -19,13 +19,14 @@ package org.elasticsearch.index.mapper; +import org.apache.lucene.document.StoredField; import org.apache.lucene.index.LeafReaderContext; import org.apache.lucene.search.Query; import org.apache.lucene.search.Scorer; import org.apache.lucene.search.Weight; +import org.apache.lucene.util.BytesRef; import org.elasticsearch.ElasticsearchGenerationException; import org.elasticsearch.common.bytes.BytesArray; -import org.elasticsearch.common.bytes.BytesReference; import org.elasticsearch.common.compress.CompressedXContent; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.text.Text; @@ -179,8 +180,8 @@ public DocumentMapper(MapperService mapperService, Mapping mapping) { TypeFieldMapper.NAME, SeqNoFieldMapper.NAME, SeqNoFieldMapper.PRIMARY_TERM_NAME, SeqNoFieldMapper.TOMBSTONE_NAME); this.deleteTombstoneMetadataFieldMappers = Stream.of(mapping.metadataMappers) .filter(field -> deleteTombstoneMetadataFields.contains(field.name())).toArray(MetadataFieldMapper[]::new); - final Collection noopTombstoneMetadataFields = Arrays.asList(VersionFieldMapper.NAME, SourceFieldMapper.NAME, - SeqNoFieldMapper.NAME, SeqNoFieldMapper.PRIMARY_TERM_NAME, SeqNoFieldMapper.TOMBSTONE_NAME); + final Collection noopTombstoneMetadataFields = Arrays.asList( + VersionFieldMapper.NAME, SeqNoFieldMapper.NAME, SeqNoFieldMapper.PRIMARY_TERM_NAME, SeqNoFieldMapper.TOMBSTONE_NAME); this.noopTombstoneMetadataFieldMappers = Stream.of(mapping.metadataMappers) .filter(field -> noopTombstoneMetadataFields.contains(field.name())).toArray(MetadataFieldMapper[]::new); } @@ -263,10 +264,14 @@ public ParsedDocument createDeleteTombstoneDoc(String index, String type, String return documentParser.parseDocument(emptySource, deleteTombstoneMetadataFieldMappers).toTombstone(); } - public ParsedDocument createNoopTombstoneDoc(String index, BytesReference source) throws MapperParsingException { + public ParsedDocument createNoopTombstoneDoc(String index, String reason) throws MapperParsingException { final String id = ""; // _id won't be used. - final SourceToParse sourceToParse = SourceToParse.source(index, type, id, source, XContentType.JSON); - return documentParser.parseDocument(sourceToParse, noopTombstoneMetadataFieldMappers).toTombstone(); + final SourceToParse sourceToParse = SourceToParse.source(index, type, id, new BytesArray("{}"), XContentType.JSON); + final ParsedDocument parsedDoc = documentParser.parseDocument(sourceToParse, noopTombstoneMetadataFieldMappers).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; } /** diff --git a/server/src/main/java/org/elasticsearch/index/shard/IndexShard.java b/server/src/main/java/org/elasticsearch/index/shard/IndexShard.java index 7e5049d69e236..0ff8cff04473e 100644 --- a/server/src/main/java/org/elasticsearch/index/shard/IndexShard.java +++ b/server/src/main/java/org/elasticsearch/index/shard/IndexShard.java @@ -52,7 +52,6 @@ import org.elasticsearch.cluster.routing.ShardRouting; import org.elasticsearch.common.Booleans; import org.elasticsearch.common.Nullable; -import org.elasticsearch.common.bytes.BytesReference; import org.elasticsearch.common.collect.Tuple; import org.elasticsearch.common.io.stream.BytesStreamOutput; import org.elasticsearch.common.lease.Releasable; @@ -2607,7 +2606,7 @@ public void afterRefresh(boolean didRefresh) throws IOException { } private EngineConfig.TombstoneDocSupplier tombstoneDocSupplier() { - final RootObjectMapper.Builder noopRootMapper = new RootObjectMapper.Builder("__noop").enabled(false); // Don't parse _source + final RootObjectMapper.Builder noopRootMapper = new RootObjectMapper.Builder("__noop"); final DocumentMapper noopDocumentMapper = new DocumentMapper.Builder(noopRootMapper, mapperService).build(mapperService); return new EngineConfig.TombstoneDocSupplier() { @Override @@ -2615,8 +2614,8 @@ public ParsedDocument newDeleteTombstoneDoc(String type, String id) { return docMapper(type).getDocumentMapper().createDeleteTombstoneDoc(shardId.getIndexName(), type, id); } @Override - public ParsedDocument newNoopTombstoneDoc(BytesReference source) { - return noopDocumentMapper.createNoopTombstoneDoc(shardId.getIndexName(), source); + public ParsedDocument newNoopTombstoneDoc(String reason) { + return noopDocumentMapper.createNoopTombstoneDoc(shardId.getIndexName(), reason); } }; } diff --git a/server/src/test/java/org/elasticsearch/index/shard/IndexShardTests.java b/server/src/test/java/org/elasticsearch/index/shard/IndexShardTests.java index 24de373ecc996..8555c11371e35 100644 --- a/server/src/test/java/org/elasticsearch/index/shard/IndexShardTests.java +++ b/server/src/test/java/org/elasticsearch/index/shard/IndexShardTests.java @@ -56,7 +56,6 @@ import org.elasticsearch.common.UUIDs; import org.elasticsearch.common.breaker.CircuitBreaker; import org.elasticsearch.common.bytes.BytesArray; -import org.elasticsearch.common.bytes.BytesReference; import org.elasticsearch.common.component.AbstractLifecycleComponent; import org.elasticsearch.common.io.stream.BytesStreamOutput; import org.elasticsearch.common.io.stream.StreamInput; @@ -72,7 +71,6 @@ import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentFactory; import org.elasticsearch.common.xcontent.XContentType; -import org.elasticsearch.common.xcontent.json.JsonXContent; import org.elasticsearch.core.internal.io.IOUtils; import org.elasticsearch.env.NodeEnvironment; import org.elasticsearch.index.IndexSettings; @@ -3110,8 +3108,7 @@ public void testSupplyTombstoneDoc() throws Exception { assertThat(deleteDoc.getField(IdFieldMapper.NAME).binaryValue(), equalTo(Uid.encodeId(id))); assertThat(deleteDoc.getField(SeqNoFieldMapper.TOMBSTONE_NAME).numericValue().longValue(), equalTo(1L)); - BytesReference reason = BytesReference.bytes( - JsonXContent.contentBuilder().startObject().field(Engine.NoOp.REASON_FIELD_NAME, randomUnicodeOfLength(200)).endObject()); + final String reason = randomUnicodeOfLength(200); ParsedDocument noopTombstone = shard.getEngine().config().getTombstoneDocSupplier().newNoopTombstoneDoc(reason); assertThat(noopTombstone.docs(), hasSize(1)); ParseContext.Document noopDoc = noopTombstone.docs().get(0); @@ -3119,7 +3116,7 @@ public void testSupplyTombstoneDoc() throws Exception { containsInAnyOrder(VersionFieldMapper.NAME, SourceFieldMapper.NAME, SeqNoFieldMapper.TOMBSTONE_NAME, SeqNoFieldMapper.NAME, SeqNoFieldMapper.NAME, SeqNoFieldMapper.PRIMARY_TERM_NAME)); assertThat(noopDoc.getField(SeqNoFieldMapper.TOMBSTONE_NAME).numericValue().longValue(), equalTo(1L)); - assertThat(noopDoc.getField(SourceFieldMapper.NAME).binaryValue(), equalTo(reason.toBytesRef())); + assertThat(noopDoc.getField(SourceFieldMapper.NAME).stringValue(), equalTo(reason)); closeShards(shard); } diff --git a/test/framework/src/main/java/org/elasticsearch/index/engine/EngineTestCase.java b/test/framework/src/main/java/org/elasticsearch/index/engine/EngineTestCase.java index c0ccdd21e6d57..0578650b9b036 100644 --- a/test/framework/src/main/java/org/elasticsearch/index/engine/EngineTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/index/engine/EngineTestCase.java @@ -306,7 +306,7 @@ public ParsedDocument newDeleteTombstoneDoc(String type, String id) { } @Override - public ParsedDocument newNoopTombstoneDoc(BytesReference source) { + public ParsedDocument newNoopTombstoneDoc(String reason) { final ParseContext.Document doc = new ParseContext.Document(); SeqNoFieldMapper.SequenceIDFields seqID = SeqNoFieldMapper.SequenceIDFields.emptySeqID(); doc.add(seqID.seqNo); @@ -316,10 +316,10 @@ public ParsedDocument newNoopTombstoneDoc(BytesReference source) { doc.add(seqID.tombstoneField); Field versionField = new NumericDocValuesField(VersionFieldMapper.NAME, 0); doc.add(versionField); - BytesRef bytesRef = source.toBytesRef(); - doc.add(new StoredField(SourceFieldMapper.NAME, bytesRef.bytes, bytesRef.offset, bytesRef.length)); + BytesRef byteRef = new BytesRef(reason); + doc.add(new StoredField(SourceFieldMapper.NAME, byteRef.bytes, byteRef.offset, byteRef.length)); return new ParsedDocument(versionField, seqID, null, null, null, - Collections.singletonList(doc), source, XContentType.JSON, null); + Collections.singletonList(doc), null, XContentType.JSON, null); } }; } From 4afe2fc6c5bb193b1a042c733d64513598376c2f Mon Sep 17 00:00:00 2001 From: Nhat Nguyen Date: Mon, 14 May 2018 17:35:24 -0400 Subject: [PATCH 3/3] fix _source test --- .../java/org/elasticsearch/index/shard/IndexShardTests.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/server/src/test/java/org/elasticsearch/index/shard/IndexShardTests.java b/server/src/test/java/org/elasticsearch/index/shard/IndexShardTests.java index 8555c11371e35..b96e9679f4df6 100644 --- a/server/src/test/java/org/elasticsearch/index/shard/IndexShardTests.java +++ b/server/src/test/java/org/elasticsearch/index/shard/IndexShardTests.java @@ -29,6 +29,7 @@ import org.apache.lucene.search.TopDocs; import org.apache.lucene.store.AlreadyClosedException; import org.apache.lucene.store.IOContext; +import org.apache.lucene.util.BytesRef; import org.apache.lucene.util.Constants; import org.elasticsearch.Version; import org.elasticsearch.action.ActionListener; @@ -3116,7 +3117,7 @@ public void testSupplyTombstoneDoc() throws Exception { containsInAnyOrder(VersionFieldMapper.NAME, SourceFieldMapper.NAME, SeqNoFieldMapper.TOMBSTONE_NAME, SeqNoFieldMapper.NAME, SeqNoFieldMapper.NAME, SeqNoFieldMapper.PRIMARY_TERM_NAME)); assertThat(noopDoc.getField(SeqNoFieldMapper.TOMBSTONE_NAME).numericValue().longValue(), equalTo(1L)); - assertThat(noopDoc.getField(SourceFieldMapper.NAME).stringValue(), equalTo(reason)); + assertThat(noopDoc.getField(SourceFieldMapper.NAME).binaryValue(), equalTo(new BytesRef(reason))); closeShards(shard); }