From a39ad3a3c5ed0b93c1a47e558a36093b961b9af4 Mon Sep 17 00:00:00 2001 From: Simon Willnauer Date: Tue, 5 Mar 2019 14:07:46 +0100 Subject: [PATCH 1/3] Return cached segments stats if `include_unloaded_segments` is true Today we don't return segments stats for closed indices which makes it hard to tell how much memory such an index would require. With this change we return the statistics if requested by setting `include_unloaded_segments` to true on the rest request. Relates to #39512/ --- .../index/engine/NoOpEngine.java | 42 ++++++++++++++++--- .../index/engine/NoOpEngineTests.java | 12 +++++- 2 files changed, 47 insertions(+), 7 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 fe1ad7a1a144f..a41f07e994b93 100644 --- a/server/src/main/java/org/elasticsearch/index/engine/NoOpEngine.java +++ b/server/src/main/java/org/elasticsearch/index/engine/NoOpEngine.java @@ -23,9 +23,13 @@ import org.apache.lucene.index.IndexCommit; import org.apache.lucene.index.IndexWriter; import org.apache.lucene.index.LeafReader; +import org.apache.lucene.index.LeafReaderContext; +import org.apache.lucene.index.SegmentReader; import org.apache.lucene.store.Directory; +import org.elasticsearch.common.lucene.Lucene; import java.io.IOException; +import java.io.UncheckedIOException; import java.util.List; import java.util.function.Function; @@ -36,8 +40,20 @@ */ public final class NoOpEngine extends ReadOnlyEngine { + private final SegmentsStats stats; + public NoOpEngine(EngineConfig config) { super(config, null, null, true, Function.identity()); + this.stats = new SegmentsStats(); + Directory directory = store.directory(); + try (DirectoryReader reader = DirectoryReader.open(directory)) { + for (LeafReaderContext ctx : reader.getContext().leaves()) { + SegmentReader segmentReader = Lucene.segmentReader(ctx.reader()); + fillSegmentStats(segmentReader, true, stats); + } + } catch (IOException e) { + throw new UncheckedIOException(e); + } } @Override @@ -47,17 +63,17 @@ protected DirectoryReader open(final IndexCommit commit) throws IOException { final IndexCommit indexCommit = indexCommits.get(indexCommits.size() - 1); return new DirectoryReader(directory, new LeafReader[0]) { @Override - protected DirectoryReader doOpenIfChanged() throws IOException { + protected DirectoryReader doOpenIfChanged() { return null; } @Override - protected DirectoryReader doOpenIfChanged(IndexCommit commit) throws IOException { + protected DirectoryReader doOpenIfChanged(IndexCommit commit) { return null; } @Override - protected DirectoryReader doOpenIfChanged(IndexWriter writer, boolean applyAllDeletes) throws IOException { + protected DirectoryReader doOpenIfChanged(IndexWriter writer, boolean applyAllDeletes) { return null; } @@ -67,17 +83,17 @@ public long getVersion() { } @Override - public boolean isCurrent() throws IOException { + public boolean isCurrent() { return true; } @Override - public IndexCommit getIndexCommit() throws IOException { + public IndexCommit getIndexCommit() { return indexCommit; } @Override - protected void doClose() throws IOException { + protected void doClose() { } @Override @@ -86,4 +102,18 @@ public CacheHelper getReaderCacheHelper() { } }; } + + @Override + public SegmentsStats segmentsStats(boolean includeSegmentFileSizes, boolean includeUnloadedSegments) { + if (includeUnloadedSegments) { + final SegmentsStats stats = new SegmentsStats(); + stats.add(this.stats); + if (includeSegmentFileSizes == false) { + stats.clearFileSizes(); + } + return stats; + } else { + return super.segmentsStats(includeSegmentFileSizes, includeUnloadedSegments); + } + } } 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 b70ccf03aacaf..3a857a2046842 100644 --- a/server/src/test/java/org/elasticsearch/index/engine/NoOpEngineTests.java +++ b/server/src/test/java/org/elasticsearch/index/engine/NoOpEngineTests.java @@ -100,7 +100,7 @@ public void testNoopAfterRegularEngine() throws IOException { noOpEngine.close(); } - public void testNoOpEngineDocStats() throws Exception { + public void testNoOpEngineStats() throws Exception { IOUtils.close(engine, store); final AtomicLong globalCheckpoint = new AtomicLong(SequenceNumbers.NO_OPS_PERFORMED); try (Store store = createStore()) { @@ -131,8 +131,11 @@ public void testNoOpEngineDocStats() throws Exception { } final DocsStats expectedDocStats; + boolean includeFileSize = randomBoolean(); + final SegmentsStats expectedSegmentStats; try (InternalEngine engine = createEngine(config)) { expectedDocStats = engine.docStats(); + expectedSegmentStats = engine.segmentsStats(includeFileSize, true); } try (NoOpEngine noOpEngine = new NoOpEngine(config)) { @@ -140,6 +143,13 @@ public void testNoOpEngineDocStats() throws Exception { assertEquals(expectedDocStats.getDeleted(), noOpEngine.docStats().getDeleted()); assertEquals(expectedDocStats.getTotalSizeInBytes(), noOpEngine.docStats().getTotalSizeInBytes()); assertEquals(expectedDocStats.getAverageSizeInBytes(), noOpEngine.docStats().getAverageSizeInBytes()); + assertEquals(expectedSegmentStats.getCount(), noOpEngine.segmentsStats(includeFileSize, true).getCount()); + assertEquals(expectedSegmentStats.getMemoryInBytes(), noOpEngine.segmentsStats(includeFileSize, true).getMemoryInBytes()); + assertEquals(expectedSegmentStats.getFileSizes().size(), + noOpEngine.segmentsStats(includeFileSize, true).getFileSizes().size()); + + assertEquals(0, noOpEngine.segmentsStats(includeFileSize, false).getFileSizes().size()); + assertEquals(0, noOpEngine.segmentsStats(includeFileSize, false).getMemoryInBytes()); } catch (AssertionError e) { logger.error(config.getMergePolicy()); throw e; From 31844b6873deb2c1cc4d36e56c306be641752e09 Mon Sep 17 00:00:00 2001 From: Simon Willnauer Date: Sun, 10 Mar 2019 16:45:32 +0100 Subject: [PATCH 2/3] add test and allow indices stats on closed indices --- .../rest-api-spec/api/indices.stats.json | 11 ++++ .../test/indices.stats/30_segments.yml | 57 +++++++++++++++++++ .../indices/stats/IndicesStatsRequest.java | 5 ++ .../admin/indices/RestIndicesStatsAction.java | 9 ++- .../rest/action/cat/RestIndicesAction.java | 2 +- 5 files changed, 81 insertions(+), 3 deletions(-) create mode 100644 rest-api-spec/src/main/resources/rest-api-spec/test/indices.stats/30_segments.yml diff --git a/rest-api-spec/src/main/resources/rest-api-spec/api/indices.stats.json b/rest-api-spec/src/main/resources/rest-api-spec/api/indices.stats.json index 270c379baa709..c86a2c1147a9b 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/api/indices.stats.json +++ b/rest-api-spec/src/main/resources/rest-api-spec/api/indices.stats.json @@ -57,6 +57,17 @@ "type": "boolean", "description": "If set to true segment stats will include stats for segments that are not currently loaded into memory", "default": false + }, + "expand_wildcards": { + "type" : "enum", + "options" : ["open","closed","none","all"], + "default" : "open", + "description" : "Whether to expand wildcard expression to concrete indices that are open, closed or both." + }, + "forbid_closed_indices": { + "type": "boolean", + "description": "If set to false stats will also collected from closed indices if explicitly specified or if expand_wildcards expands to closed indices", + "default": true } } }, diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/indices.stats/30_segments.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/indices.stats/30_segments.yml new file mode 100644 index 0000000000000..37bdbeb41516b --- /dev/null +++ b/rest-api-spec/src/main/resources/rest-api-spec/test/indices.stats/30_segments.yml @@ -0,0 +1,57 @@ +--- +setup: + - do: + indices.create: + index: test + body: + settings: + number_of_shards: 1 + number_of_replicas: 0 + + - do: + cluster.health: + wait_for_no_initializing_shards: true + +--- +"Segment Stats": + - do: + indices.stats: + metric: [ segments ] + - set: { indices.test.primaries.segments.count: num_segments } + + - do: + index: + index: test + id: 1 + body: { "foo": "bar" } + + - do: + indices.flush: + index: test + + - do: + indices.stats: + metric: [ segments ] + - gt: { indices.test.primaries.segments.count: $num_segments } + - set: { indices.test.primaries.segments.count: num_segments_after_flush } + + - do: + indices.close: + index: test + + - do: + indices.stats: + metric: segments + expand_wildcards: closed + forbid_closed_indices: false + + - match: { indices.test.primaries.segments.count: 0 } + + - do: + indices.stats: + metric: segments + include_unloaded_segments: true + expand_wildcards: closed + forbid_closed_indices: false + + - match: { indices.test.primaries.segments.count: $num_segments_after_flush } diff --git a/server/src/main/java/org/elasticsearch/action/admin/indices/stats/IndicesStatsRequest.java b/server/src/main/java/org/elasticsearch/action/admin/indices/stats/IndicesStatsRequest.java index ebc085ef47e60..b162a23258591 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/indices/stats/IndicesStatsRequest.java +++ b/server/src/main/java/org/elasticsearch/action/admin/indices/stats/IndicesStatsRequest.java @@ -271,6 +271,11 @@ public IndicesStatsRequest includeSegmentFileSizes(boolean includeSegmentFileSiz return this; } + public IndicesStatsRequest includeUnloadedSegments(boolean includeUnloadedSegments) { + flags.includeUnloadedSegments(includeUnloadedSegments); + return this; + } + @Override public void writeTo(StreamOutput out) throws IOException { super.writeTo(out); diff --git a/server/src/main/java/org/elasticsearch/rest/action/admin/indices/RestIndicesStatsAction.java b/server/src/main/java/org/elasticsearch/rest/action/admin/indices/RestIndicesStatsAction.java index 85d9dc29ea83a..c2d16ce5ac687 100644 --- a/server/src/main/java/org/elasticsearch/rest/action/admin/indices/RestIndicesStatsAction.java +++ b/server/src/main/java/org/elasticsearch/rest/action/admin/indices/RestIndicesStatsAction.java @@ -69,7 +69,12 @@ public String getName() { @Override public RestChannelConsumer prepareRequest(final RestRequest request, final NodeClient client) throws IOException { IndicesStatsRequest indicesStatsRequest = new IndicesStatsRequest(); - indicesStatsRequest.indicesOptions(IndicesOptions.fromRequest(request, indicesStatsRequest.indicesOptions())); + boolean forbidClosedIndices = request.paramAsBoolean("forbid_closed_indices", true); + IndicesOptions defaultIndicesOption = forbidClosedIndices ? indicesStatsRequest.indicesOptions() + : IndicesOptions.strictExpandOpen(); + assert indicesStatsRequest.indicesOptions() == IndicesOptions.strictExpandOpenAndForbidClosed() : "IndicesStats default indices " + + "options changed"; + indicesStatsRequest.indicesOptions(IndicesOptions.fromRequest(request, defaultIndicesOption)); indicesStatsRequest.indices(Strings.splitStringByCommaToArray(request.param("index"))); indicesStatsRequest.types(Strings.splitStringByCommaToArray(request.param("types"))); @@ -121,7 +126,7 @@ public RestChannelConsumer prepareRequest(final RestRequest request, final NodeC if (indicesStatsRequest.segments()) { indicesStatsRequest.includeSegmentFileSizes(request.paramAsBoolean("include_segment_file_sizes", false)); - indicesStatsRequest.includeSegmentFileSizes(request.paramAsBoolean("include_unloaded_segments", false)); + indicesStatsRequest.includeUnloadedSegments(request.paramAsBoolean("include_unloaded_segments", false)); } return channel -> client.admin().indices().stats(indicesStatsRequest, new RestToXContentListener<>(channel)); diff --git a/server/src/main/java/org/elasticsearch/rest/action/cat/RestIndicesAction.java b/server/src/main/java/org/elasticsearch/rest/action/cat/RestIndicesAction.java index e28b85c62993e..fb560cf7f77aa 100644 --- a/server/src/main/java/org/elasticsearch/rest/action/cat/RestIndicesAction.java +++ b/server/src/main/java/org/elasticsearch/rest/action/cat/RestIndicesAction.java @@ -116,7 +116,7 @@ public void processResponse(final ClusterHealthResponse clusterHealthResponse) { indicesStatsRequest.indices(indices); indicesStatsRequest.indicesOptions(strictExpandIndicesOptions); indicesStatsRequest.all(); - indicesStatsRequest.includeSegmentFileSizes(request.paramAsBoolean("include_unloaded_segments", false)); + indicesStatsRequest.includeUnloadedSegments(request.paramAsBoolean("include_unloaded_segments", false)); client.admin().indices().stats(indicesStatsRequest, new RestResponseListener(channel) { @Override From d734371a0a155904c7bbdd6a5453e98e579eb280 Mon Sep 17 00:00:00 2001 From: Simon Willnauer Date: Sun, 10 Mar 2019 22:27:53 +0100 Subject: [PATCH 3/3] skip bwc test --- .../rest-api-spec/test/indices.stats/30_segments.yml | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/indices.stats/30_segments.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/indices.stats/30_segments.yml index 37bdbeb41516b..49bbf924cb690 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/test/indices.stats/30_segments.yml +++ b/rest-api-spec/src/main/resources/rest-api-spec/test/indices.stats/30_segments.yml @@ -14,6 +14,11 @@ setup: --- "Segment Stats": + + - skip: + version: " - 7.99.99" + reason: forbid_closed_indices is not supported in ealier version + - do: indices.stats: metric: [ segments ]