From c9f7c37367d307cdb5508e7608539468b0299d62 Mon Sep 17 00:00:00 2001 From: Nick Knize Date: Mon, 21 Feb 2022 16:06:43 -0600 Subject: [PATCH] [Remove] Type Specific Index Stats (#2198) Removes type specific index stats since only one type is allowed. Signed-off-by: Nicholas Walter Knize --- .../java/org/opensearch/client/CrudIT.java | 2 +- .../indices/stats/IndexStatsIT.java | 34 ------------ .../admin/indices/stats/CommonStats.java | 2 +- .../admin/indices/stats/CommonStatsFlags.java | 29 +++------- .../indices/stats/IndicesStatsRequest.java | 17 ------ .../stats/IndicesStatsRequestBuilder.java | 9 --- .../action/update/TransportUpdateAction.java | 2 +- .../opensearch/index/shard/IndexShard.java | 8 +-- .../opensearch/index/shard/IndexingStats.java | 55 ++++--------------- .../index/shard/InternalIndexingStats.java | 54 +----------------- .../admin/cluster/RestNodesStatsAction.java | 3 - .../admin/indices/RestIndicesStatsAction.java | 5 -- 12 files changed, 26 insertions(+), 194 deletions(-) diff --git a/client/rest-high-level/src/test/java/org/opensearch/client/CrudIT.java b/client/rest-high-level/src/test/java/org/opensearch/client/CrudIT.java index 46a75985a936b..f3f087524bc9e 100644 --- a/client/rest-high-level/src/test/java/org/opensearch/client/CrudIT.java +++ b/client/rest-high-level/src/test/java/org/opensearch/client/CrudIT.java @@ -438,7 +438,7 @@ public void testMultiGet() throws IOException { } } - public void testMultiGetWithTypes() throws IOException { + public void testMultiGetWithIds() throws IOException { BulkRequest bulk = new BulkRequest(); bulk.setRefreshPolicy(RefreshPolicy.IMMEDIATE); bulk.add(new IndexRequest("index", "type", "id1").source("{\"field\":\"value1\"}", XContentType.JSON)); diff --git a/server/src/internalClusterTest/java/org/opensearch/indices/stats/IndexStatsIT.java b/server/src/internalClusterTest/java/org/opensearch/indices/stats/IndexStatsIT.java index 6448728076707..cca01a9ec6dcb 100644 --- a/server/src/internalClusterTest/java/org/opensearch/indices/stats/IndexStatsIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/indices/stats/IndexStatsIT.java @@ -1079,40 +1079,6 @@ public void testGroupsParam() throws Exception { } - public void testTypesParam() throws Exception { - createIndex("test1"); - createIndex("test2"); - - ensureGreen(); - - client().prepareIndex("test1", "bar", Integer.toString(1)).setSource("foo", "bar").execute().actionGet(); - client().prepareIndex("test2", "baz", Integer.toString(1)).setSource("foo", "bar").execute().actionGet(); - refresh(); - - IndicesStatsRequestBuilder builder = client().admin().indices().prepareStats(); - IndicesStatsResponse stats = builder.execute().actionGet(); - - assertThat(stats.getTotal().indexing.getTotal().getIndexCount(), greaterThan(0L)); - assertThat(stats.getTotal().indexing.getTypeStats(), is(nullValue())); - - stats = builder.setTypes("bar").execute().actionGet(); - assertThat(stats.getTotal().indexing.getTypeStats().get("bar").getIndexCount(), greaterThan(0L)); - assertThat(stats.getTotal().indexing.getTypeStats().containsKey("baz"), is(false)); - - stats = builder.setTypes("bar", "baz").execute().actionGet(); - assertThat(stats.getTotal().indexing.getTypeStats().get("bar").getIndexCount(), greaterThan(0L)); - assertThat(stats.getTotal().indexing.getTypeStats().get("baz").getIndexCount(), greaterThan(0L)); - - stats = builder.setTypes("*").execute().actionGet(); - assertThat(stats.getTotal().indexing.getTypeStats().get("bar").getIndexCount(), greaterThan(0L)); - assertThat(stats.getTotal().indexing.getTypeStats().get("baz").getIndexCount(), greaterThan(0L)); - - stats = builder.setTypes("*r").execute().actionGet(); - assertThat(stats.getTotal().indexing.getTypeStats().get("bar").getIndexCount(), greaterThan(0L)); - assertThat(stats.getTotal().indexing.getTypeStats().containsKey("baz"), is(false)); - - } - private static void set(Flag flag, IndicesStatsRequestBuilder builder, boolean set) { switch (flag) { case Docs: diff --git a/server/src/main/java/org/opensearch/action/admin/indices/stats/CommonStats.java b/server/src/main/java/org/opensearch/action/admin/indices/stats/CommonStats.java index 015d52f15f907..2949af00a30d0 100644 --- a/server/src/main/java/org/opensearch/action/admin/indices/stats/CommonStats.java +++ b/server/src/main/java/org/opensearch/action/admin/indices/stats/CommonStats.java @@ -190,7 +190,7 @@ public CommonStats(IndicesQueryCache indicesQueryCache, IndexShard indexShard, C store = indexShard.storeStats(); break; case Indexing: - indexing = indexShard.indexingStats(flags.types()); + indexing = indexShard.indexingStats(); break; case Get: get = indexShard.getStats(); diff --git a/server/src/main/java/org/opensearch/action/admin/indices/stats/CommonStatsFlags.java b/server/src/main/java/org/opensearch/action/admin/indices/stats/CommonStatsFlags.java index 6c6c94d84127c..e17b497ce312a 100644 --- a/server/src/main/java/org/opensearch/action/admin/indices/stats/CommonStatsFlags.java +++ b/server/src/main/java/org/opensearch/action/admin/indices/stats/CommonStatsFlags.java @@ -34,6 +34,7 @@ import org.opensearch.LegacyESVersion; import org.opensearch.Version; +import org.opensearch.common.Strings; import org.opensearch.common.io.stream.StreamInput; import org.opensearch.common.io.stream.StreamOutput; import org.opensearch.common.io.stream.Writeable; @@ -48,7 +49,6 @@ public class CommonStatsFlags implements Writeable, Cloneable { public static final CommonStatsFlags NONE = new CommonStatsFlags().clear(); private EnumSet flags = EnumSet.allOf(Flag.class); - private String[] types = null; private String[] groups = null; private String[] fieldDataFields = null; private String[] completionDataFields = null; @@ -75,7 +75,9 @@ public CommonStatsFlags(StreamInput in) throws IOException { flags.add(flag); } } - types = in.readStringArray(); + if (in.getVersion().before(Version.V_2_0_0)) { + in.readStringArray(); + } groups = in.readStringArray(); fieldDataFields = in.readStringArray(); completionDataFields = in.readStringArray(); @@ -97,7 +99,9 @@ public void writeTo(StreamOutput out) throws IOException { } out.writeLong(longFlags); - out.writeStringArrayNullable(types); + if (out.getVersion().before(Version.V_2_0_0)) { + out.writeStringArrayNullable(Strings.EMPTY_ARRAY); + } out.writeStringArrayNullable(groups); out.writeStringArrayNullable(fieldDataFields); out.writeStringArrayNullable(completionDataFields); @@ -116,7 +120,6 @@ public void writeTo(StreamOutput out) throws IOException { */ public CommonStatsFlags all() { flags = EnumSet.allOf(Flag.class); - types = null; groups = null; fieldDataFields = null; completionDataFields = null; @@ -132,7 +135,6 @@ public CommonStatsFlags all() { */ public CommonStatsFlags clear() { flags = EnumSet.noneOf(Flag.class); - types = null; groups = null; fieldDataFields = null; completionDataFields = null; @@ -151,23 +153,6 @@ public Flag[] getFlags() { return flags.toArray(new Flag[flags.size()]); } - /** - * Document types to return stats for. Mainly affects {@link Flag#Indexing} when - * enabled, returning specific indexing stats for those types. - */ - public CommonStatsFlags types(String... types) { - this.types = types; - return this; - } - - /** - * Document types to return stats for. Mainly affects {@link Flag#Indexing} when - * enabled, returning specific indexing stats for those types. - */ - public String[] types() { - return this.types; - } - /** * Sets specific search group stats to retrieve the stats for. Mainly affects search * when enabled. diff --git a/server/src/main/java/org/opensearch/action/admin/indices/stats/IndicesStatsRequest.java b/server/src/main/java/org/opensearch/action/admin/indices/stats/IndicesStatsRequest.java index c5e99119d3cb7..bbe69b700b876 100644 --- a/server/src/main/java/org/opensearch/action/admin/indices/stats/IndicesStatsRequest.java +++ b/server/src/main/java/org/opensearch/action/admin/indices/stats/IndicesStatsRequest.java @@ -90,23 +90,6 @@ public IndicesStatsRequest flags(CommonStatsFlags flags) { return this; } - /** - * Document types to return stats for. Mainly affects {@link #indexing(boolean)} when - * enabled, returning specific indexing stats for those types. - */ - public IndicesStatsRequest types(String... types) { - flags.types(types); - return this; - } - - /** - * Document types to return stats for. Mainly affects {@link #indexing(boolean)} when - * enabled, returning specific indexing stats for those types. - */ - public String[] types() { - return this.flags.types(); - } - /** * Sets specific search group stats to retrieve the stats for. Mainly affects search * when enabled. diff --git a/server/src/main/java/org/opensearch/action/admin/indices/stats/IndicesStatsRequestBuilder.java b/server/src/main/java/org/opensearch/action/admin/indices/stats/IndicesStatsRequestBuilder.java index afb0790367c7f..23c33401966b4 100644 --- a/server/src/main/java/org/opensearch/action/admin/indices/stats/IndicesStatsRequestBuilder.java +++ b/server/src/main/java/org/opensearch/action/admin/indices/stats/IndicesStatsRequestBuilder.java @@ -78,15 +78,6 @@ public final IndicesStatsRequestBuilder setTimeout(TimeValue timeout) { return this; } - /** - * Document types to return stats for. Mainly affects {@link #setIndexing(boolean)} when - * enabled, returning specific indexing stats for those types. - */ - public IndicesStatsRequestBuilder setTypes(String... types) { - request.types(types); - return this; - } - public IndicesStatsRequestBuilder setGroups(String... groups) { request.groups(groups); return this; diff --git a/server/src/main/java/org/opensearch/action/update/TransportUpdateAction.java b/server/src/main/java/org/opensearch/action/update/TransportUpdateAction.java index 475f16cb96ae0..533d4f4918957 100644 --- a/server/src/main/java/org/opensearch/action/update/TransportUpdateAction.java +++ b/server/src/main/java/org/opensearch/action/update/TransportUpdateAction.java @@ -325,7 +325,7 @@ protected void shardOperation(final UpdateRequest request, final ActionListener< if (indexServiceOrNull != null) { IndexShard shard = indexService.getShardOrNull(shardId.getId()); if (shard != null) { - shard.noopUpdate(request.type()); + shard.noopUpdate(); } } listener.onResponse(update); diff --git a/server/src/main/java/org/opensearch/index/shard/IndexShard.java b/server/src/main/java/org/opensearch/index/shard/IndexShard.java index bc4ea006613bb..63238ae5ea02d 100644 --- a/server/src/main/java/org/opensearch/index/shard/IndexShard.java +++ b/server/src/main/java/org/opensearch/index/shard/IndexShard.java @@ -1259,7 +1259,7 @@ public IndexingStats indexingStats(String... types) { throttled = engine.isThrottled(); throttleTimeInMillis = engine.getIndexThrottleTimeInMillis(); } - return internalIndexingStats.stats(throttled, throttleTimeInMillis, types); + return internalIndexingStats.stats(throttled, throttleTimeInMillis); } public SearchStats searchStats(String... groups) { @@ -2847,11 +2847,9 @@ public boolean pendingInSync() { /** * Should be called for each no-op update operation to increment relevant statistics. - * - * @param type the doc type of the update */ - public void noopUpdate(String type) { - internalIndexingStats.noopUpdate(type); + public void noopUpdate() { + internalIndexingStats.noopUpdate(); } public void maybeCheckIndex() { diff --git a/server/src/main/java/org/opensearch/index/shard/IndexingStats.java b/server/src/main/java/org/opensearch/index/shard/IndexingStats.java index 0f64f97a256ee..bdc5373e0b9b3 100644 --- a/server/src/main/java/org/opensearch/index/shard/IndexingStats.java +++ b/server/src/main/java/org/opensearch/index/shard/IndexingStats.java @@ -32,7 +32,7 @@ package org.opensearch.index.shard; -import org.opensearch.common.Nullable; +import org.opensearch.Version; import org.opensearch.common.io.stream.StreamInput; import org.opensearch.common.io.stream.StreamOutput; import org.opensearch.common.io.stream.Writeable; @@ -40,9 +40,9 @@ import org.opensearch.common.xcontent.ToXContent; import org.opensearch.common.xcontent.ToXContentFragment; import org.opensearch.common.xcontent.XContentBuilder; +import org.opensearch.index.mapper.MapperService; import java.io.IOException; -import java.util.HashMap; import java.util.Map; public class IndexingStats implements Writeable, ToXContentFragment { @@ -219,47 +219,30 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws private final Stats totalStats; - @Nullable - private Map typeStats; - public IndexingStats() { totalStats = new Stats(); } public IndexingStats(StreamInput in) throws IOException { totalStats = new Stats(in); - if (in.readBoolean()) { - typeStats = in.readMap(StreamInput::readString, Stats::new); + if (in.getVersion().before(Version.V_2_0_0)) { + if (in.readBoolean()) { + Map typeStats = in.readMap(StreamInput::readString, Stats::new); + assert typeStats.size() == 1; + assert typeStats.containsKey(MapperService.SINGLE_MAPPING_NAME); + } } } - public IndexingStats(Stats totalStats, @Nullable Map typeStats) { + public IndexingStats(Stats totalStats) { this.totalStats = totalStats; - this.typeStats = typeStats; } public void add(IndexingStats indexingStats) { - add(indexingStats, true); - } - - public void add(IndexingStats indexingStats, boolean includeTypes) { if (indexingStats == null) { return; } addTotals(indexingStats); - if (includeTypes && indexingStats.typeStats != null && !indexingStats.typeStats.isEmpty()) { - if (typeStats == null) { - typeStats = new HashMap<>(indexingStats.typeStats.size()); - } - for (Map.Entry entry : indexingStats.typeStats.entrySet()) { - Stats stats = typeStats.get(entry.getKey()); - if (stats == null) { - typeStats.put(entry.getKey(), entry.getValue()); - } else { - stats.add(entry.getValue()); - } - } - } } public void addTotals(IndexingStats indexingStats) { @@ -273,31 +256,16 @@ public Stats getTotal() { return this.totalStats; } - @Nullable - public Map getTypeStats() { - return this.typeStats; - } - @Override public XContentBuilder toXContent(XContentBuilder builder, ToXContent.Params params) throws IOException { builder.startObject(Fields.INDEXING); totalStats.toXContent(builder, params); - if (typeStats != null && !typeStats.isEmpty()) { - builder.startObject(Fields.TYPES); - for (Map.Entry entry : typeStats.entrySet()) { - builder.startObject(entry.getKey()); - entry.getValue().toXContent(builder, params); - builder.endObject(); - } - builder.endObject(); - } builder.endObject(); return builder; } static final class Fields { static final String INDEXING = "indexing"; - static final String TYPES = "types"; static final String INDEX_TOTAL = "index_total"; static final String INDEX_TIME = "index_time"; static final String INDEX_TIME_IN_MILLIS = "index_time_in_millis"; @@ -316,11 +284,8 @@ static final class Fields { @Override public void writeTo(StreamOutput out) throws IOException { totalStats.writeTo(out); - if (typeStats == null || typeStats.isEmpty()) { + if (out.getVersion().before(Version.V_2_0_0)) { out.writeBoolean(false); - } else { - out.writeBoolean(true); - out.writeMap(typeStats, StreamOutput::writeString, (stream, stats) -> stats.writeTo(stream)); } } } diff --git a/server/src/main/java/org/opensearch/index/shard/InternalIndexingStats.java b/server/src/main/java/org/opensearch/index/shard/InternalIndexingStats.java index ac5ae9a59fc66..76d64ab918163 100644 --- a/server/src/main/java/org/opensearch/index/shard/InternalIndexingStats.java +++ b/server/src/main/java/org/opensearch/index/shard/InternalIndexingStats.java @@ -32,56 +32,33 @@ package org.opensearch.index.shard; -import org.opensearch.common.collect.MapBuilder; import org.opensearch.common.metrics.CounterMetric; import org.opensearch.common.metrics.MeanMetric; -import org.opensearch.common.regex.Regex; import org.opensearch.index.engine.Engine; -import java.util.HashMap; -import java.util.Map; import java.util.concurrent.TimeUnit; -import static java.util.Collections.emptyMap; - /** * Internal class that maintains relevant indexing statistics / metrics. * @see IndexShard */ final class InternalIndexingStats implements IndexingOperationListener { private final StatsHolder totalStats = new StatsHolder(); - private volatile Map typesStats = emptyMap(); /** * Returns the stats, including type specific stats. If the types are null/0 length, then nothing * is returned for them. If they are set, then only types provided will be returned, or * {@code _all} for all types. */ - IndexingStats stats(boolean isThrottled, long currentThrottleInMillis, String... types) { + IndexingStats stats(boolean isThrottled, long currentThrottleInMillis) { IndexingStats.Stats total = totalStats.stats(isThrottled, currentThrottleInMillis); - Map typesSt = null; - if (types != null && types.length > 0) { - typesSt = new HashMap<>(typesStats.size()); - if (types.length == 1 && types[0].equals("_all")) { - for (Map.Entry entry : typesStats.entrySet()) { - typesSt.put(entry.getKey(), entry.getValue().stats(isThrottled, currentThrottleInMillis)); - } - } else { - for (Map.Entry entry : typesStats.entrySet()) { - if (Regex.simpleMatch(types, entry.getKey())) { - typesSt.put(entry.getKey(), entry.getValue().stats(isThrottled, currentThrottleInMillis)); - } - } - } - } - return new IndexingStats(total, typesSt); + return new IndexingStats(total); } @Override public Engine.Index preIndex(ShardId shardId, Engine.Index operation) { if (operation.origin().isRecovery() == false) { totalStats.indexCurrent.inc(); - typeStats(operation.type()).indexCurrent.inc(); } return operation; } @@ -94,9 +71,6 @@ public void postIndex(ShardId shardId, Engine.Index index, Engine.IndexResult re long took = result.getTook(); totalStats.indexMetric.inc(took); totalStats.indexCurrent.dec(); - StatsHolder typeStats = typeStats(index.type()); - typeStats.indexMetric.inc(took); - typeStats.indexCurrent.dec(); } break; case FAILURE: @@ -111,9 +85,7 @@ public void postIndex(ShardId shardId, Engine.Index index, Engine.IndexResult re public void postIndex(ShardId shardId, Engine.Index index, Exception ex) { if (!index.origin().isRecovery()) { totalStats.indexCurrent.dec(); - typeStats(index.type()).indexCurrent.dec(); totalStats.indexFailed.inc(); - typeStats(index.type()).indexFailed.inc(); } } @@ -121,7 +93,6 @@ public void postIndex(ShardId shardId, Engine.Index index, Exception ex) { public Engine.Delete preDelete(ShardId shardId, Engine.Delete delete) { if (!delete.origin().isRecovery()) { totalStats.deleteCurrent.inc(); - typeStats(delete.type()).deleteCurrent.inc(); } return delete; @@ -135,9 +106,6 @@ public void postDelete(ShardId shardId, Engine.Delete delete, Engine.DeleteResul long took = result.getTook(); totalStats.deleteMetric.inc(took); totalStats.deleteCurrent.dec(); - StatsHolder typeStats = typeStats(delete.type()); - typeStats.deleteMetric.inc(took); - typeStats.deleteCurrent.dec(); } break; case FAILURE: @@ -152,27 +120,11 @@ public void postDelete(ShardId shardId, Engine.Delete delete, Engine.DeleteResul public void postDelete(ShardId shardId, Engine.Delete delete, Exception ex) { if (!delete.origin().isRecovery()) { totalStats.deleteCurrent.dec(); - typeStats(delete.type()).deleteCurrent.dec(); } } - public void noopUpdate(String type) { + void noopUpdate() { totalStats.noopUpdates.inc(); - typeStats(type).noopUpdates.inc(); - } - - private StatsHolder typeStats(String type) { - StatsHolder stats = typesStats.get(type); - if (stats == null) { - synchronized (this) { - stats = typesStats.get(type); - if (stats == null) { - stats = new StatsHolder(); - typesStats = MapBuilder.newMapBuilder(typesStats).put(type, stats).immutableMap(); - } - } - } - return stats; } static class StatsHolder { diff --git a/server/src/main/java/org/opensearch/rest/action/admin/cluster/RestNodesStatsAction.java b/server/src/main/java/org/opensearch/rest/action/admin/cluster/RestNodesStatsAction.java index 972c5284b382f..3c66b0740536f 100644 --- a/server/src/main/java/org/opensearch/rest/action/admin/cluster/RestNodesStatsAction.java +++ b/server/src/main/java/org/opensearch/rest/action/admin/cluster/RestNodesStatsAction.java @@ -193,9 +193,6 @@ public RestChannelConsumer prepareRequest(final RestRequest request, final NodeC if (nodesStatsRequest.indices().isSet(Flag.Search) && (request.hasParam("groups"))) { nodesStatsRequest.indices().groups(request.paramAsStringArray("groups", null)); } - if (nodesStatsRequest.indices().isSet(Flag.Indexing) && (request.hasParam("types"))) { - nodesStatsRequest.indices().types(request.paramAsStringArray("types", null)); - } if (nodesStatsRequest.indices().isSet(Flag.Segments)) { nodesStatsRequest.indices().includeSegmentFileSizes(request.paramAsBoolean("include_segment_file_sizes", false)); } diff --git a/server/src/main/java/org/opensearch/rest/action/admin/indices/RestIndicesStatsAction.java b/server/src/main/java/org/opensearch/rest/action/admin/indices/RestIndicesStatsAction.java index 696bff33a73a4..eabe14a7614ac 100644 --- a/server/src/main/java/org/opensearch/rest/action/admin/indices/RestIndicesStatsAction.java +++ b/server/src/main/java/org/opensearch/rest/action/admin/indices/RestIndicesStatsAction.java @@ -102,7 +102,6 @@ public RestChannelConsumer prepareRequest(final RestRequest request, final NodeC + "options changed"; indicesStatsRequest.indicesOptions(IndicesOptions.fromRequest(request, defaultIndicesOption)); indicesStatsRequest.indices(Strings.splitStringByCommaToArray(request.param("index"))); - indicesStatsRequest.types(Strings.splitStringByCommaToArray(request.param("types"))); Set metrics = Strings.tokenizeByCommaToSet(request.param("metric", "_all")); // short cut, if no metrics have been specified in URI @@ -139,10 +138,6 @@ public RestChannelConsumer prepareRequest(final RestRequest request, final NodeC indicesStatsRequest.groups(Strings.splitStringByCommaToArray(request.param("groups"))); } - if (request.hasParam("types")) { - indicesStatsRequest.types(Strings.splitStringByCommaToArray(request.param("types"))); - } - if (indicesStatsRequest.completion() && (request.hasParam("fields") || request.hasParam("completion_fields"))) { indicesStatsRequest.completionFields( request.paramAsStringArray("completion_fields", request.paramAsStringArray("fields", Strings.EMPTY_ARRAY))