From 4ceabfb42c016309d388aeacdf88c2e146787aef Mon Sep 17 00:00:00 2001 From: Nhat Nguyen Date: Tue, 10 Mar 2020 19:17:39 -0400 Subject: [PATCH] Fix doc_stats and segment_stats of ReadOnlyEngine (#53345) We can't always have the same segment stats and doc stats between InternalEngine and ReadOnlyEngine if there are some fully deleted segments. ReadOnlyEngine always filters out them. InternalEngine, however, will keep them if peer recovery retention leases exist or the number of the retaining operations is non-zero. This change reverts the fix in #51331 and uses the wrapped reader to calculate the segment stats and doc stats. For the test, we need to disable the extra retaining soft-deletes operations. Closes #51303 --- .../index/engine/NoOpEngine.java | 19 ++++++++----- .../index/engine/ReadOnlyEngine.java | 27 ------------------- .../index/engine/NoOpEngineTests.java | 9 ++++++- .../index/engine/FrozenEngine.java | 27 +++++++++++-------- 4 files changed, 37 insertions(+), 45 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 06520d3036c31..6ca80f64ed5c7 100644 --- a/server/src/main/java/org/elasticsearch/index/engine/NoOpEngine.java +++ b/server/src/main/java/org/elasticsearch/index/engine/NoOpEngine.java @@ -28,6 +28,7 @@ import org.apache.lucene.store.Directory; import org.elasticsearch.common.lucene.Lucene; import org.elasticsearch.common.util.concurrent.ReleasableLock; +import org.elasticsearch.index.shard.DocsStats; import org.elasticsearch.index.store.Store; import org.elasticsearch.index.translog.Translog; import org.elasticsearch.index.translog.TranslogConfig; @@ -48,18 +49,19 @@ */ public final class NoOpEngine extends ReadOnlyEngine { - private final SegmentsStats stats; + private final SegmentsStats segmentsStats; + private final DocsStats docsStats; public NoOpEngine(EngineConfig config) { super(config, null, null, true, Function.identity()); - this.stats = new SegmentsStats(); + this.segmentsStats = new SegmentsStats(); Directory directory = store.directory(); - // 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)) { + try (DirectoryReader reader = openDirectory(directory, config.getIndexSettings().isSoftDeleteEnabled())) { for (LeafReaderContext ctx : reader.getContext().leaves()) { SegmentReader segmentReader = Lucene.segmentReader(ctx.reader()); - fillSegmentStats(segmentReader, true, stats); + fillSegmentStats(segmentReader, true, segmentsStats); } + this.docsStats = docsStats(reader); } catch (IOException e) { throw new UncheckedIOException(e); } @@ -116,7 +118,7 @@ public CacheHelper getReaderCacheHelper() { public SegmentsStats segmentsStats(boolean includeSegmentFileSizes, boolean includeUnloadedSegments) { if (includeUnloadedSegments) { final SegmentsStats stats = new SegmentsStats(); - stats.add(this.stats); + stats.add(this.segmentsStats); if (includeSegmentFileSizes == false) { stats.clearFileSizes(); } @@ -126,6 +128,11 @@ public SegmentsStats segmentsStats(boolean includeSegmentFileSizes, boolean incl } } + @Override + public DocsStats docStats() { + return docsStats; + } + /** * This implementation will trim existing translog files using a {@link TranslogDeletionPolicy} * that retains nothing but the last translog generation from safe commit. 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 2ffdcb71425eb..2ad93a5a6ea26 100644 --- a/server/src/main/java/org/elasticsearch/index/engine/ReadOnlyEngine.java +++ b/server/src/main/java/org/elasticsearch/index/engine/ReadOnlyEngine.java @@ -22,7 +22,6 @@ import org.apache.lucene.index.DirectoryReader; import org.apache.lucene.index.IndexCommit; import org.apache.lucene.index.IndexWriter; -import org.apache.lucene.index.SegmentCommitInfo; import org.apache.lucene.index.SegmentInfos; import org.apache.lucene.index.SoftDeletesDirectoryReaderWrapper; import org.apache.lucene.search.ReferenceManager; @@ -36,7 +35,6 @@ import org.elasticsearch.index.mapper.MapperService; import org.elasticsearch.index.seqno.SeqNoStats; import org.elasticsearch.index.seqno.SequenceNumbers; -import org.elasticsearch.index.shard.DocsStats; import org.elasticsearch.index.store.Store; import org.elasticsearch.index.translog.Translog; import org.elasticsearch.index.translog.TranslogConfig; @@ -75,7 +73,6 @@ public class ReadOnlyEngine extends Engine { private final ElasticsearchReaderManager readerManager; private final IndexCommit indexCommit; private final Lock indexWriterLock; - private final DocsStats docsStats; private final RamAccountingRefreshListener refreshListener; private final SafeCommitInfo safeCommitInfo; @@ -117,7 +114,6 @@ public ReadOnlyEngine(EngineConfig config, SeqNoStats seqNoStats, TranslogStats this.indexCommit = Lucene.getIndexCommit(lastCommittedSegmentInfos, directory); reader = wrapReader(open(indexCommit), readerWrapperFunction); readerManager = new ElasticsearchReaderManager(reader, refreshListener); - this.docsStats = docsStats(lastCommittedSegmentInfos); assert translogStats != null || obtainLock : "mutiple translogs instances should not be opened at the same time"; this.translogStats = translogStats != null ? translogStats : translogStats(config, lastCommittedSegmentInfos); this.indexWriterLock = indexWriterLock; @@ -179,24 +175,6 @@ protected DirectoryReader open(IndexCommit commit) throws IOException { return DirectoryReader.open(commit, OFF_HEAP_READER_ATTRIBUTES); } - private DocsStats docsStats(final SegmentInfos lastCommittedSegmentInfos) { - long numDocs = 0; - long numDeletedDocs = 0; - long sizeInBytes = 0; - if (lastCommittedSegmentInfos != null) { - for (SegmentCommitInfo segmentCommitInfo : lastCommittedSegmentInfos) { - numDocs += segmentCommitInfo.info.maxDoc() - segmentCommitInfo.getDelCount() - segmentCommitInfo.getSoftDelCount(); - numDeletedDocs += segmentCommitInfo.getDelCount() + segmentCommitInfo.getSoftDelCount(); - try { - sizeInBytes += segmentCommitInfo.sizeInBytes(); - } catch (IOException e) { - throw new UncheckedIOException("Failed to get size for [" + segmentCommitInfo.info.name + "]", e); - } - } - } - return new DocsStats(numDocs, numDeletedDocs, sizeInBytes); - } - @Override protected void closeNoLock(String reason, CountDownLatch closedLatch) { if (isClosed.compareAndSet(false, true)) { @@ -487,11 +465,6 @@ public void trimOperationsFromTranslog(long belowTerm, long aboveSeqNo) { public void maybePruneDeletes() { } - @Override - public DocsStats docStats() { - return docsStats; - } - @Override public void updateMaxUnsafeAutoIdTimestamp(long newTimestamp) { diff --git a/server/src/test/java/org/elasticsearch/index/engine/NoOpEngineTests.java b/server/src/test/java/org/elasticsearch/index/engine/NoOpEngineTests.java index 623bbe0ec50db..1a3e1be29b3c1 100644 --- a/server/src/test/java/org/elasticsearch/index/engine/NoOpEngineTests.java +++ b/server/src/test/java/org/elasticsearch/index/engine/NoOpEngineTests.java @@ -23,6 +23,7 @@ import org.apache.lucene.index.IndexReader; import org.apache.lucene.index.NoMergePolicy; import org.apache.lucene.store.LockObtainFailedException; +import org.elasticsearch.cluster.metadata.IndexMetaData; import org.elasticsearch.cluster.routing.IndexShardRoutingTable; import org.elasticsearch.cluster.routing.ShardRouting; import org.elasticsearch.cluster.routing.ShardRoutingState; @@ -104,10 +105,16 @@ public void testNoopAfterRegularEngine() throws IOException { public void testNoOpEngineStats() throws Exception { IOUtils.close(engine, store); + Settings.Builder settings = Settings.builder() + .put(defaultSettings.getSettings()) + .put(IndexSettings.INDEX_SOFT_DELETES_RETENTION_OPERATIONS_SETTING.getKey(), 0); + IndexSettings indexSettings = IndexSettingsModule.newIndexSettings( + IndexMetaData.builder(defaultSettings.getIndexMetaData()).settings(settings).build()); + final AtomicLong globalCheckpoint = new AtomicLong(SequenceNumbers.NO_OPS_PERFORMED); try (Store store = createStore()) { Path translogPath = createTempDir(); - EngineConfig config = config(defaultSettings, store, translogPath, NoMergePolicy.INSTANCE, null, null, globalCheckpoint::get); + EngineConfig config = config(indexSettings, store, translogPath, NoMergePolicy.INSTANCE, null, null, globalCheckpoint::get); final int numDocs = scaledRandomIntBetween(10, 3000); int deletions = 0; try (InternalEngine engine = createEngine(config)) { 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 ba57fa827a88e..c5d2849dd9da6 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 @@ -36,6 +36,7 @@ import org.elasticsearch.common.lucene.index.ElasticsearchDirectoryReader; import org.elasticsearch.common.settings.Setting; import org.elasticsearch.core.internal.io.IOUtils; +import org.elasticsearch.index.shard.DocsStats; import org.elasticsearch.index.shard.SearchOperationListener; import org.elasticsearch.search.internal.SearchContext; import org.elasticsearch.transport.TransportRequest; @@ -69,7 +70,8 @@ public final class FrozenEngine extends ReadOnlyEngine { public static final Setting INDEX_FROZEN = Setting.boolSetting("index.frozen", false, Setting.Property.IndexScope, Setting.Property.PrivateIndex); - private final SegmentsStats stats; + private final SegmentsStats segmentsStats; + private final DocsStats docsStats; private volatile ElasticsearchDirectoryReader lastOpenedReader; private final ElasticsearchDirectoryReader canMatchReader; @@ -78,17 +80,16 @@ public FrozenEngine(EngineConfig config) { boolean success = false; Directory directory = store.directory(); - // 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 + try (DirectoryReader reader = openDirectory(directory, config.getIndexSettings().isSoftDeleteEnabled())) { + // we record the segment stats and doc 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(); + this.segmentsStats = new SegmentsStats(); for (LeafReaderContext ctx : reader.getContext().leaves()) { SegmentReader segmentReader = Lucene.segmentReader(ctx.reader()); - fillSegmentStats(segmentReader, true, stats); + fillSegmentStats(segmentReader, true, segmentsStats); } - final DirectoryReader wrappedReader = config.getIndexSettings().isSoftDeleteEnabled() ? - new SoftDeletesDirectoryReaderWrapper(reader, Lucene.SOFT_DELETES_FIELD) : reader; + this.docsStats = docsStats(reader); + final DirectoryReader wrappedReader = new SoftDeletesDirectoryReaderWrapper(reader, Lucene.SOFT_DELETES_FIELD); canMatchReader = ElasticsearchDirectoryReader.wrap( new RewriteCachingDirectoryReader(directory, wrappedReader.leaves()), config.getShardId()); success = true; @@ -171,8 +172,7 @@ private synchronized ElasticsearchDirectoryReader getOrOpenReader() throws IOExc for (ReferenceManager.RefreshListener listeners : config ().getInternalRefreshListener()) { listeners.beforeRefresh(); } - final DirectoryReader dirReader = openDirectory(engineConfig.getStore().directory(), - engineConfig.getIndexSettings().isSoftDeleteEnabled()); + final DirectoryReader dirReader = openDirectory(store.directory(), engineConfig.getIndexSettings().isSoftDeleteEnabled()); reader = lastOpenedReader = wrapReader(dirReader, Function.identity()); processReader(reader); reader.getReaderCacheHelper().addClosedListener(this::onReaderClosed); @@ -596,7 +596,7 @@ public LeafReader getDelegate() { public SegmentsStats segmentsStats(boolean includeSegmentFileSizes, boolean includeUnloadedSegments) { if (includeUnloadedSegments) { final SegmentsStats stats = new SegmentsStats(); - stats.add(this.stats); + stats.add(this.segmentsStats); if (includeSegmentFileSizes == false) { stats.clearFileSizes(); } @@ -607,6 +607,11 @@ public SegmentsStats segmentsStats(boolean includeSegmentFileSizes, boolean incl } + @Override + public DocsStats docStats() { + return docsStats; + } + synchronized boolean isReaderOpen() { return lastOpenedReader != null; } // this is mainly for tests