From 8b00fd0be5d943646a18fec3d73fd4aa10e7bd64 Mon Sep 17 00:00:00 2001 From: Nhat Nguyen Date: Thu, 23 Jan 2020 08:26:27 -0500 Subject: [PATCH] Do not wrap soft-deletes reader for segment stats (#51331) IndexWriter might not filter out fully deleted segments if retention leases exist or the number of the retaining operations is non-zero. SoftDeletesDirectoryReaderWrapper, however, always filters out fully deleted segments. This change uses the original directory reader when calculating segment stats instead. Relates #51192 Closes #51303 --- .../org/elasticsearch/index/engine/NoOpEngine.java | 3 ++- .../elasticsearch/index/engine/ReadOnlyEngine.java | 7 +++---- .../elasticsearch/index/engine/FrozenEngine.java | 13 +++++++++---- 3 files changed, 14 insertions(+), 9 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/index/engine/NoOpEngine.java b/server/src/main/java/org/elasticsearch/index/engine/NoOpEngine.java index 89dee26cc2895..06520d3036c31 100644 --- a/server/src/main/java/org/elasticsearch/index/engine/NoOpEngine.java +++ b/server/src/main/java/org/elasticsearch/index/engine/NoOpEngine.java @@ -54,7 +54,8 @@ public NoOpEngine(EngineConfig config) { super(config, null, null, true, Function.identity()); this.stats = new SegmentsStats(); Directory directory = store.directory(); - try (DirectoryReader reader = openDirectory(directory, config.getIndexSettings())) { + // Do not wrap soft-deletes reader when calculating segment stats as the wrapper might filter out fully deleted segments. + try (DirectoryReader reader = openDirectory(directory, false)) { for (LeafReaderContext ctx : reader.getContext().leaves()) { SegmentReader segmentReader = Lucene.segmentReader(ctx.reader()); fillSegmentStats(segmentReader, true, stats); diff --git a/server/src/main/java/org/elasticsearch/index/engine/ReadOnlyEngine.java b/server/src/main/java/org/elasticsearch/index/engine/ReadOnlyEngine.java index c294e5dd732b4..2ffdcb71425eb 100644 --- a/server/src/main/java/org/elasticsearch/index/engine/ReadOnlyEngine.java +++ b/server/src/main/java/org/elasticsearch/index/engine/ReadOnlyEngine.java @@ -33,7 +33,6 @@ 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.IndexSettings; import org.elasticsearch.index.mapper.MapperService; import org.elasticsearch.index.seqno.SeqNoStats; import org.elasticsearch.index.seqno.SequenceNumbers; @@ -536,9 +535,9 @@ public void advanceMaxSeqNoOfUpdatesOrDeletes(long maxSeqNoOfUpdatesOnPrimary) { maxSeqNoOfUpdatesOnPrimary + ">" + getMaxSeqNoOfUpdatesOrDeletes(); } - protected DirectoryReader openDirectory(Directory dir, IndexSettings indexSettings) throws IOException { - final DirectoryReader reader = DirectoryReader.open(dir, OFF_HEAP_READER_ATTRIBUTES); - if (indexSettings.isSoftDeleteEnabled()) { + protected static DirectoryReader openDirectory(Directory directory, boolean wrapSoftDeletes) throws IOException { + final DirectoryReader reader = DirectoryReader.open(directory, OFF_HEAP_READER_ATTRIBUTES); + if (wrapSoftDeletes) { return new SoftDeletesDirectoryReaderWrapper(reader, Lucene.SOFT_DELETES_FIELD); } else { return reader; diff --git a/x-pack/plugin/frozen-indices/src/main/java/org/elasticsearch/index/engine/FrozenEngine.java b/x-pack/plugin/frozen-indices/src/main/java/org/elasticsearch/index/engine/FrozenEngine.java index 05b6054837c82..ba57fa827a88e 100644 --- a/x-pack/plugin/frozen-indices/src/main/java/org/elasticsearch/index/engine/FrozenEngine.java +++ b/x-pack/plugin/frozen-indices/src/main/java/org/elasticsearch/index/engine/FrozenEngine.java @@ -21,6 +21,7 @@ import org.apache.lucene.index.PointValues; import org.apache.lucene.index.SegmentCommitInfo; import org.apache.lucene.index.SegmentReader; +import org.apache.lucene.index.SoftDeletesDirectoryReaderWrapper; import org.apache.lucene.index.SortedDocValues; import org.apache.lucene.index.SortedNumericDocValues; import org.apache.lucene.index.SortedSetDocValues; @@ -77,9 +78,8 @@ public FrozenEngine(EngineConfig config) { boolean success = false; Directory directory = store.directory(); - try (DirectoryReader reader = openDirectory(directory, config.getIndexSettings())) { - canMatchReader = ElasticsearchDirectoryReader.wrap(new RewriteCachingDirectoryReader(directory, reader.leaves()), - config.getShardId()); + // Do not wrap soft-deletes reader when calculating segment stats as the wrapper might filter out fully deleted segments. + try (DirectoryReader reader = openDirectory(directory, false)) { // we record the segment stats here - that's what the reader needs when it's open and it give the user // an idea of what it can save when it's closed this.stats = new SegmentsStats(); @@ -87,6 +87,10 @@ public FrozenEngine(EngineConfig config) { SegmentReader segmentReader = Lucene.segmentReader(ctx.reader()); fillSegmentStats(segmentReader, true, stats); } + final DirectoryReader wrappedReader = config.getIndexSettings().isSoftDeleteEnabled() ? + new SoftDeletesDirectoryReaderWrapper(reader, Lucene.SOFT_DELETES_FIELD) : reader; + canMatchReader = ElasticsearchDirectoryReader.wrap( + new RewriteCachingDirectoryReader(directory, wrappedReader.leaves()), config.getShardId()); success = true; } catch (IOException e) { throw new UncheckedIOException(e); @@ -167,7 +171,8 @@ private synchronized ElasticsearchDirectoryReader getOrOpenReader() throws IOExc for (ReferenceManager.RefreshListener listeners : config ().getInternalRefreshListener()) { listeners.beforeRefresh(); } - final DirectoryReader dirReader = openDirectory(engineConfig.getStore().directory(), engineConfig.getIndexSettings()); + final DirectoryReader dirReader = openDirectory(engineConfig.getStore().directory(), + engineConfig.getIndexSettings().isSoftDeleteEnabled()); reader = lastOpenedReader = wrapReader(dirReader, Function.identity()); processReader(reader); reader.getReaderCacheHelper().addClosedListener(this::onReaderClosed);