From 24f114766fb1dae17bb94146042d14152386882c 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 78f5a885def41..c6e3f0342bc46 100644 --- a/server/src/main/java/org/elasticsearch/index/engine/NoOpEngine.java +++ b/server/src/main/java/org/elasticsearch/index/engine/NoOpEngine.java @@ -29,6 +29,7 @@ import org.elasticsearch.common.lucene.Lucene; import org.elasticsearch.common.util.concurrent.ReleasableLock; 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; @@ -49,18 +50,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); } @@ -117,7 +119,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(); } @@ -127,6 +129,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 71780773a5bc3..04669f0f32c72 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; @@ -76,7 +74,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; private final CompletionStatsCache completionStatsCache; @@ -119,7 +116,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; @@ -185,24 +181,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)) { @@ -492,11 +470,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 08783dc4a1762..379f702c927ff 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; @@ -103,10 +104,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