From f6d3b5827d59e14fd44be6bd393906b602c66a47 Mon Sep 17 00:00:00 2001 From: Pranshu Shukla Date: Thu, 27 Jun 2024 10:32:46 +0530 Subject: [PATCH 01/25] Optimize NodeIndicesStats output behind flag Signed-off-by: Pranshu Shukla --- .../org/opensearch/nodestats/NodeStatsIT.java | 45 ++++++ .../cluster/node/stats/NodesStatsRequest.java | 10 ++ .../admin/indices/stats/CommonStatsFlags.java | 16 ++ .../opensearch/indices/IndicesService.java | 5 +- .../opensearch/indices/NodeIndicesStats.java | 142 ++++++++++++++++-- .../admin/cluster/RestNodesStatsAction.java | 1 + .../rest/action/cat/RestNodesAction.java | 2 +- 7 files changed, 204 insertions(+), 17 deletions(-) diff --git a/server/src/internalClusterTest/java/org/opensearch/nodestats/NodeStatsIT.java b/server/src/internalClusterTest/java/org/opensearch/nodestats/NodeStatsIT.java index f270cb1399072..8f7bdc832a126 100644 --- a/server/src/internalClusterTest/java/org/opensearch/nodestats/NodeStatsIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/nodestats/NodeStatsIT.java @@ -10,6 +10,8 @@ import org.opensearch.ExceptionsHelper; import org.opensearch.action.DocWriteResponse; +import org.opensearch.action.admin.cluster.node.stats.NodesStatsResponse; +import org.opensearch.action.admin.indices.stats.CommonStatsFlags; import org.opensearch.action.bulk.BulkItemResponse; import org.opensearch.action.bulk.BulkRequest; import org.opensearch.action.bulk.BulkResponse; @@ -19,6 +21,7 @@ import org.opensearch.action.index.IndexResponse; import org.opensearch.action.update.UpdateRequest; import org.opensearch.action.update.UpdateResponse; +import org.opensearch.cluster.ClusterState; import org.opensearch.index.IndexNotFoundException; import org.opensearch.index.engine.DocumentMissingException; import org.opensearch.index.engine.VersionConflictEngineException; @@ -31,6 +34,7 @@ import java.util.Arrays; import java.util.Comparator; import java.util.Map; +import java.util.ArrayList; import java.util.concurrent.atomic.AtomicLong; import static java.util.Collections.singletonMap; @@ -242,6 +246,47 @@ public void testNodeIndicesStatsDocStatusStatsCreateDeleteUpdate() { } } } + public void testNodeIndicesStatsOptimisedResponse() { + internalCluster().startNode(); + ensureGreen(); + String indexName = "test1"; + index(indexName, "type", "1", "f", "f"); + refresh(); + ClusterState clusterState = client().admin().cluster().prepareState().get().getState(); + + NodesStatsResponse response = client().admin().cluster().prepareNodesStats().get(); + response.getNodes().forEach(nodeStats -> { + assertNotNull(nodeStats.getIndices().getShardStats(clusterState.metadata().index(indexName).getIndex())); + assertNull(nodeStats.getIndices().getIndexStats(clusterState.metadata().index(indexName).getIndex())); + }); + CommonStatsFlags commonStatsFlags = new CommonStatsFlags(); + commonStatsFlags.optimizeNodeIndicesStatsOnLevel(true); + response = client().admin().cluster().prepareNodesStats().setIndices(commonStatsFlags).get(); + response.getNodes().forEach(nodeStats -> { + assertNull(nodeStats.getIndices().getShardStats(clusterState.metadata().index(indexName).getIndex())); + assertNull(nodeStats.getIndices().getIndexStats(clusterState.metadata().index(indexName).getIndex())); + + }); + ArrayList level_arg = new ArrayList<>(); + level_arg.add("indices"); + commonStatsFlags.setLevels(level_arg.toArray(new String[0])); + response = client().admin().cluster().prepareNodesStats().setIndices(commonStatsFlags).get(); + response.getNodes().forEach(nodeStats -> { + assertNotNull(nodeStats.getIndices().getIndexStats(clusterState.metadata().index(indexName).getIndex())); + assertNull(nodeStats.getIndices().getShardStats(clusterState.metadata().index(indexName).getIndex())); + }); + + level_arg.clear(); + level_arg.add("shards"); + commonStatsFlags.setLevels(level_arg.toArray(new String[0])); + response = client().admin().cluster().prepareNodesStats().setIndices(commonStatsFlags).get(); + response.getNodes().forEach(nodeStats -> { + assertNotNull(nodeStats.getIndices().getShardStats(clusterState.metadata().index(indexName).getIndex())); + assertNull(nodeStats.getIndices().getIndexStats(clusterState.metadata().index(indexName).getIndex())); + }); + } + + private void assertDocStatusStats() { DocStatusStats docStatusStats = client().admin() diff --git a/server/src/main/java/org/opensearch/action/admin/cluster/node/stats/NodesStatsRequest.java b/server/src/main/java/org/opensearch/action/admin/cluster/node/stats/NodesStatsRequest.java index 379836cf442e3..202a9cdf3cf4f 100644 --- a/server/src/main/java/org/opensearch/action/admin/cluster/node/stats/NodesStatsRequest.java +++ b/server/src/main/java/org/opensearch/action/admin/cluster/node/stats/NodesStatsRequest.java @@ -127,6 +127,16 @@ public NodesStatsRequest indices(boolean indices) { return this; } + /** + * Use Optimized Response filtered based on level + */ + public NodesStatsRequest useOptimizedNodeIndicesStats(boolean useOptimizedNodeIndicesStats){ + if (this.indices!=null) { + this.indices.optimizeNodeIndicesStatsOnLevel(true); + } + return this; + } + /** * Get the names of requested metrics, excluding indices, which are * handled separately. 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 ca2685e093d3f..5b69cc6eefb35 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 @@ -67,6 +67,8 @@ public class CommonStatsFlags implements Writeable, Cloneable { // Used for metric CACHE_STATS, to determine which caches to report stats for private EnumSet includeCaches = EnumSet.noneOf(CacheType.class); private String[] levels = new String[0]; + private boolean optimizeNodeIndicesStatsOnLevel = false; + /** * @param flags flags to set. If no flags are supplied, default flags will be set. @@ -100,6 +102,9 @@ public CommonStatsFlags(StreamInput in) throws IOException { includeCaches = in.readEnumSet(CacheType.class); levels = in.readStringArray(); } + if (in.getVersion().onOrAfter(Version.V_2_15_0)) { + optimizeNodeIndicesStatsOnLevel = in.readBoolean(); + } } @Override @@ -124,6 +129,9 @@ public void writeTo(StreamOutput out) throws IOException { out.writeEnumSet(includeCaches); out.writeStringArrayNullable(levels); } + if (out.getVersion().onOrAfter(Version.V_2_15_0)){ + out.writeBoolean(optimizeNodeIndicesStatsOnLevel); + } } /** @@ -262,6 +270,14 @@ public boolean includeSegmentFileSizes() { return this.includeSegmentFileSizes; } + public void optimizeNodeIndicesStatsOnLevel(boolean optimizeNodeIndicesStatsOnLevel) { + this.optimizeNodeIndicesStatsOnLevel = optimizeNodeIndicesStatsOnLevel; + } + + public boolean optimizeNodeIndicesStatsOnLevel() { + return this.optimizeNodeIndicesStatsOnLevel; + } + public boolean isSet(Flag flag) { return flags.contains(flag); } diff --git a/server/src/main/java/org/opensearch/indices/IndicesService.java b/server/src/main/java/org/opensearch/indices/IndicesService.java index 251be8a990055..af3345942d3c4 100644 --- a/server/src/main/java/org/opensearch/indices/IndicesService.java +++ b/server/src/main/java/org/opensearch/indices/IndicesService.java @@ -622,7 +622,10 @@ public NodeIndicesStats stats(CommonStatsFlags flags) { break; } } - + if (flags.optimizeNodeIndicesStatsOnLevel()) { + logger.info("Picked NodeIndicesStats"); + return new NodeIndicesStats(commonStats, statsByShard(this, flags), searchRequestStats, flags.getLevels()); + } return new NodeIndicesStats(commonStats, statsByShard(this, flags), searchRequestStats); } diff --git a/server/src/main/java/org/opensearch/indices/NodeIndicesStats.java b/server/src/main/java/org/opensearch/indices/NodeIndicesStats.java index 35b6fd395ee12..a38e9289aca39 100644 --- a/server/src/main/java/org/opensearch/indices/NodeIndicesStats.java +++ b/server/src/main/java/org/opensearch/indices/NodeIndicesStats.java @@ -32,6 +32,9 @@ package org.opensearch.indices; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; +import org.opensearch.Version; import org.opensearch.action.admin.indices.stats.CommonStats; import org.opensearch.action.admin.indices.stats.IndexShardStats; import org.opensearch.action.admin.indices.stats.ShardStats; @@ -63,6 +66,7 @@ import java.io.IOException; import java.util.ArrayList; +import java.util.Arrays; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -75,22 +79,21 @@ @PublicApi(since = "1.0.0") public class NodeIndicesStats implements Writeable, ToXContentFragment { private CommonStats stats; + private Map statsByIndex; private Map> statsByShard; public NodeIndicesStats(StreamInput in) throws IOException { stats = new CommonStats(in); + if (in.getVersion().onOrAfter(Version.V_2_15_0)) { + // contains statsByIndex + if (in.readBoolean()) { + statsByIndex = new HashMap<>(); + readStatsByIndex(in); + } + } if (in.readBoolean()) { - int entries = in.readVInt(); statsByShard = new HashMap<>(); - for (int i = 0; i < entries; i++) { - Index index = new Index(in); - int indexShardListSize = in.readVInt(); - List indexShardStats = new ArrayList<>(indexShardListSize); - for (int j = 0; j < indexShardListSize; j++) { - indexShardStats.add(new IndexShardStats(in)); - } - statsByShard.put(index, indexShardStats); - } + readStatsByShards(in); } } @@ -112,6 +115,57 @@ public NodeIndicesStats(CommonStats oldStats, Map> } } + public NodeIndicesStats( + CommonStats oldStats, + Map> statsByShard, + SearchRequestStats searchRequestStats, + String[] levels + ) { + // make a total common stats from old ones and current ones + this.stats = oldStats; + for (List shardStatsList : statsByShard.values()) { + for (IndexShardStats indexShardStats : shardStatsList) { + for (ShardStats shardStats : indexShardStats.getShards()) { + stats.add(shardStats.getStats()); + } + } + } + + if (this.stats.search != null) { + this.stats.search.setSearchRequestStats(searchRequestStats); + } + + if (levels != null) { + if (Arrays.stream(levels).anyMatch(NodeIndicesStats.levels.indices::equals)) { + this.statsByIndex = createStatsByIndex(statsByShard); + } else if (Arrays.stream(levels).anyMatch(NodeIndicesStats.levels.shards::equals)) { + this.statsByShard = statsByShard; + } + } + } + + private void readStatsByIndex(StreamInput in) throws IOException { + int indexEntries = in.readVInt(); + for (int i = 0; i < indexEntries; i++) { + Index index = new Index(in); + CommonStats commonStats = new CommonStats(in); + statsByIndex.put(index, commonStats); + } + } + + private void readStatsByShards(StreamInput in) throws IOException { + int entries = in.readVInt(); + for (int i = 0; i < entries; i++) { + Index index = new Index(in); + int indexShardListSize = in.readVInt(); + List indexShardStats = new ArrayList<>(indexShardListSize); + for (int j = 0; j < indexShardListSize; j++) { + indexShardStats.add(new IndexShardStats(in)); + } + statsByShard.put(index, indexShardStats); + } + } + @Nullable public StoreStats getStore() { return stats.getStore(); @@ -195,7 +249,31 @@ public RecoveryStats getRecoveryStats() { @Override public void writeTo(StreamOutput out) throws IOException { stats.writeTo(out); + + if (out.getVersion().onOrAfter(Version.V_2_15_0)) { + out.writeBoolean(statsByIndex != null); + if (statsByIndex != null) { + writeStatsByIndex(out); + } + } + out.writeBoolean(statsByShard != null); + if (statsByShard != null) { + writeStatsByShards(out); + } + } + + private void writeStatsByIndex(StreamOutput out) throws IOException { + if (statsByIndex != null) { + out.writeVInt(statsByIndex.size()); + for (Map.Entry entry : statsByIndex.entrySet()) { + entry.getKey().writeTo(out); + entry.getValue().writeTo(out); + } + } + } + + private void writeStatsByShards(StreamOutput out) throws IOException { if (statsByShard != null) { out.writeVInt(statsByShard.size()); for (Map.Entry> entry : statsByShard.entrySet()) { @@ -222,16 +300,18 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws builder.startObject(Fields.INDICES); stats.toXContent(builder, params); - if ("indices".equals(level)) { - Map indexStats = createStatsByIndex(); + if (levels.indices.equals(level)) { builder.startObject(Fields.INDICES); - for (Map.Entry entry : indexStats.entrySet()) { + if (statsByIndex == null && statsByShard!=null) { + statsByIndex = createStatsByIndex(statsByShard); + } + for (Map.Entry entry : statsByIndex.entrySet()) { builder.startObject(entry.getKey().getName()); entry.getValue().toXContent(builder, params); builder.endObject(); } builder.endObject(); - } else if ("shards".equals(level)) { + } else if (levels.shards.equals(level)) { builder.startObject("shards"); for (Map.Entry> entry : statsByShard.entrySet()) { builder.startArray(entry.getKey().getName()); @@ -251,7 +331,7 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws return builder; } - private Map createStatsByIndex() { + private Map createStatsByIndex(Map> statsByShard) { Map statsMap = new HashMap<>(); for (Map.Entry> entry : statsByShard.entrySet()) { if (!statsMap.containsKey(entry.getKey())) { @@ -276,6 +356,14 @@ public List getShardStats(Index index) { } } + public CommonStats getIndexStats(Index index) { + if (statsByIndex == null) { + return null; + } else { + return statsByIndex.get(index); + } + } + /** * Fields used for parsing and toXContent * @@ -284,4 +372,28 @@ public List getShardStats(Index index) { static final class Fields { static final String INDICES = "indices"; } + + /** + * Levels for the NodeIndicesStats + */ + public enum levels { + node("node"), + indices("indices"), + shards("shards"); + + private final String name; + + levels(String name) { + this.name = name; + } + + @Override + public String toString() { + return name; + } + + public boolean equals(String value) { + return this.name.equals(value); + } + } } 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 267bfde576dec..1bac32696b325 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 @@ -232,6 +232,7 @@ public RestChannelConsumer prepareRequest(final RestRequest request, final NodeC // If no levels are passed in this results in an empty array. String[] levels = Strings.splitStringByCommaToArray(request.param("level")); nodesStatsRequest.indices().setLevels(levels); + nodesStatsRequest.indices().optimizeNodeIndicesStatsOnLevel(true); return channel -> client.admin().cluster().nodesStats(nodesStatsRequest, new NodesResponseRestListener<>(channel)); } diff --git a/server/src/main/java/org/opensearch/rest/action/cat/RestNodesAction.java b/server/src/main/java/org/opensearch/rest/action/cat/RestNodesAction.java index e11012a23fce7..91894f2542536 100644 --- a/server/src/main/java/org/opensearch/rest/action/cat/RestNodesAction.java +++ b/server/src/main/java/org/opensearch/rest/action/cat/RestNodesAction.java @@ -138,7 +138,7 @@ public void processResponse(final NodesInfoResponse nodesInfoResponse) { NodesStatsRequest nodesStatsRequest = new NodesStatsRequest(); nodesStatsRequest.timeout(request.param("timeout")); nodesStatsRequest.clear() - .indices(true) + .indices(true).useOptimizedNodeIndicesStats(true) .addMetrics( NodesStatsRequest.Metric.JVM.metricName(), NodesStatsRequest.Metric.OS.metricName(), From a267aeff794f3c69cc9d63b408208bd49686c571 Mon Sep 17 00:00:00 2001 From: Pranshu Shukla Date: Wed, 10 Jul 2024 11:50:49 +0530 Subject: [PATCH 02/25] Modify optimisation approach without Cluster Settings Signed-off-by: Pranshu Shukla --- .../org/opensearch/nodestats/NodeStatsIT.java | 147 +++++++++++--- .../cluster/node/stats/NodesStatsRequest.java | 4 +- .../admin/indices/stats/CommonStatsFlags.java | 5 +- .../opensearch/indices/IndicesService.java | 1 - .../opensearch/indices/NodeIndicesStats.java | 101 ++++----- .../rest/action/cat/RestNodesAction.java | 3 +- .../cluster/node/stats/NodeStatsTests.java | 191 ++++++++++++++++++ 7 files changed, 360 insertions(+), 92 deletions(-) diff --git a/server/src/internalClusterTest/java/org/opensearch/nodestats/NodeStatsIT.java b/server/src/internalClusterTest/java/org/opensearch/nodestats/NodeStatsIT.java index 8f7bdc832a126..08f09a1421fd7 100644 --- a/server/src/internalClusterTest/java/org/opensearch/nodestats/NodeStatsIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/nodestats/NodeStatsIT.java @@ -22,6 +22,11 @@ import org.opensearch.action.update.UpdateRequest; import org.opensearch.action.update.UpdateResponse; import org.opensearch.cluster.ClusterState; +import org.opensearch.common.xcontent.XContentFactory; +import org.opensearch.common.xcontent.XContentHelper; +import org.opensearch.core.common.bytes.BytesReference; +import org.opensearch.core.xcontent.ToXContent; +import org.opensearch.core.xcontent.XContentBuilder; import org.opensearch.index.IndexNotFoundException; import org.opensearch.index.engine.DocumentMissingException; import org.opensearch.index.engine.VersionConflictEngineException; @@ -31,10 +36,13 @@ import org.opensearch.test.OpenSearchIntegTestCase.Scope; import org.hamcrest.MatcherAssert; +import java.io.IOException; +import java.util.ArrayList; import java.util.Arrays; +import java.util.Collections; import java.util.Comparator; +import java.util.LinkedHashMap; import java.util.Map; -import java.util.ArrayList; import java.util.concurrent.atomic.AtomicLong; import static java.util.Collections.singletonMap; @@ -246,7 +254,13 @@ public void testNodeIndicesStatsDocStatusStatsCreateDeleteUpdate() { } } } - public void testNodeIndicesStatsOptimisedResponse() { + + /** + * Default behavior - without consideration of request level param on level, the NodeStatsRequest always + * returns ShardStats which is aggregated on the coordinator node when creating the XContent. + */ + public void testNodeIndicesStatsDefaultResponse() { + String testLevel = randomFrom("null", "node", "indices", "shards", "unknown"); internalCluster().startNode(); ensureGreen(); String indexName = "test1"; @@ -254,39 +268,126 @@ public void testNodeIndicesStatsOptimisedResponse() { refresh(); ClusterState clusterState = client().admin().cluster().prepareState().get().getState(); - NodesStatsResponse response = client().admin().cluster().prepareNodesStats().get(); + NodesStatsResponse response; + if (!testLevel.equals("null")) { + ArrayList level_arg = new ArrayList<>(); + level_arg.add(testLevel); + + CommonStatsFlags commonStatsFlags = new CommonStatsFlags(); + commonStatsFlags.setLevels(level_arg.toArray(new String[0])); + response = client().admin().cluster().prepareNodesStats().setIndices(commonStatsFlags).get(); + } else { + response = client().admin().cluster().prepareNodesStats().get(); + } + response.getNodes().forEach(nodeStats -> { assertNotNull(nodeStats.getIndices().getShardStats(clusterState.metadata().index(indexName).getIndex())); - assertNull(nodeStats.getIndices().getIndexStats(clusterState.metadata().index(indexName).getIndex())); + try { + // Without any param - default is level = nodes + XContentBuilder builder = XContentFactory.jsonBuilder(); + builder.startObject(); + builder = nodeStats.getIndices().toXContent(builder, ToXContent.EMPTY_PARAMS); + builder.endObject(); + + Map xContentMap = xContentBuilderToMap(builder); + LinkedHashMap indicesStatsMap = (LinkedHashMap) xContentMap.get("indices"); + assertFalse(indicesStatsMap.containsKey("indices")); + assertFalse(indicesStatsMap.containsKey("shards")); + + // With param containing level as 'indices', the indices stats are returned + builder = XContentFactory.jsonBuilder(); + builder.startObject(); + builder = nodeStats.getIndices() + .toXContent(builder, new ToXContent.MapParams(Collections.singletonMap("level", "indices"))); + builder.endObject(); + + xContentMap = xContentBuilderToMap(builder); + indicesStatsMap = (LinkedHashMap) xContentMap.get("indices"); + assertTrue(indicesStatsMap.containsKey("indices")); + assertFalse(indicesStatsMap.containsKey("shards")); + + LinkedHashMap indexLevelStats = (LinkedHashMap) indicesStatsMap.get("indices"); + assertTrue(indexLevelStats.containsKey(indexName)); + + // With param containing level as 'shards', the shard stats are returned + builder = XContentFactory.jsonBuilder(); + builder.startObject(); + builder = nodeStats.getIndices().toXContent(builder, new ToXContent.MapParams(Collections.singletonMap("level", "shards"))); + builder.endObject(); + + xContentMap = xContentBuilderToMap(builder); + indicesStatsMap = (LinkedHashMap) xContentMap.get("indices"); + assertFalse(indicesStatsMap.containsKey("indices")); + assertTrue(indicesStatsMap.containsKey("shards")); + + LinkedHashMap shardLevelStats = (LinkedHashMap) indicesStatsMap.get("shards"); + assertTrue(shardLevelStats.containsKey(indexName)); + } catch (IOException e) { + throw new RuntimeException(e); + } }); + } + + /** + * Optimized behavior - to avoid unnecessary IO in the form of shard-stats when not required, we not honor the levels on the + * individual data nodes instead and pre-compute information as required. + */ + public void testNodeIndicesStatsOptimizedResponse() { + String testLevel = randomFrom("null", "node", "indices", "shards", "unknown"); + internalCluster().startNode(); + ensureGreen(); + String indexName = "test1"; + index(indexName, "type", "1", "f", "f"); + refresh(); + + NodesStatsResponse response; CommonStatsFlags commonStatsFlags = new CommonStatsFlags(); commonStatsFlags.optimizeNodeIndicesStatsOnLevel(true); - response = client().admin().cluster().prepareNodesStats().setIndices(commonStatsFlags).get(); - response.getNodes().forEach(nodeStats -> { - assertNull(nodeStats.getIndices().getShardStats(clusterState.metadata().index(indexName).getIndex())); - assertNull(nodeStats.getIndices().getIndexStats(clusterState.metadata().index(indexName).getIndex())); + if (!testLevel.equals("null")) { + ArrayList level_arg = new ArrayList<>(); + level_arg.add(testLevel); - }); - ArrayList level_arg = new ArrayList<>(); - level_arg.add("indices"); - commonStatsFlags.setLevels(level_arg.toArray(new String[0])); + commonStatsFlags.setLevels(level_arg.toArray(new String[0])); + } response = client().admin().cluster().prepareNodesStats().setIndices(commonStatsFlags).get(); - response.getNodes().forEach(nodeStats -> { - assertNotNull(nodeStats.getIndices().getIndexStats(clusterState.metadata().index(indexName).getIndex())); - assertNull(nodeStats.getIndices().getShardStats(clusterState.metadata().index(indexName).getIndex())); - }); - level_arg.clear(); - level_arg.add("shards"); - commonStatsFlags.setLevels(level_arg.toArray(new String[0])); - response = client().admin().cluster().prepareNodesStats().setIndices(commonStatsFlags).get(); response.getNodes().forEach(nodeStats -> { - assertNotNull(nodeStats.getIndices().getShardStats(clusterState.metadata().index(indexName).getIndex())); - assertNull(nodeStats.getIndices().getIndexStats(clusterState.metadata().index(indexName).getIndex())); + try { + XContentBuilder builder = XContentFactory.jsonBuilder(); + builder.startObject(); + builder = nodeStats.getIndices().toXContent(builder, new ToXContent.MapParams(Collections.singletonMap("level", "shards"))); + builder.endObject(); + + Map xContentMap = xContentBuilderToMap(builder); + LinkedHashMap indicesStatsMap = (LinkedHashMap) xContentMap.get("indices"); + LinkedHashMap indicesStats = (LinkedHashMap) indicesStatsMap.get("indices"); + LinkedHashMap shardStats = (LinkedHashMap) indicesStatsMap.get("shards"); + + switch (testLevel) { + case "shards": + assertFalse(shardStats.isEmpty()); + assertFalse(indicesStats.isEmpty()); + break; + case "indices": + assertTrue(shardStats.isEmpty()); + assertFalse(indicesStats.isEmpty()); + break; + case "node": + case "null": + case "unknown": + assertTrue(shardStats.isEmpty()); + assertTrue(indicesStats.isEmpty()); + break; + } + } catch (IOException e) { + throw new RuntimeException(e); + } }); } - + private Map xContentBuilderToMap(XContentBuilder xContentBuilder) { + return XContentHelper.convertToMap(BytesReference.bytes(xContentBuilder), true, xContentBuilder.contentType()).v2(); + } private void assertDocStatusStats() { DocStatusStats docStatusStats = client().admin() diff --git a/server/src/main/java/org/opensearch/action/admin/cluster/node/stats/NodesStatsRequest.java b/server/src/main/java/org/opensearch/action/admin/cluster/node/stats/NodesStatsRequest.java index 202a9cdf3cf4f..970c4340dd33b 100644 --- a/server/src/main/java/org/opensearch/action/admin/cluster/node/stats/NodesStatsRequest.java +++ b/server/src/main/java/org/opensearch/action/admin/cluster/node/stats/NodesStatsRequest.java @@ -130,8 +130,8 @@ public NodesStatsRequest indices(boolean indices) { /** * Use Optimized Response filtered based on level */ - public NodesStatsRequest useOptimizedNodeIndicesStats(boolean useOptimizedNodeIndicesStats){ - if (this.indices!=null) { + public NodesStatsRequest useOptimizedNodeIndicesStats(boolean useOptimizedNodeIndicesStats) { + if (this.indices != null) { this.indices.optimizeNodeIndicesStatsOnLevel(true); } return this; 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 5b69cc6eefb35..faccdcf02362c 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 @@ -69,7 +69,6 @@ public class CommonStatsFlags implements Writeable, Cloneable { private String[] levels = new String[0]; private boolean optimizeNodeIndicesStatsOnLevel = false; - /** * @param flags flags to set. If no flags are supplied, default flags will be set. */ @@ -102,7 +101,7 @@ public CommonStatsFlags(StreamInput in) throws IOException { includeCaches = in.readEnumSet(CacheType.class); levels = in.readStringArray(); } - if (in.getVersion().onOrAfter(Version.V_2_15_0)) { + if (in.getVersion().onOrAfter(Version.V_2_16_0)) { optimizeNodeIndicesStatsOnLevel = in.readBoolean(); } } @@ -129,7 +128,7 @@ public void writeTo(StreamOutput out) throws IOException { out.writeEnumSet(includeCaches); out.writeStringArrayNullable(levels); } - if (out.getVersion().onOrAfter(Version.V_2_15_0)){ + if (out.getVersion().onOrAfter(Version.V_2_16_0)) { out.writeBoolean(optimizeNodeIndicesStatsOnLevel); } } diff --git a/server/src/main/java/org/opensearch/indices/IndicesService.java b/server/src/main/java/org/opensearch/indices/IndicesService.java index 3ed524521dfec..ec4004bb6bb88 100644 --- a/server/src/main/java/org/opensearch/indices/IndicesService.java +++ b/server/src/main/java/org/opensearch/indices/IndicesService.java @@ -693,7 +693,6 @@ public NodeIndicesStats stats(CommonStatsFlags flags) { } } if (flags.optimizeNodeIndicesStatsOnLevel()) { - logger.info("Picked NodeIndicesStats"); return new NodeIndicesStats(commonStats, statsByShard(this, flags), searchRequestStats, flags.getLevels()); } return new NodeIndicesStats(commonStats, statsByShard(this, flags), searchRequestStats); diff --git a/server/src/main/java/org/opensearch/indices/NodeIndicesStats.java b/server/src/main/java/org/opensearch/indices/NodeIndicesStats.java index a38e9289aca39..c3677745c716f 100644 --- a/server/src/main/java/org/opensearch/indices/NodeIndicesStats.java +++ b/server/src/main/java/org/opensearch/indices/NodeIndicesStats.java @@ -32,8 +32,6 @@ package org.opensearch.indices; -import org.apache.logging.log4j.LogManager; -import org.apache.logging.log4j.Logger; import org.opensearch.Version; import org.opensearch.action.admin.indices.stats.CommonStats; import org.opensearch.action.admin.indices.stats.IndexShardStats; @@ -78,13 +76,13 @@ */ @PublicApi(since = "1.0.0") public class NodeIndicesStats implements Writeable, ToXContentFragment { - private CommonStats stats; - private Map statsByIndex; - private Map> statsByShard; + protected CommonStats stats; + protected Map statsByIndex; + protected Map> statsByShard; public NodeIndicesStats(StreamInput in) throws IOException { stats = new CommonStats(in); - if (in.getVersion().onOrAfter(Version.V_2_15_0)) { + if (in.getVersion().onOrAfter(Version.V_2_16_0)) { // contains statsByIndex if (in.readBoolean()) { statsByIndex = new HashMap<>(); @@ -136,11 +134,17 @@ public NodeIndicesStats( } if (levels != null) { - if (Arrays.stream(levels).anyMatch(NodeIndicesStats.levels.indices::equals)) { - this.statsByIndex = createStatsByIndex(statsByShard); - } else if (Arrays.stream(levels).anyMatch(NodeIndicesStats.levels.shards::equals)) { - this.statsByShard = statsByShard; - } + Arrays.stream(levels).anyMatch(level -> { + switch (level) { + case Fields.INDICES: + this.statsByIndex = createStatsByIndex(statsByShard); + return true; + case Fields.SHARDS: + this.statsByShard = statsByShard; + return true; + } + return false; + }); } } @@ -250,7 +254,7 @@ public RecoveryStats getRecoveryStats() { public void writeTo(StreamOutput out) throws IOException { stats.writeTo(out); - if (out.getVersion().onOrAfter(Version.V_2_15_0)) { + if (out.getVersion().onOrAfter(Version.V_2_16_0)) { out.writeBoolean(statsByIndex != null); if (statsByIndex != null) { writeStatsByIndex(out); @@ -300,29 +304,33 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws builder.startObject(Fields.INDICES); stats.toXContent(builder, params); - if (levels.indices.equals(level)) { - builder.startObject(Fields.INDICES); - if (statsByIndex == null && statsByShard!=null) { + if (Fields.INDICES.equals(level)) { + if (statsByIndex == null && statsByShard != null) { statsByIndex = createStatsByIndex(statsByShard); } - for (Map.Entry entry : statsByIndex.entrySet()) { - builder.startObject(entry.getKey().getName()); - entry.getValue().toXContent(builder, params); - builder.endObject(); + builder.startObject(Fields.INDICES); + if (statsByIndex != null) { + for (Map.Entry entry : statsByIndex.entrySet()) { + builder.startObject(entry.getKey().getName()); + entry.getValue().toXContent(builder, params); + builder.endObject(); + } } builder.endObject(); - } else if (levels.shards.equals(level)) { + } else if (Fields.SHARDS.equals(level)) { builder.startObject("shards"); - for (Map.Entry> entry : statsByShard.entrySet()) { - builder.startArray(entry.getKey().getName()); - for (IndexShardStats indexShardStats : entry.getValue()) { - builder.startObject().startObject(String.valueOf(indexShardStats.getShardId().getId())); - for (ShardStats shardStats : indexShardStats.getShards()) { - shardStats.toXContent(builder, params); + if (statsByShard != null) { + for (Map.Entry> entry : statsByShard.entrySet()) { + builder.startArray(entry.getKey().getName()); + for (IndexShardStats indexShardStats : entry.getValue()) { + builder.startObject().startObject(String.valueOf(indexShardStats.getShardId().getId())); + for (ShardStats shardStats : indexShardStats.getShards()) { + shardStats.toXContent(builder, params); + } + builder.endObject().endObject(); } - builder.endObject().endObject(); + builder.endArray(); } - builder.endArray(); } builder.endObject(); } @@ -356,44 +364,13 @@ public List getShardStats(Index index) { } } - public CommonStats getIndexStats(Index index) { - if (statsByIndex == null) { - return null; - } else { - return statsByIndex.get(index); - } - } - /** * Fields used for parsing and toXContent * * @opensearch.internal */ - static final class Fields { - static final String INDICES = "indices"; - } - - /** - * Levels for the NodeIndicesStats - */ - public enum levels { - node("node"), - indices("indices"), - shards("shards"); - - private final String name; - - levels(String name) { - this.name = name; - } - - @Override - public String toString() { - return name; - } - - public boolean equals(String value) { - return this.name.equals(value); - } + public static final class Fields { + public static final String INDICES = "indices"; + public static final String SHARDS = "shards"; } } diff --git a/server/src/main/java/org/opensearch/rest/action/cat/RestNodesAction.java b/server/src/main/java/org/opensearch/rest/action/cat/RestNodesAction.java index 91894f2542536..cd5d76a29047d 100644 --- a/server/src/main/java/org/opensearch/rest/action/cat/RestNodesAction.java +++ b/server/src/main/java/org/opensearch/rest/action/cat/RestNodesAction.java @@ -138,7 +138,8 @@ public void processResponse(final NodesInfoResponse nodesInfoResponse) { NodesStatsRequest nodesStatsRequest = new NodesStatsRequest(); nodesStatsRequest.timeout(request.param("timeout")); nodesStatsRequest.clear() - .indices(true).useOptimizedNodeIndicesStats(true) + .indices(true) + .useOptimizedNodeIndicesStats(true) .addMetrics( NodesStatsRequest.Metric.JVM.metricName(), NodesStatsRequest.Metric.OS.metricName(), diff --git a/server/src/test/java/org/opensearch/action/admin/cluster/node/stats/NodeStatsTests.java b/server/src/test/java/org/opensearch/action/admin/cluster/node/stats/NodeStatsTests.java index f7bc96bdfe769..88db8efff41b6 100644 --- a/server/src/test/java/org/opensearch/action/admin/cluster/node/stats/NodeStatsTests.java +++ b/server/src/test/java/org/opensearch/action/admin/cluster/node/stats/NodeStatsTests.java @@ -32,13 +32,19 @@ package org.opensearch.action.admin.cluster.node.stats; +import org.opensearch.Version; import org.opensearch.action.admin.indices.stats.CommonStats; import org.opensearch.action.admin.indices.stats.CommonStatsFlags; +import org.opensearch.action.admin.indices.stats.IndexShardStats; +import org.opensearch.action.admin.indices.stats.ShardStats; import org.opensearch.action.search.SearchRequestStats; import org.opensearch.cluster.coordination.PendingClusterStateStats; import org.opensearch.cluster.coordination.PersistedStateStats; import org.opensearch.cluster.coordination.PublishClusterStateStats; import org.opensearch.cluster.node.DiscoveryNode; +import org.opensearch.cluster.routing.ShardRouting; +import org.opensearch.cluster.routing.ShardRoutingState; +import org.opensearch.cluster.routing.TestShardRouting; import org.opensearch.cluster.routing.WeightedRoutingStats; import org.opensearch.cluster.service.ClusterManagerThrottlingStats; import org.opensearch.cluster.service.ClusterStateStats; @@ -53,6 +59,7 @@ import org.opensearch.common.settings.ClusterSettings; import org.opensearch.common.settings.Settings; import org.opensearch.core.common.io.stream.StreamInput; +import org.opensearch.core.index.Index; import org.opensearch.core.index.shard.ShardId; import org.opensearch.core.indices.breaker.AllCircuitBreakerStats; import org.opensearch.core.indices.breaker.CircuitBreakerStats; @@ -63,6 +70,10 @@ import org.opensearch.index.SegmentReplicationRejectionStats; import org.opensearch.index.remote.RemoteSegmentStats; import org.opensearch.index.remote.RemoteTranslogTransferTracker; +import org.opensearch.index.shard.DocsStats; +import org.opensearch.index.shard.IndexingStats; +import org.opensearch.index.shard.ShardPath; +import org.opensearch.index.store.StoreStats; import org.opensearch.index.translog.RemoteTranslogStats; import org.opensearch.indices.NodeIndicesStats; import org.opensearch.ingest.IngestStats; @@ -88,6 +99,7 @@ import org.opensearch.transport.TransportStats; import java.io.IOException; +import java.nio.file.Path; import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; @@ -1065,4 +1077,183 @@ private static RemoteTranslogTransferTracker.Stats getRandomRemoteTranslogTransf private OperationStats getPipelineStats(List pipelineStats, String id) { return pipelineStats.stream().filter(p1 -> p1.getPipelineId().equals(id)).findFirst().map(p2 -> p2.getStats()).orElse(null); } + + public static class MockNodeIndicesStats extends NodeIndicesStats { + + public MockNodeIndicesStats(StreamInput in) throws IOException { + super(in); + } + + public MockNodeIndicesStats( + CommonStats oldStats, + Map> statsByShard, + SearchRequestStats searchRequestStats + ) { + super(oldStats, statsByShard, searchRequestStats); + } + + public MockNodeIndicesStats( + CommonStats oldStats, + Map> statsByShard, + SearchRequestStats searchRequestStats, + String[] levels + ) { + super(oldStats, statsByShard, searchRequestStats, levels); + } + + public CommonStats getStats() { + return this.stats; + } + + public Map getStatsByIndex() { + return this.statsByIndex; + } + + public Map> getStatsByShard() { + return this.statsByShard; + } + } + + public void testNodeIndicesStatsSerializationWithOldESVersionNodes() throws IOException { + long numDocs = randomLongBetween(0, 10000); + long numDeletedDocs = randomLongBetween(0, 100); + CommonStats commonStats = new CommonStats(CommonStatsFlags.NONE); + + commonStats.docs = new DocsStats(numDocs, numDeletedDocs, 0); + commonStats.store = new StoreStats(100, 0L); + commonStats.indexing = new IndexingStats(); + + CommonStatsFlags commonStatsFlags = new CommonStatsFlags(); + commonStatsFlags.clear(); + commonStatsFlags.set(CommonStatsFlags.Flag.Docs, true); + commonStatsFlags.set(CommonStatsFlags.Flag.Store, true); + commonStatsFlags.set(CommonStatsFlags.Flag.Indexing, true); + + Index newIndex = new Index("index", "_na_"); + + MockNodeIndicesStats mockNodeIndicesStats = generateMockNodeIndicesStats(commonStats, newIndex, commonStatsFlags); + + // To test out scenario when the incoming node stats response is from a node with an older ES Version. + try (BytesStreamOutput out = new BytesStreamOutput()) { + out.setVersion(Version.V_2_13_0); + mockNodeIndicesStats.writeTo(out); + try (StreamInput in = out.bytes().streamInput()) { + in.setVersion(Version.V_2_13_0); + MockNodeIndicesStats newNodeIndicesStats = new MockNodeIndicesStats(in); + + List incomingIndexStats = newNodeIndicesStats.getStatsByShard().get(newIndex); + incomingIndexStats.forEach(indexShardStats -> { + ShardStats shardStats = Arrays.stream(indexShardStats.getShards()).findFirst().get(); + DocsStats incomingDocStats = shardStats.getStats().docs; + + DocsStats hostDocStats = new DocsStats(numDocs, numDeletedDocs, 0); + assertEquals(incomingDocStats.getCount(), hostDocStats.getCount()); + assertEquals(incomingDocStats.getTotalSizeInBytes(), hostDocStats.getTotalSizeInBytes()); + assertEquals(incomingDocStats.getAverageSizeInBytes(), hostDocStats.getAverageSizeInBytes()); + assertEquals(incomingDocStats.getDeleted(), hostDocStats.getDeleted()); + }); + } + } + } + + public void testNodeIndicesStatsSerializationOnNewVersions() throws IOException { + long numDocs = randomLongBetween(0, 10000); + long numDeletedDocs = randomLongBetween(0, 100); + String levelParam = randomFrom("node", "indices", "shards"); + + CommonStats commonStats = new CommonStats(CommonStatsFlags.NONE); + + commonStats.docs = new DocsStats(numDocs, numDeletedDocs, 0); + commonStats.store = new StoreStats(100, 0L); + commonStats.indexing = new IndexingStats(); + + CommonStatsFlags commonStatsFlags = new CommonStatsFlags(); + commonStatsFlags.clear(); + commonStatsFlags.set(CommonStatsFlags.Flag.Docs, true); + commonStatsFlags.set(CommonStatsFlags.Flag.Store, true); + commonStatsFlags.set(CommonStatsFlags.Flag.Indexing, true); + commonStatsFlags.optimizeNodeIndicesStatsOnLevel(true); + + ArrayList level_arg = new ArrayList<>(); + level_arg.add(levelParam); + + commonStatsFlags.setLevels(level_arg.toArray(new String[0])); + + Index newIndex = new Index("index", "_na_"); + + MockNodeIndicesStats mockNodeIndicesStats = generateMockNodeIndicesStats(commonStats, newIndex, commonStatsFlags); + + // To test out scenario when the incoming node stats response is from a node with an older ES Version. + try (BytesStreamOutput out = new BytesStreamOutput()) { + mockNodeIndicesStats.writeTo(out); + try (StreamInput in = out.bytes().streamInput()) { + MockNodeIndicesStats newNodeIndicesStats = new MockNodeIndicesStats(in); + switch (levelParam) { + case "node": + assertNull(newNodeIndicesStats.getStatsByIndex()); + assertNull(newNodeIndicesStats.getStatsByShard()); + break; + case "indices": + assertNull(newNodeIndicesStats.getStatsByShard()); + assertNotNull(newNodeIndicesStats.getStatsByIndex()); + break; + case "shards": + assertNull(newNodeIndicesStats.getStatsByIndex()); + assertNotNull(newNodeIndicesStats.getStatsByShard()); + break; + } + } + } + } + + public MockNodeIndicesStats generateMockNodeIndicesStats(CommonStats commonStats, Index index, CommonStatsFlags commonStatsFlags) { + DiscoveryNode localNode = new DiscoveryNode("local", buildNewFakeTransportAddress(), Version.CURRENT); + Map> statsByShard = new HashMap<>(); + List indexShardStatsList = new ArrayList<>(); + Index statsIndex = null; + for (int i = 0; i < 2; i++) { + ShardRoutingState shardRoutingState = ShardRoutingState.fromValue((byte) randomIntBetween(2, 3)); + ShardRouting shardRouting = TestShardRouting.newShardRouting( + index.getName(), + i, + localNode.getId(), + randomBoolean(), + shardRoutingState + ); + + if (statsIndex == null) { + statsIndex = shardRouting.shardId().getIndex(); + } + + Path path = createTempDir().resolve("indices") + .resolve(shardRouting.shardId().getIndex().getUUID()) + .resolve(String.valueOf(shardRouting.shardId().id())); + + ShardStats shardStats = new ShardStats( + shardRouting, + new ShardPath(false, path, path, shardRouting.shardId()), + commonStats, + null, + null, + null + ); + IndexShardStats indexShardStats = new IndexShardStats(shardRouting.shardId(), new ShardStats[] { shardStats }); + indexShardStatsList.add(indexShardStats); + } + + statsByShard.put(statsIndex, indexShardStatsList); + + ClusterSettings clusterSettings = new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS); + + if (commonStatsFlags.optimizeNodeIndicesStatsOnLevel()) { + return new MockNodeIndicesStats( + new CommonStats(commonStatsFlags), + statsByShard, + new SearchRequestStats(clusterSettings), + commonStatsFlags.getLevels() + ); + } else { + return new MockNodeIndicesStats(new CommonStats(commonStatsFlags), statsByShard, new SearchRequestStats(clusterSettings)); + } + } } From 857e7e129007bd10fe86e2766547d25068666c38 Mon Sep 17 00:00:00 2001 From: Pranshu Shukla Date: Thu, 11 Jul 2024 16:33:16 +0530 Subject: [PATCH 03/25] Bumping changes to major version 3_0_0 to avoid BWC test failures Signed-off-by: Pranshu Shukla --- .../action/admin/indices/stats/CommonStatsFlags.java | 4 ++-- .../main/java/org/opensearch/indices/NodeIndicesStats.java | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) 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 faccdcf02362c..1404ba3132f3a 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 @@ -101,7 +101,7 @@ public CommonStatsFlags(StreamInput in) throws IOException { includeCaches = in.readEnumSet(CacheType.class); levels = in.readStringArray(); } - if (in.getVersion().onOrAfter(Version.V_2_16_0)) { + if (in.getVersion().onOrAfter(Version.V_3_0_0)) { optimizeNodeIndicesStatsOnLevel = in.readBoolean(); } } @@ -128,7 +128,7 @@ public void writeTo(StreamOutput out) throws IOException { out.writeEnumSet(includeCaches); out.writeStringArrayNullable(levels); } - if (out.getVersion().onOrAfter(Version.V_2_16_0)) { + if (out.getVersion().onOrAfter(Version.V_3_0_0)) { out.writeBoolean(optimizeNodeIndicesStatsOnLevel); } } diff --git a/server/src/main/java/org/opensearch/indices/NodeIndicesStats.java b/server/src/main/java/org/opensearch/indices/NodeIndicesStats.java index c3677745c716f..a21d988695ed4 100644 --- a/server/src/main/java/org/opensearch/indices/NodeIndicesStats.java +++ b/server/src/main/java/org/opensearch/indices/NodeIndicesStats.java @@ -82,7 +82,7 @@ public class NodeIndicesStats implements Writeable, ToXContentFragment { public NodeIndicesStats(StreamInput in) throws IOException { stats = new CommonStats(in); - if (in.getVersion().onOrAfter(Version.V_2_16_0)) { + if (in.getVersion().onOrAfter(Version.V_3_0_0)) { // contains statsByIndex if (in.readBoolean()) { statsByIndex = new HashMap<>(); @@ -254,7 +254,7 @@ public RecoveryStats getRecoveryStats() { public void writeTo(StreamOutput out) throws IOException { stats.writeTo(out); - if (out.getVersion().onOrAfter(Version.V_2_16_0)) { + if (out.getVersion().onOrAfter(Version.V_3_0_0)) { out.writeBoolean(statsByIndex != null); if (statsByIndex != null) { writeStatsByIndex(out); From 9876c1304d46eca71ed8cd88a91dd53b7a8ab855 Mon Sep 17 00:00:00 2001 From: Pranshu Shukla Date: Fri, 12 Jul 2024 18:21:28 +0530 Subject: [PATCH 04/25] Fix Failing Tests Signed-off-by: Pranshu Shukla --- .../org/opensearch/nodestats/NodeStatsIT.java | 77 ++++++++++++------- .../opensearch/indices/NodeIndicesStats.java | 25 ++++-- .../cluster/node/stats/NodeStatsTests.java | 2 +- 3 files changed, 68 insertions(+), 36 deletions(-) diff --git a/server/src/internalClusterTest/java/org/opensearch/nodestats/NodeStatsIT.java b/server/src/internalClusterTest/java/org/opensearch/nodestats/NodeStatsIT.java index 08f09a1421fd7..bbb5398c89aa7 100644 --- a/server/src/internalClusterTest/java/org/opensearch/nodestats/NodeStatsIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/nodestats/NodeStatsIT.java @@ -31,6 +31,7 @@ import org.opensearch.index.engine.DocumentMissingException; import org.opensearch.index.engine.VersionConflictEngineException; import org.opensearch.index.shard.IndexingStats.Stats.DocStatusStats; +import org.opensearch.indices.NodeIndicesStats; import org.opensearch.test.OpenSearchIntegTestCase; import org.opensearch.test.OpenSearchIntegTestCase.ClusterScope; import org.opensearch.test.OpenSearchIntegTestCase.Scope; @@ -260,7 +261,13 @@ public void testNodeIndicesStatsDocStatusStatsCreateDeleteUpdate() { * returns ShardStats which is aggregated on the coordinator node when creating the XContent. */ public void testNodeIndicesStatsDefaultResponse() { - String testLevel = randomFrom("null", "node", "indices", "shards", "unknown"); + String testLevel = randomFrom( + "null", + NodeIndicesStats.Fields.NODE, + NodeIndicesStats.Fields.INDICES, + NodeIndicesStats.Fields.SHARDS, + "unknown" + ); internalCluster().startNode(); ensureGreen(); String indexName = "test1"; @@ -290,37 +297,38 @@ public void testNodeIndicesStatsDefaultResponse() { builder.endObject(); Map xContentMap = xContentBuilderToMap(builder); - LinkedHashMap indicesStatsMap = (LinkedHashMap) xContentMap.get("indices"); - assertFalse(indicesStatsMap.containsKey("indices")); - assertFalse(indicesStatsMap.containsKey("shards")); + LinkedHashMap indicesStatsMap = (LinkedHashMap) xContentMap.get(NodeIndicesStats.Fields.INDICES); + assertFalse(indicesStatsMap.containsKey(NodeIndicesStats.Fields.INDICES)); + assertFalse(indicesStatsMap.containsKey(NodeIndicesStats.Fields.SHARDS)); // With param containing level as 'indices', the indices stats are returned builder = XContentFactory.jsonBuilder(); builder.startObject(); builder = nodeStats.getIndices() - .toXContent(builder, new ToXContent.MapParams(Collections.singletonMap("level", "indices"))); + .toXContent(builder, new ToXContent.MapParams(Collections.singletonMap("level", NodeIndicesStats.Fields.INDICES))); builder.endObject(); xContentMap = xContentBuilderToMap(builder); - indicesStatsMap = (LinkedHashMap) xContentMap.get("indices"); - assertTrue(indicesStatsMap.containsKey("indices")); - assertFalse(indicesStatsMap.containsKey("shards")); + indicesStatsMap = (LinkedHashMap) xContentMap.get(NodeIndicesStats.Fields.INDICES); + assertTrue(indicesStatsMap.containsKey(NodeIndicesStats.Fields.INDICES)); + assertFalse(indicesStatsMap.containsKey(NodeIndicesStats.Fields.SHARDS)); - LinkedHashMap indexLevelStats = (LinkedHashMap) indicesStatsMap.get("indices"); + LinkedHashMap indexLevelStats = (LinkedHashMap) indicesStatsMap.get(NodeIndicesStats.Fields.INDICES); assertTrue(indexLevelStats.containsKey(indexName)); // With param containing level as 'shards', the shard stats are returned builder = XContentFactory.jsonBuilder(); builder.startObject(); - builder = nodeStats.getIndices().toXContent(builder, new ToXContent.MapParams(Collections.singletonMap("level", "shards"))); + builder = nodeStats.getIndices() + .toXContent(builder, new ToXContent.MapParams(Collections.singletonMap("level", NodeIndicesStats.Fields.SHARDS))); builder.endObject(); xContentMap = xContentBuilderToMap(builder); - indicesStatsMap = (LinkedHashMap) xContentMap.get("indices"); - assertFalse(indicesStatsMap.containsKey("indices")); - assertTrue(indicesStatsMap.containsKey("shards")); + indicesStatsMap = (LinkedHashMap) xContentMap.get(NodeIndicesStats.Fields.INDICES); + assertFalse(indicesStatsMap.containsKey(NodeIndicesStats.Fields.INDICES)); + assertTrue(indicesStatsMap.containsKey(NodeIndicesStats.Fields.SHARDS)); - LinkedHashMap shardLevelStats = (LinkedHashMap) indicesStatsMap.get("shards"); + LinkedHashMap shardLevelStats = (LinkedHashMap) indicesStatsMap.get(NodeIndicesStats.Fields.SHARDS); assertTrue(shardLevelStats.containsKey(indexName)); } catch (IOException e) { throw new RuntimeException(e); @@ -333,7 +341,18 @@ public void testNodeIndicesStatsDefaultResponse() { * individual data nodes instead and pre-compute information as required. */ public void testNodeIndicesStatsOptimizedResponse() { - String testLevel = randomFrom("null", "node", "indices", "shards", "unknown"); + String testLevel = randomFrom( + "null", + NodeIndicesStats.Fields.NODE, + NodeIndicesStats.Fields.INDICES, + NodeIndicesStats.Fields.SHARDS, + "unknown" + ); + String testToXContentLevel = randomFrom( + NodeIndicesStats.Fields.NODE, + NodeIndicesStats.Fields.INDICES, + NodeIndicesStats.Fields.SHARDS + ); internalCluster().startNode(); ensureGreen(); String indexName = "test1"; @@ -355,28 +374,28 @@ public void testNodeIndicesStatsOptimizedResponse() { try { XContentBuilder builder = XContentFactory.jsonBuilder(); builder.startObject(); - builder = nodeStats.getIndices().toXContent(builder, new ToXContent.MapParams(Collections.singletonMap("level", "shards"))); + builder = nodeStats.getIndices() + .toXContent(builder, new ToXContent.MapParams(Collections.singletonMap("level", testToXContentLevel))); builder.endObject(); Map xContentMap = xContentBuilderToMap(builder); - LinkedHashMap indicesStatsMap = (LinkedHashMap) xContentMap.get("indices"); - LinkedHashMap indicesStats = (LinkedHashMap) indicesStatsMap.get("indices"); - LinkedHashMap shardStats = (LinkedHashMap) indicesStatsMap.get("shards"); - + LinkedHashMap indicesStatsMap = (LinkedHashMap) xContentMap.get(NodeIndicesStats.Fields.INDICES); + LinkedHashMap indicesStats = (LinkedHashMap) indicesStatsMap.get(NodeIndicesStats.Fields.INDICES); + LinkedHashMap shardStats = (LinkedHashMap) indicesStatsMap.get(NodeIndicesStats.Fields.SHARDS); switch (testLevel) { - case "shards": - assertFalse(shardStats.isEmpty()); - assertFalse(indicesStats.isEmpty()); + case NodeIndicesStats.Fields.SHARDS: + assertFalse(shardStats == null || shardStats.isEmpty()); + assertFalse(indicesStats == null || indicesStats.isEmpty()); break; - case "indices": - assertTrue(shardStats.isEmpty()); - assertFalse(indicesStats.isEmpty()); + case NodeIndicesStats.Fields.INDICES: + assertTrue(shardStats == null || shardStats.isEmpty()); + assertFalse(indicesStats == null || indicesStats.isEmpty()); break; - case "node": + case NodeIndicesStats.Fields.NODE: case "null": case "unknown": - assertTrue(shardStats.isEmpty()); - assertTrue(indicesStats.isEmpty()); + assertTrue(shardStats == null || shardStats.isEmpty()); + assertTrue(indicesStats == null || indicesStats.isEmpty()); break; } } catch (IOException e) { diff --git a/server/src/main/java/org/opensearch/indices/NodeIndicesStats.java b/server/src/main/java/org/opensearch/indices/NodeIndicesStats.java index a21d988695ed4..cc285f882e606 100644 --- a/server/src/main/java/org/opensearch/indices/NodeIndicesStats.java +++ b/server/src/main/java/org/opensearch/indices/NodeIndicesStats.java @@ -292,12 +292,23 @@ private void writeStatsByShards(StreamOutput out) throws IOException { @Override public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { - final String level = params.param("level", "node"); - final boolean isLevelValid = "indices".equalsIgnoreCase(level) - || "node".equalsIgnoreCase(level) - || "shards".equalsIgnoreCase(level); + final String level = params.param("level", Fields.NODE); + final boolean isLevelValid = Fields.NODE.equalsIgnoreCase(level) + || Fields.INDICES.equalsIgnoreCase(level) + || Fields.SHARDS.equalsIgnoreCase(level); if (!isLevelValid) { - throw new IllegalArgumentException("level parameter must be one of [indices] or [node] or [shards] but was [" + level + "]"); + throw new IllegalArgumentException( + "level parameter must be one of [" + + Fields.INDICES + + "] or " + + "[" + + Fields.NODE + + "] or [" + + Fields.SHARDS + + "] but was [" + + level + + "]" + ); } // "node" level @@ -318,7 +329,7 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws } builder.endObject(); } else if (Fields.SHARDS.equals(level)) { - builder.startObject("shards"); + builder.startObject(Fields.SHARDS); if (statsByShard != null) { for (Map.Entry> entry : statsByShard.entrySet()) { builder.startArray(entry.getKey().getName()); @@ -372,5 +383,7 @@ public List getShardStats(Index index) { public static final class Fields { public static final String INDICES = "indices"; public static final String SHARDS = "shards"; + public static final String NODE = "node"; + } } diff --git a/server/src/test/java/org/opensearch/action/admin/cluster/node/stats/NodeStatsTests.java b/server/src/test/java/org/opensearch/action/admin/cluster/node/stats/NodeStatsTests.java index 88db8efff41b6..010f28f498c05 100644 --- a/server/src/test/java/org/opensearch/action/admin/cluster/node/stats/NodeStatsTests.java +++ b/server/src/test/java/org/opensearch/action/admin/cluster/node/stats/NodeStatsTests.java @@ -1159,7 +1159,7 @@ public void testNodeIndicesStatsSerializationWithOldESVersionNodes() throws IOEx public void testNodeIndicesStatsSerializationOnNewVersions() throws IOException { long numDocs = randomLongBetween(0, 10000); long numDeletedDocs = randomLongBetween(0, 100); - String levelParam = randomFrom("node", "indices", "shards"); + String levelParam = randomFrom(NodeIndicesStats.Fields.INDICES, "indices", "shards"); CommonStats commonStats = new CommonStats(CommonStatsFlags.NONE); From 61bf2e0d977f3e28e09cbcb36856273b78b46437 Mon Sep 17 00:00:00 2001 From: Pranshu Shukla Date: Fri, 12 Jul 2024 19:56:42 +0530 Subject: [PATCH 05/25] Fix test logic Signed-off-by: Pranshu Shukla --- .../org/opensearch/nodestats/NodeStatsIT.java | 45 ++++++++++++++----- 1 file changed, 33 insertions(+), 12 deletions(-) diff --git a/server/src/internalClusterTest/java/org/opensearch/nodestats/NodeStatsIT.java b/server/src/internalClusterTest/java/org/opensearch/nodestats/NodeStatsIT.java index bbb5398c89aa7..1fd958293dca6 100644 --- a/server/src/internalClusterTest/java/org/opensearch/nodestats/NodeStatsIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/nodestats/NodeStatsIT.java @@ -348,11 +348,6 @@ public void testNodeIndicesStatsOptimizedResponse() { NodeIndicesStats.Fields.SHARDS, "unknown" ); - String testToXContentLevel = randomFrom( - NodeIndicesStats.Fields.NODE, - NodeIndicesStats.Fields.INDICES, - NodeIndicesStats.Fields.SHARDS - ); internalCluster().startNode(); ensureGreen(); String indexName = "test1"; @@ -373,9 +368,10 @@ public void testNodeIndicesStatsOptimizedResponse() { response.getNodes().forEach(nodeStats -> { try { XContentBuilder builder = XContentFactory.jsonBuilder(); + builder.startObject(); builder = nodeStats.getIndices() - .toXContent(builder, new ToXContent.MapParams(Collections.singletonMap("level", testToXContentLevel))); + .toXContent(builder, new ToXContent.MapParams(Collections.singletonMap("level", NodeIndicesStats.Fields.SHARDS))); builder.endObject(); Map xContentMap = xContentBuilderToMap(builder); @@ -384,18 +380,43 @@ public void testNodeIndicesStatsOptimizedResponse() { LinkedHashMap shardStats = (LinkedHashMap) indicesStatsMap.get(NodeIndicesStats.Fields.SHARDS); switch (testLevel) { case NodeIndicesStats.Fields.SHARDS: - assertFalse(shardStats == null || shardStats.isEmpty()); - assertFalse(indicesStats == null || indicesStats.isEmpty()); + assertFalse(shardStats.isEmpty()); + assertNull(indicesStats); + break; + case NodeIndicesStats.Fields.INDICES: + assertTrue(shardStats.isEmpty()); + assertNull(indicesStats); + break; + case NodeIndicesStats.Fields.NODE: + case "null": + case "unknown": + assertNull(shardStats); + assertNull(indicesStats); break; + } + + builder = XContentFactory.jsonBuilder(); + + builder.startObject(); + builder = nodeStats.getIndices() + .toXContent(builder, new ToXContent.MapParams(Collections.singletonMap("level", NodeIndicesStats.Fields.INDICES))); + builder.endObject(); + + xContentMap = xContentBuilderToMap(builder); + indicesStatsMap = (LinkedHashMap) xContentMap.get(NodeIndicesStats.Fields.INDICES); + indicesStats = (LinkedHashMap) indicesStatsMap.get(NodeIndicesStats.Fields.INDICES); + shardStats = (LinkedHashMap) indicesStatsMap.get(NodeIndicesStats.Fields.SHARDS); + switch (testLevel) { + case NodeIndicesStats.Fields.SHARDS: case NodeIndicesStats.Fields.INDICES: - assertTrue(shardStats == null || shardStats.isEmpty()); - assertFalse(indicesStats == null || indicesStats.isEmpty()); + assertNull(shardStats); + assertFalse(indicesStats.isEmpty()); break; case NodeIndicesStats.Fields.NODE: case "null": case "unknown": - assertTrue(shardStats == null || shardStats.isEmpty()); - assertTrue(indicesStats == null || indicesStats.isEmpty()); + assertNull(shardStats); + assertNull(indicesStats); break; } } catch (IOException e) { From 73de1883c0eceb4c6a06220397f2287ea9ee444b Mon Sep 17 00:00:00 2001 From: Pranshu Shukla Date: Mon, 15 Jul 2024 11:55:50 +0530 Subject: [PATCH 06/25] fix test for misc level param output Signed-off-by: Pranshu Shukla --- .../java/org/opensearch/nodestats/NodeStatsIT.java | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/server/src/internalClusterTest/java/org/opensearch/nodestats/NodeStatsIT.java b/server/src/internalClusterTest/java/org/opensearch/nodestats/NodeStatsIT.java index 1fd958293dca6..52fd36edc99a7 100644 --- a/server/src/internalClusterTest/java/org/opensearch/nodestats/NodeStatsIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/nodestats/NodeStatsIT.java @@ -384,13 +384,10 @@ public void testNodeIndicesStatsOptimizedResponse() { assertNull(indicesStats); break; case NodeIndicesStats.Fields.INDICES: - assertTrue(shardStats.isEmpty()); - assertNull(indicesStats); - break; case NodeIndicesStats.Fields.NODE: case "null": case "unknown": - assertNull(shardStats); + assertTrue(shardStats.isEmpty()); assertNull(indicesStats); break; } @@ -416,7 +413,7 @@ public void testNodeIndicesStatsOptimizedResponse() { case "null": case "unknown": assertNull(shardStats); - assertNull(indicesStats); + assertTrue(indicesStats.isEmpty()); break; } } catch (IOException e) { From 339bd12a876c889b1f26c3dd9c815efd14d583c5 Mon Sep 17 00:00:00 2001 From: Pranshu Shukla Date: Mon, 15 Jul 2024 14:23:16 +0530 Subject: [PATCH 07/25] Retry Build Signed-off-by: Pranshu Shukla From fa2373f7847192383de8d46a97152eeda25763e5 Mon Sep 17 00:00:00 2001 From: Pranshu Shukla Date: Mon, 15 Jul 2024 14:29:05 +0530 Subject: [PATCH 08/25] Retry Build Signed-off-by: Pranshu Shukla From b7db4bd57d43acedfc5320454e81059b71a5e347 Mon Sep 17 00:00:00 2001 From: Pranshu Shukla Date: Tue, 16 Jul 2024 10:52:24 +0530 Subject: [PATCH 09/25] Add ChangeLogs Signed-off-by: Pranshu Shukla --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 62bb73d80f2c1..c21cf5300d198 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), ## [Unreleased 2.x] ### Added +- Optimize NodeIndicesStats output behind flag ([#14454](https://github.com/opensearch-project/OpenSearch/pull/14454)) - Add fingerprint ingest processor ([#13724](https://github.com/opensearch-project/OpenSearch/pull/13724)) - [Remote Store] Rate limiter for remote store low priority uploads ([#14374](https://github.com/opensearch-project/OpenSearch/pull/14374/)) - Apply the date histogram rewrite optimization to range aggregation ([#13865](https://github.com/opensearch-project/OpenSearch/pull/13865)) From 40fa8ce78689751eb66f875cd074bd669a973c84 Mon Sep 17 00:00:00 2001 From: Pranshu Shukla Date: Wed, 17 Jul 2024 16:26:40 +0530 Subject: [PATCH 10/25] Adding code coverage Signed-off-by: Pranshu Shukla --- .../org/opensearch/nodestats/NodeStatsIT.java | 275 +++++++++--------- 1 file changed, 140 insertions(+), 135 deletions(-) diff --git a/server/src/internalClusterTest/java/org/opensearch/nodestats/NodeStatsIT.java b/server/src/internalClusterTest/java/org/opensearch/nodestats/NodeStatsIT.java index 52fd36edc99a7..98db912b547c2 100644 --- a/server/src/internalClusterTest/java/org/opensearch/nodestats/NodeStatsIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/nodestats/NodeStatsIT.java @@ -43,6 +43,7 @@ import java.util.Collections; import java.util.Comparator; import java.util.LinkedHashMap; +import java.util.List; import java.util.Map; import java.util.concurrent.atomic.AtomicLong; @@ -260,14 +261,14 @@ public void testNodeIndicesStatsDocStatusStatsCreateDeleteUpdate() { * Default behavior - without consideration of request level param on level, the NodeStatsRequest always * returns ShardStats which is aggregated on the coordinator node when creating the XContent. */ - public void testNodeIndicesStatsDefaultResponse() { - String testLevel = randomFrom( - "null", - NodeIndicesStats.Fields.NODE, - NodeIndicesStats.Fields.INDICES, - NodeIndicesStats.Fields.SHARDS, - "unknown" - ); + public void testNodeIndicesStatWithDiscoveryNodesListInRequest() { + List testLevels = new ArrayList<>(); + testLevels.add("null"); + testLevels.add(NodeIndicesStats.Fields.NODE); + testLevels.add(NodeIndicesStats.Fields.INDICES); + testLevels.add(NodeIndicesStats.Fields.SHARDS); + testLevels.add("unknown"); + internalCluster().startNode(); ensureGreen(); String indexName = "test1"; @@ -275,64 +276,66 @@ public void testNodeIndicesStatsDefaultResponse() { refresh(); ClusterState clusterState = client().admin().cluster().prepareState().get().getState(); - NodesStatsResponse response; - if (!testLevel.equals("null")) { - ArrayList level_arg = new ArrayList<>(); - level_arg.add(testLevel); - - CommonStatsFlags commonStatsFlags = new CommonStatsFlags(); - commonStatsFlags.setLevels(level_arg.toArray(new String[0])); - response = client().admin().cluster().prepareNodesStats().setIndices(commonStatsFlags).get(); - } else { - response = client().admin().cluster().prepareNodesStats().get(); - } - - response.getNodes().forEach(nodeStats -> { - assertNotNull(nodeStats.getIndices().getShardStats(clusterState.metadata().index(indexName).getIndex())); - try { - // Without any param - default is level = nodes - XContentBuilder builder = XContentFactory.jsonBuilder(); - builder.startObject(); - builder = nodeStats.getIndices().toXContent(builder, ToXContent.EMPTY_PARAMS); - builder.endObject(); - - Map xContentMap = xContentBuilderToMap(builder); - LinkedHashMap indicesStatsMap = (LinkedHashMap) xContentMap.get(NodeIndicesStats.Fields.INDICES); - assertFalse(indicesStatsMap.containsKey(NodeIndicesStats.Fields.INDICES)); - assertFalse(indicesStatsMap.containsKey(NodeIndicesStats.Fields.SHARDS)); - - // With param containing level as 'indices', the indices stats are returned - builder = XContentFactory.jsonBuilder(); - builder.startObject(); - builder = nodeStats.getIndices() - .toXContent(builder, new ToXContent.MapParams(Collections.singletonMap("level", NodeIndicesStats.Fields.INDICES))); - builder.endObject(); - - xContentMap = xContentBuilderToMap(builder); - indicesStatsMap = (LinkedHashMap) xContentMap.get(NodeIndicesStats.Fields.INDICES); - assertTrue(indicesStatsMap.containsKey(NodeIndicesStats.Fields.INDICES)); - assertFalse(indicesStatsMap.containsKey(NodeIndicesStats.Fields.SHARDS)); - - LinkedHashMap indexLevelStats = (LinkedHashMap) indicesStatsMap.get(NodeIndicesStats.Fields.INDICES); - assertTrue(indexLevelStats.containsKey(indexName)); - - // With param containing level as 'shards', the shard stats are returned - builder = XContentFactory.jsonBuilder(); - builder.startObject(); - builder = nodeStats.getIndices() - .toXContent(builder, new ToXContent.MapParams(Collections.singletonMap("level", NodeIndicesStats.Fields.SHARDS))); - builder.endObject(); - - xContentMap = xContentBuilderToMap(builder); - indicesStatsMap = (LinkedHashMap) xContentMap.get(NodeIndicesStats.Fields.INDICES); - assertFalse(indicesStatsMap.containsKey(NodeIndicesStats.Fields.INDICES)); - assertTrue(indicesStatsMap.containsKey(NodeIndicesStats.Fields.SHARDS)); - - LinkedHashMap shardLevelStats = (LinkedHashMap) indicesStatsMap.get(NodeIndicesStats.Fields.SHARDS); - assertTrue(shardLevelStats.containsKey(indexName)); - } catch (IOException e) { - throw new RuntimeException(e); + testLevels.forEach(testLevel -> { + NodesStatsResponse response; + if (!testLevel.equals("null")) { + ArrayList level_arg = new ArrayList<>(); + level_arg.add(testLevel); + + CommonStatsFlags commonStatsFlags = new CommonStatsFlags(); + commonStatsFlags.setLevels(level_arg.toArray(new String[0])); + response = client().admin().cluster().prepareNodesStats().setIndices(commonStatsFlags).get(); + } else { + response = client().admin().cluster().prepareNodesStats().get(); } + + response.getNodes().forEach(nodeStats -> { + assertNotNull(nodeStats.getIndices().getShardStats(clusterState.metadata().index(indexName).getIndex())); + try { + // Without any param - default is level = nodes + XContentBuilder builder = XContentFactory.jsonBuilder(); + builder.startObject(); + builder = nodeStats.getIndices().toXContent(builder, ToXContent.EMPTY_PARAMS); + builder.endObject(); + + Map xContentMap = xContentBuilderToMap(builder); + LinkedHashMap indicesStatsMap = (LinkedHashMap) xContentMap.get(NodeIndicesStats.Fields.INDICES); + assertFalse(indicesStatsMap.containsKey(NodeIndicesStats.Fields.INDICES)); + assertFalse(indicesStatsMap.containsKey(NodeIndicesStats.Fields.SHARDS)); + + // With param containing level as 'indices', the indices stats are returned + builder = XContentFactory.jsonBuilder(); + builder.startObject(); + builder = nodeStats.getIndices() + .toXContent(builder, new ToXContent.MapParams(Collections.singletonMap("level", NodeIndicesStats.Fields.INDICES))); + builder.endObject(); + + xContentMap = xContentBuilderToMap(builder); + indicesStatsMap = (LinkedHashMap) xContentMap.get(NodeIndicesStats.Fields.INDICES); + assertTrue(indicesStatsMap.containsKey(NodeIndicesStats.Fields.INDICES)); + assertFalse(indicesStatsMap.containsKey(NodeIndicesStats.Fields.SHARDS)); + + LinkedHashMap indexLevelStats = (LinkedHashMap) indicesStatsMap.get(NodeIndicesStats.Fields.INDICES); + assertTrue(indexLevelStats.containsKey(indexName)); + + // With param containing level as 'shards', the shard stats are returned + builder = XContentFactory.jsonBuilder(); + builder.startObject(); + builder = nodeStats.getIndices() + .toXContent(builder, new ToXContent.MapParams(Collections.singletonMap("level", NodeIndicesStats.Fields.SHARDS))); + builder.endObject(); + + xContentMap = xContentBuilderToMap(builder); + indicesStatsMap = (LinkedHashMap) xContentMap.get(NodeIndicesStats.Fields.INDICES); + assertFalse(indicesStatsMap.containsKey(NodeIndicesStats.Fields.INDICES)); + assertTrue(indicesStatsMap.containsKey(NodeIndicesStats.Fields.SHARDS)); + + LinkedHashMap shardLevelStats = (LinkedHashMap) indicesStatsMap.get(NodeIndicesStats.Fields.SHARDS); + assertTrue(shardLevelStats.containsKey(indexName)); + } catch (IOException e) { + throw new RuntimeException(e); + } + }); }); } @@ -340,85 +343,87 @@ public void testNodeIndicesStatsDefaultResponse() { * Optimized behavior - to avoid unnecessary IO in the form of shard-stats when not required, we not honor the levels on the * individual data nodes instead and pre-compute information as required. */ - public void testNodeIndicesStatsOptimizedResponse() { - String testLevel = randomFrom( - "null", - NodeIndicesStats.Fields.NODE, - NodeIndicesStats.Fields.INDICES, - NodeIndicesStats.Fields.SHARDS, - "unknown" - ); + public void testNodeIndicesStatWithDiscoveryNodesListNotInRequest() { + List testLevels = new ArrayList<>(); + testLevels.add("null"); + testLevels.add(NodeIndicesStats.Fields.NODE); + testLevels.add(NodeIndicesStats.Fields.INDICES); + testLevels.add(NodeIndicesStats.Fields.SHARDS); + testLevels.add("unknown"); + internalCluster().startNode(); ensureGreen(); String indexName = "test1"; index(indexName, "type", "1", "f", "f"); refresh(); - NodesStatsResponse response; - CommonStatsFlags commonStatsFlags = new CommonStatsFlags(); - commonStatsFlags.optimizeNodeIndicesStatsOnLevel(true); - if (!testLevel.equals("null")) { - ArrayList level_arg = new ArrayList<>(); - level_arg.add(testLevel); + testLevels.forEach(testLevel -> { + NodesStatsResponse response; + CommonStatsFlags commonStatsFlags = new CommonStatsFlags(); + commonStatsFlags.optimizeNodeIndicesStatsOnLevel(true); + if (!testLevel.equals("null")) { + ArrayList level_arg = new ArrayList<>(); + level_arg.add(testLevel); - commonStatsFlags.setLevels(level_arg.toArray(new String[0])); - } - response = client().admin().cluster().prepareNodesStats().setIndices(commonStatsFlags).get(); - - response.getNodes().forEach(nodeStats -> { - try { - XContentBuilder builder = XContentFactory.jsonBuilder(); - - builder.startObject(); - builder = nodeStats.getIndices() - .toXContent(builder, new ToXContent.MapParams(Collections.singletonMap("level", NodeIndicesStats.Fields.SHARDS))); - builder.endObject(); - - Map xContentMap = xContentBuilderToMap(builder); - LinkedHashMap indicesStatsMap = (LinkedHashMap) xContentMap.get(NodeIndicesStats.Fields.INDICES); - LinkedHashMap indicesStats = (LinkedHashMap) indicesStatsMap.get(NodeIndicesStats.Fields.INDICES); - LinkedHashMap shardStats = (LinkedHashMap) indicesStatsMap.get(NodeIndicesStats.Fields.SHARDS); - switch (testLevel) { - case NodeIndicesStats.Fields.SHARDS: - assertFalse(shardStats.isEmpty()); - assertNull(indicesStats); - break; - case NodeIndicesStats.Fields.INDICES: - case NodeIndicesStats.Fields.NODE: - case "null": - case "unknown": - assertTrue(shardStats.isEmpty()); - assertNull(indicesStats); - break; - } + commonStatsFlags.setLevels(level_arg.toArray(new String[0])); + } + response = client().admin().cluster().prepareNodesStats().setIndices(commonStatsFlags).get(); - builder = XContentFactory.jsonBuilder(); - - builder.startObject(); - builder = nodeStats.getIndices() - .toXContent(builder, new ToXContent.MapParams(Collections.singletonMap("level", NodeIndicesStats.Fields.INDICES))); - builder.endObject(); - - xContentMap = xContentBuilderToMap(builder); - indicesStatsMap = (LinkedHashMap) xContentMap.get(NodeIndicesStats.Fields.INDICES); - indicesStats = (LinkedHashMap) indicesStatsMap.get(NodeIndicesStats.Fields.INDICES); - shardStats = (LinkedHashMap) indicesStatsMap.get(NodeIndicesStats.Fields.SHARDS); - switch (testLevel) { - case NodeIndicesStats.Fields.SHARDS: - case NodeIndicesStats.Fields.INDICES: - assertNull(shardStats); - assertFalse(indicesStats.isEmpty()); - break; - case NodeIndicesStats.Fields.NODE: - case "null": - case "unknown": - assertNull(shardStats); - assertTrue(indicesStats.isEmpty()); - break; + response.getNodes().forEach(nodeStats -> { + try { + XContentBuilder builder = XContentFactory.jsonBuilder(); + + builder.startObject(); + builder = nodeStats.getIndices() + .toXContent(builder, new ToXContent.MapParams(Collections.singletonMap("level", NodeIndicesStats.Fields.SHARDS))); + builder.endObject(); + + Map xContentMap = xContentBuilderToMap(builder); + LinkedHashMap indicesStatsMap = (LinkedHashMap) xContentMap.get(NodeIndicesStats.Fields.INDICES); + LinkedHashMap indicesStats = (LinkedHashMap) indicesStatsMap.get(NodeIndicesStats.Fields.INDICES); + LinkedHashMap shardStats = (LinkedHashMap) indicesStatsMap.get(NodeIndicesStats.Fields.SHARDS); + switch (testLevel) { + case NodeIndicesStats.Fields.SHARDS: + assertFalse(shardStats.isEmpty()); + assertNull(indicesStats); + break; + case NodeIndicesStats.Fields.INDICES: + case NodeIndicesStats.Fields.NODE: + case "null": + case "unknown": + assertTrue(shardStats.isEmpty()); + assertNull(indicesStats); + break; + } + + builder = XContentFactory.jsonBuilder(); + + builder.startObject(); + builder = nodeStats.getIndices() + .toXContent(builder, new ToXContent.MapParams(Collections.singletonMap("level", NodeIndicesStats.Fields.INDICES))); + builder.endObject(); + + xContentMap = xContentBuilderToMap(builder); + indicesStatsMap = (LinkedHashMap) xContentMap.get(NodeIndicesStats.Fields.INDICES); + indicesStats = (LinkedHashMap) indicesStatsMap.get(NodeIndicesStats.Fields.INDICES); + shardStats = (LinkedHashMap) indicesStatsMap.get(NodeIndicesStats.Fields.SHARDS); + switch (testLevel) { + case NodeIndicesStats.Fields.SHARDS: + case NodeIndicesStats.Fields.INDICES: + assertNull(shardStats); + assertFalse(indicesStats.isEmpty()); + break; + case NodeIndicesStats.Fields.NODE: + case "null": + case "unknown": + assertNull(shardStats); + assertTrue(indicesStats.isEmpty()); + break; + } + } catch (IOException e) { + throw new RuntimeException(e); } - } catch (IOException e) { - throw new RuntimeException(e); - } + }); }); } From 8aef256c13c2afe24e24424af872603eef0093cb Mon Sep 17 00:00:00 2001 From: Pranshu Shukla Date: Wed, 17 Jul 2024 21:57:35 +0530 Subject: [PATCH 11/25] Fix Flakyness of tests Signed-off-by: Pranshu Shukla --- .../org/opensearch/nodestats/NodeStatsIT.java | 22 +++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/server/src/internalClusterTest/java/org/opensearch/nodestats/NodeStatsIT.java b/server/src/internalClusterTest/java/org/opensearch/nodestats/NodeStatsIT.java index 98db912b547c2..b36306188c355 100644 --- a/server/src/internalClusterTest/java/org/opensearch/nodestats/NodeStatsIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/nodestats/NodeStatsIT.java @@ -22,6 +22,7 @@ import org.opensearch.action.update.UpdateRequest; import org.opensearch.action.update.UpdateResponse; import org.opensearch.cluster.ClusterState; +import org.opensearch.common.settings.Settings; import org.opensearch.common.xcontent.XContentFactory; import org.opensearch.common.xcontent.XContentHelper; import org.opensearch.core.common.bytes.BytesReference; @@ -48,6 +49,7 @@ import java.util.concurrent.atomic.AtomicLong; import static java.util.Collections.singletonMap; +import static org.opensearch.test.hamcrest.OpenSearchAssertions.assertAcked; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.greaterThanOrEqualTo; @@ -272,8 +274,14 @@ public void testNodeIndicesStatWithDiscoveryNodesListInRequest() { internalCluster().startNode(); ensureGreen(); String indexName = "test1"; - index(indexName, "type", "1", "f", "f"); - refresh(); + assertAcked( + prepareCreate( + indexName, + clusterService().state().getNodes().getSize(), + Settings.builder().put("number_of_shards", 2).put("number_of_replicas", clusterService().state().getNodes().getSize() - 1) + ) + ); + ensureGreen(); ClusterState clusterState = client().admin().cluster().prepareState().get().getState(); testLevels.forEach(testLevel -> { @@ -354,8 +362,14 @@ public void testNodeIndicesStatWithDiscoveryNodesListNotInRequest() { internalCluster().startNode(); ensureGreen(); String indexName = "test1"; - index(indexName, "type", "1", "f", "f"); - refresh(); + assertAcked( + prepareCreate( + indexName, + clusterService().state().getNodes().getSize(), + Settings.builder().put("number_of_shards", 2).put("number_of_replicas", clusterService().state().getNodes().getSize() - 1) + ) + ); + ensureGreen(); testLevels.forEach(testLevel -> { NodesStatsResponse response; From 6672c25d7e46b1fe2067915cb9587e036df2d6db Mon Sep 17 00:00:00 2001 From: Pranshu Shukla Date: Thu, 18 Jul 2024 10:05:35 +0530 Subject: [PATCH 12/25] Adding UTs for code coverage Signed-off-by: Pranshu Shukla --- .../cluster/node/stats/NodeStatsTests.java | 119 ++++++++++++++---- 1 file changed, 98 insertions(+), 21 deletions(-) diff --git a/server/src/test/java/org/opensearch/action/admin/cluster/node/stats/NodeStatsTests.java b/server/src/test/java/org/opensearch/action/admin/cluster/node/stats/NodeStatsTests.java index 010f28f498c05..d99e50b19e4fc 100644 --- a/server/src/test/java/org/opensearch/action/admin/cluster/node/stats/NodeStatsTests.java +++ b/server/src/test/java/org/opensearch/action/admin/cluster/node/stats/NodeStatsTests.java @@ -58,11 +58,16 @@ import org.opensearch.common.metrics.OperationStats; import org.opensearch.common.settings.ClusterSettings; import org.opensearch.common.settings.Settings; +import org.opensearch.common.xcontent.XContentFactory; +import org.opensearch.common.xcontent.XContentHelper; +import org.opensearch.core.common.bytes.BytesReference; import org.opensearch.core.common.io.stream.StreamInput; import org.opensearch.core.index.Index; import org.opensearch.core.index.shard.ShardId; import org.opensearch.core.indices.breaker.AllCircuitBreakerStats; import org.opensearch.core.indices.breaker.CircuitBreakerStats; +import org.opensearch.core.xcontent.ToXContent; +import org.opensearch.core.xcontent.XContentBuilder; import org.opensearch.discovery.DiscoveryStats; import org.opensearch.gateway.remote.RemotePersistenceStats; import org.opensearch.http.HttpStats; @@ -108,6 +113,7 @@ import java.util.List; import java.util.Map; import java.util.TreeMap; +import java.util.LinkedHashMap; import java.util.function.Function; import java.util.stream.Collectors; import java.util.stream.StreamSupport; @@ -1159,8 +1165,66 @@ public void testNodeIndicesStatsSerializationWithOldESVersionNodes() throws IOEx public void testNodeIndicesStatsSerializationOnNewVersions() throws IOException { long numDocs = randomLongBetween(0, 10000); long numDeletedDocs = randomLongBetween(0, 100); - String levelParam = randomFrom(NodeIndicesStats.Fields.INDICES, "indices", "shards"); + List levelParams = new ArrayList<>(); + levelParams.add(NodeIndicesStats.Fields.INDICES); + levelParams.add(NodeIndicesStats.Fields.SHARDS); + levelParams.add(NodeIndicesStats.Fields.NODE); + CommonStats commonStats = new CommonStats(CommonStatsFlags.NONE); + + commonStats.docs = new DocsStats(numDocs, numDeletedDocs, 0); + commonStats.store = new StoreStats(100, 0L); + commonStats.indexing = new IndexingStats(); + + CommonStatsFlags commonStatsFlags = new CommonStatsFlags(); + commonStatsFlags.clear(); + commonStatsFlags.set(CommonStatsFlags.Flag.Docs, true); + commonStatsFlags.set(CommonStatsFlags.Flag.Store, true); + commonStatsFlags.set(CommonStatsFlags.Flag.Indexing, true); + commonStatsFlags.optimizeNodeIndicesStatsOnLevel(true); + levelParams.forEach(levelParam -> { + ArrayList level_arg = new ArrayList<>(); + level_arg.add(levelParam); + + commonStatsFlags.setLevels(level_arg.toArray(new String[0])); + + Index newIndex = new Index("index", "_na_"); + + MockNodeIndicesStats mockNodeIndicesStats = generateMockNodeIndicesStats(commonStats, newIndex, commonStatsFlags); + + // To test out scenario when the incoming node stats response is from a node with an older ES Version. + try (BytesStreamOutput out = new BytesStreamOutput()) { + mockNodeIndicesStats.writeTo(out); + try (StreamInput in = out.bytes().streamInput()) { + MockNodeIndicesStats newNodeIndicesStats = new MockNodeIndicesStats(in); + switch (levelParam) { + case "node": + assertNull(newNodeIndicesStats.getStatsByIndex()); + assertNull(newNodeIndicesStats.getStatsByShard()); + break; + case "indices": + assertNull(newNodeIndicesStats.getStatsByShard()); + assertNotNull(newNodeIndicesStats.getStatsByIndex()); + break; + case "shards": + assertNull(newNodeIndicesStats.getStatsByIndex()); + assertNotNull(newNodeIndicesStats.getStatsByShard()); + break; + } + } + } catch (IOException e) { + throw new RuntimeException(e); + } + }); + } + + public void testNodeIndicesStatsToXContent() { + long numDocs = randomLongBetween(0, 10000); + long numDeletedDocs = randomLongBetween(0, 100); + List levelParams = new ArrayList<>(); + levelParams.add(NodeIndicesStats.Fields.INDICES); + levelParams.add(NodeIndicesStats.Fields.SHARDS); + levelParams.add(NodeIndicesStats.Fields.NODE); CommonStats commonStats = new CommonStats(CommonStatsFlags.NONE); commonStats.docs = new DocsStats(numDocs, numDeletedDocs, 0); @@ -1174,38 +1238,51 @@ public void testNodeIndicesStatsSerializationOnNewVersions() throws IOException commonStatsFlags.set(CommonStatsFlags.Flag.Indexing, true); commonStatsFlags.optimizeNodeIndicesStatsOnLevel(true); - ArrayList level_arg = new ArrayList<>(); - level_arg.add(levelParam); + levelParams.forEach(levelParam -> { + ArrayList level_arg = new ArrayList<>(); + level_arg.add(levelParam); - commonStatsFlags.setLevels(level_arg.toArray(new String[0])); + commonStatsFlags.setLevels(level_arg.toArray(new String[0])); - Index newIndex = new Index("index", "_na_"); + Index newIndex = new Index("index", "_na_"); - MockNodeIndicesStats mockNodeIndicesStats = generateMockNodeIndicesStats(commonStats, newIndex, commonStatsFlags); + MockNodeIndicesStats mockNodeIndicesStats = generateMockNodeIndicesStats(commonStats, newIndex, commonStatsFlags); + + XContentBuilder builder = null; + try { + builder = XContentFactory.jsonBuilder(); + builder.startObject(); + builder = mockNodeIndicesStats.toXContent(builder, + new ToXContent.MapParams(Collections.singletonMap("level", levelParam))); + builder.endObject(); + + Map xContentMap = xContentBuilderToMap(builder); + LinkedHashMap indicesStatsMap = (LinkedHashMap) xContentMap.get(NodeIndicesStats.Fields.INDICES); - // To test out scenario when the incoming node stats response is from a node with an older ES Version. - try (BytesStreamOutput out = new BytesStreamOutput()) { - mockNodeIndicesStats.writeTo(out); - try (StreamInput in = out.bytes().streamInput()) { - MockNodeIndicesStats newNodeIndicesStats = new MockNodeIndicesStats(in); switch (levelParam) { - case "node": - assertNull(newNodeIndicesStats.getStatsByIndex()); - assertNull(newNodeIndicesStats.getStatsByShard()); + case NodeIndicesStats.Fields.NODE: + assertFalse(indicesStatsMap.containsKey(NodeIndicesStats.Fields.INDICES)); + assertFalse(indicesStatsMap.containsKey(NodeIndicesStats.Fields.SHARDS)); break; - case "indices": - assertNull(newNodeIndicesStats.getStatsByShard()); - assertNotNull(newNodeIndicesStats.getStatsByIndex()); + case NodeIndicesStats.Fields.INDICES: + assertTrue(indicesStatsMap.containsKey(NodeIndicesStats.Fields.INDICES)); + assertFalse(indicesStatsMap.containsKey(NodeIndicesStats.Fields.SHARDS)); break; - case "shards": - assertNull(newNodeIndicesStats.getStatsByIndex()); - assertNotNull(newNodeIndicesStats.getStatsByShard()); + case NodeIndicesStats.Fields.SHARDS: + assertFalse(indicesStatsMap.containsKey(NodeIndicesStats.Fields.INDICES)); + assertTrue(indicesStatsMap.containsKey(NodeIndicesStats.Fields.SHARDS)); break; } + } catch (IOException e) { + throw new RuntimeException(e); } - } + + }); } + private Map xContentBuilderToMap(XContentBuilder xContentBuilder) { + return XContentHelper.convertToMap(BytesReference.bytes(xContentBuilder), true, xContentBuilder.contentType()).v2(); + } public MockNodeIndicesStats generateMockNodeIndicesStats(CommonStats commonStats, Index index, CommonStatsFlags commonStatsFlags) { DiscoveryNode localNode = new DiscoveryNode("local", buildNewFakeTransportAddress(), Version.CURRENT); Map> statsByShard = new HashMap<>(); From c148d0fc53851169d94d8dd1d63349f8f8d8c78e Mon Sep 17 00:00:00 2001 From: Pranshu Shukla Date: Thu, 18 Jul 2024 10:14:01 +0530 Subject: [PATCH 13/25] fix spotless checks and more UTs Signed-off-by: Pranshu Shukla --- .../action/RestStatsActionTests.java | 48 +++++++++++++++++++ .../cluster/node/stats/NodeStatsTests.java | 6 +-- 2 files changed, 51 insertions(+), 3 deletions(-) create mode 100644 server/src/test/java/org/opensearch/action/RestStatsActionTests.java diff --git a/server/src/test/java/org/opensearch/action/RestStatsActionTests.java b/server/src/test/java/org/opensearch/action/RestStatsActionTests.java new file mode 100644 index 0000000000000..df38d601f8c92 --- /dev/null +++ b/server/src/test/java/org/opensearch/action/RestStatsActionTests.java @@ -0,0 +1,48 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.action; + +import org.opensearch.client.node.NodeClient; +import org.opensearch.common.settings.Settings; +import org.opensearch.common.settings.SettingsFilter; +import org.opensearch.rest.action.admin.cluster.RestClusterStatsAction; +import org.opensearch.rest.action.admin.cluster.RestNodesInfoAction; +import org.opensearch.rest.action.admin.cluster.RestNodesStatsAction; +import org.opensearch.test.OpenSearchTestCase; +import org.opensearch.test.rest.FakeRestRequest; +import org.opensearch.threadpool.TestThreadPool; +import org.junit.After; + +import java.io.IOException; +import java.util.Collections; + +public class RestStatsActionTests extends OpenSearchTestCase { + private final TestThreadPool threadPool = new TestThreadPool(RestStatsActionTests.class.getName()); + private final NodeClient client = new NodeClient(Settings.EMPTY, threadPool); + + @After + public void terminateThreadPool() { + terminate(threadPool); + } + + public void testClusterStatsActionPrepareRequestNoError() throws IOException { + RestClusterStatsAction action = new RestClusterStatsAction(); + action.prepareRequest(new FakeRestRequest(), client); + } + + public void testNodesStatsActionPrepareRequestNoError() throws IOException { + RestNodesStatsAction action = new RestNodesStatsAction(); + action.prepareRequest(new FakeRestRequest(), client); + } + + public void testNodesInfoActionPrepareRequestNoError() throws IOException { + RestNodesInfoAction action = new RestNodesInfoAction(new SettingsFilter(Collections.singleton("foo.filtered"))); + action.prepareRequest(new FakeRestRequest(), client); + } +} diff --git a/server/src/test/java/org/opensearch/action/admin/cluster/node/stats/NodeStatsTests.java b/server/src/test/java/org/opensearch/action/admin/cluster/node/stats/NodeStatsTests.java index d99e50b19e4fc..40c86995c1cea 100644 --- a/server/src/test/java/org/opensearch/action/admin/cluster/node/stats/NodeStatsTests.java +++ b/server/src/test/java/org/opensearch/action/admin/cluster/node/stats/NodeStatsTests.java @@ -110,10 +110,10 @@ import java.util.Collections; import java.util.HashMap; import java.util.Iterator; +import java.util.LinkedHashMap; import java.util.List; import java.util.Map; import java.util.TreeMap; -import java.util.LinkedHashMap; import java.util.function.Function; import java.util.stream.Collectors; import java.util.stream.StreamSupport; @@ -1252,8 +1252,7 @@ public void testNodeIndicesStatsToXContent() { try { builder = XContentFactory.jsonBuilder(); builder.startObject(); - builder = mockNodeIndicesStats.toXContent(builder, - new ToXContent.MapParams(Collections.singletonMap("level", levelParam))); + builder = mockNodeIndicesStats.toXContent(builder, new ToXContent.MapParams(Collections.singletonMap("level", levelParam))); builder.endObject(); Map xContentMap = xContentBuilderToMap(builder); @@ -1283,6 +1282,7 @@ public void testNodeIndicesStatsToXContent() { private Map xContentBuilderToMap(XContentBuilder xContentBuilder) { return XContentHelper.convertToMap(BytesReference.bytes(xContentBuilder), true, xContentBuilder.contentType()).v2(); } + public MockNodeIndicesStats generateMockNodeIndicesStats(CommonStats commonStats, Index index, CommonStatsFlags commonStatsFlags) { DiscoveryNode localNode = new DiscoveryNode("local", buildNewFakeTransportAddress(), Version.CURRENT); Map> statsByShard = new HashMap<>(); From 8416ec8d70100fb1f2ab340d1973a9f7bb520fee Mon Sep 17 00:00:00 2001 From: Pranshu Shukla Date: Thu, 18 Jul 2024 16:36:10 +0530 Subject: [PATCH 14/25] Refactoring naming Signed-off-by: Pranshu Shukla --- .../java/org/opensearch/nodestats/NodeStatsIT.java | 8 ++++---- .../cluster/node/stats/NodesStatsRequest.java | 6 +++--- .../admin/indices/stats/CommonStatsFlags.java | 14 +++++++------- .../org/opensearch/indices/IndicesService.java | 2 +- .../action/admin/cluster/RestNodesStatsAction.java | 2 +- .../rest/action/cat/RestNodesAction.java | 2 +- .../admin/cluster/node/stats/NodeStatsTests.java | 10 +++++----- 7 files changed, 22 insertions(+), 22 deletions(-) diff --git a/server/src/internalClusterTest/java/org/opensearch/nodestats/NodeStatsIT.java b/server/src/internalClusterTest/java/org/opensearch/nodestats/NodeStatsIT.java index b36306188c355..e7ce7006b10ab 100644 --- a/server/src/internalClusterTest/java/org/opensearch/nodestats/NodeStatsIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/nodestats/NodeStatsIT.java @@ -263,7 +263,7 @@ public void testNodeIndicesStatsDocStatusStatsCreateDeleteUpdate() { * Default behavior - without consideration of request level param on level, the NodeStatsRequest always * returns ShardStats which is aggregated on the coordinator node when creating the XContent. */ - public void testNodeIndicesStatWithDiscoveryNodesListInRequest() { + public void testNodeIndicesStatsWithoutAggregationOnNodes() { List testLevels = new ArrayList<>(); testLevels.add("null"); testLevels.add(NodeIndicesStats.Fields.NODE); @@ -348,10 +348,10 @@ public void testNodeIndicesStatWithDiscoveryNodesListInRequest() { } /** - * Optimized behavior - to avoid unnecessary IO in the form of shard-stats when not required, we not honor the levels on the + * Aggregated behavior - to avoid unnecessary IO in the form of shard-stats when not required, we not honor the levels on the * individual data nodes instead and pre-compute information as required. */ - public void testNodeIndicesStatWithDiscoveryNodesListNotInRequest() { + public void testNodeIndicesStatsWithAggregationOnNodes() { List testLevels = new ArrayList<>(); testLevels.add("null"); testLevels.add(NodeIndicesStats.Fields.NODE); @@ -374,7 +374,7 @@ public void testNodeIndicesStatWithDiscoveryNodesListNotInRequest() { testLevels.forEach(testLevel -> { NodesStatsResponse response; CommonStatsFlags commonStatsFlags = new CommonStatsFlags(); - commonStatsFlags.optimizeNodeIndicesStatsOnLevel(true); + commonStatsFlags.aggregateNodeResponsesOnLevel(true); if (!testLevel.equals("null")) { ArrayList level_arg = new ArrayList<>(); level_arg.add(testLevel); diff --git a/server/src/main/java/org/opensearch/action/admin/cluster/node/stats/NodesStatsRequest.java b/server/src/main/java/org/opensearch/action/admin/cluster/node/stats/NodesStatsRequest.java index 970c4340dd33b..27548673bca50 100644 --- a/server/src/main/java/org/opensearch/action/admin/cluster/node/stats/NodesStatsRequest.java +++ b/server/src/main/java/org/opensearch/action/admin/cluster/node/stats/NodesStatsRequest.java @@ -128,11 +128,11 @@ public NodesStatsRequest indices(boolean indices) { } /** - * Use Optimized Response filtered based on level + * Use Aggregated Response based on level passed */ - public NodesStatsRequest useOptimizedNodeIndicesStats(boolean useOptimizedNodeIndicesStats) { + public NodesStatsRequest aggregateResponsesOnLevel(boolean aggregateNodeResponsesOnLevel) { if (this.indices != null) { - this.indices.optimizeNodeIndicesStatsOnLevel(true); + this.indices.aggregateNodeResponsesOnLevel(aggregateNodeResponsesOnLevel); } return this; } 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 1404ba3132f3a..0a28be5e17dfe 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 @@ -67,7 +67,7 @@ public class CommonStatsFlags implements Writeable, Cloneable { // Used for metric CACHE_STATS, to determine which caches to report stats for private EnumSet includeCaches = EnumSet.noneOf(CacheType.class); private String[] levels = new String[0]; - private boolean optimizeNodeIndicesStatsOnLevel = false; + private boolean aggregateNodeResponsesOnLevel = false; /** * @param flags flags to set. If no flags are supplied, default flags will be set. @@ -102,7 +102,7 @@ public CommonStatsFlags(StreamInput in) throws IOException { levels = in.readStringArray(); } if (in.getVersion().onOrAfter(Version.V_3_0_0)) { - optimizeNodeIndicesStatsOnLevel = in.readBoolean(); + aggregateNodeResponsesOnLevel = in.readBoolean(); } } @@ -129,7 +129,7 @@ public void writeTo(StreamOutput out) throws IOException { out.writeStringArrayNullable(levels); } if (out.getVersion().onOrAfter(Version.V_3_0_0)) { - out.writeBoolean(optimizeNodeIndicesStatsOnLevel); + out.writeBoolean(aggregateNodeResponsesOnLevel); } } @@ -269,12 +269,12 @@ public boolean includeSegmentFileSizes() { return this.includeSegmentFileSizes; } - public void optimizeNodeIndicesStatsOnLevel(boolean optimizeNodeIndicesStatsOnLevel) { - this.optimizeNodeIndicesStatsOnLevel = optimizeNodeIndicesStatsOnLevel; + public void aggregateNodeResponsesOnLevel(boolean aggregateNodeResponsesOnLevel) { + this.aggregateNodeResponsesOnLevel = aggregateNodeResponsesOnLevel; } - public boolean optimizeNodeIndicesStatsOnLevel() { - return this.optimizeNodeIndicesStatsOnLevel; + public boolean aggregateNodeResponsesOnLevel() { + return this.aggregateNodeResponsesOnLevel; } public boolean isSet(Flag flag) { diff --git a/server/src/main/java/org/opensearch/indices/IndicesService.java b/server/src/main/java/org/opensearch/indices/IndicesService.java index ec4004bb6bb88..3ca39b0c5cb6a 100644 --- a/server/src/main/java/org/opensearch/indices/IndicesService.java +++ b/server/src/main/java/org/opensearch/indices/IndicesService.java @@ -692,7 +692,7 @@ public NodeIndicesStats stats(CommonStatsFlags flags) { break; } } - if (flags.optimizeNodeIndicesStatsOnLevel()) { + if (flags.aggregateNodeResponsesOnLevel()) { return new NodeIndicesStats(commonStats, statsByShard(this, flags), searchRequestStats, flags.getLevels()); } return new NodeIndicesStats(commonStats, statsByShard(this, flags), searchRequestStats); 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 1bac32696b325..2caba1a1376e1 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 @@ -232,7 +232,7 @@ public RestChannelConsumer prepareRequest(final RestRequest request, final NodeC // If no levels are passed in this results in an empty array. String[] levels = Strings.splitStringByCommaToArray(request.param("level")); nodesStatsRequest.indices().setLevels(levels); - nodesStatsRequest.indices().optimizeNodeIndicesStatsOnLevel(true); + nodesStatsRequest.indices().aggregateNodeResponsesOnLevel(true); return channel -> client.admin().cluster().nodesStats(nodesStatsRequest, new NodesResponseRestListener<>(channel)); } diff --git a/server/src/main/java/org/opensearch/rest/action/cat/RestNodesAction.java b/server/src/main/java/org/opensearch/rest/action/cat/RestNodesAction.java index cd5d76a29047d..a45352326ce04 100644 --- a/server/src/main/java/org/opensearch/rest/action/cat/RestNodesAction.java +++ b/server/src/main/java/org/opensearch/rest/action/cat/RestNodesAction.java @@ -139,7 +139,7 @@ public void processResponse(final NodesInfoResponse nodesInfoResponse) { nodesStatsRequest.timeout(request.param("timeout")); nodesStatsRequest.clear() .indices(true) - .useOptimizedNodeIndicesStats(true) + .aggregateResponsesOnLevel(true) .addMetrics( NodesStatsRequest.Metric.JVM.metricName(), NodesStatsRequest.Metric.OS.metricName(), diff --git a/server/src/test/java/org/opensearch/action/admin/cluster/node/stats/NodeStatsTests.java b/server/src/test/java/org/opensearch/action/admin/cluster/node/stats/NodeStatsTests.java index 40c86995c1cea..300d5dc990135 100644 --- a/server/src/test/java/org/opensearch/action/admin/cluster/node/stats/NodeStatsTests.java +++ b/server/src/test/java/org/opensearch/action/admin/cluster/node/stats/NodeStatsTests.java @@ -1120,7 +1120,7 @@ public Map> getStatsByShard() { } } - public void testNodeIndicesStatsSerializationWithOldESVersionNodes() throws IOException { + public void testNodeIndicesStatsSerializationWithOldESVersionNodesWithoutAggregations() throws IOException { long numDocs = randomLongBetween(0, 10000); long numDeletedDocs = randomLongBetween(0, 100); CommonStats commonStats = new CommonStats(CommonStatsFlags.NONE); @@ -1162,7 +1162,7 @@ public void testNodeIndicesStatsSerializationWithOldESVersionNodes() throws IOEx } } - public void testNodeIndicesStatsSerializationOnNewVersions() throws IOException { + public void testNodeIndicesStatsSerialization() throws IOException { long numDocs = randomLongBetween(0, 10000); long numDeletedDocs = randomLongBetween(0, 100); List levelParams = new ArrayList<>(); @@ -1180,7 +1180,7 @@ public void testNodeIndicesStatsSerializationOnNewVersions() throws IOException commonStatsFlags.set(CommonStatsFlags.Flag.Docs, true); commonStatsFlags.set(CommonStatsFlags.Flag.Store, true); commonStatsFlags.set(CommonStatsFlags.Flag.Indexing, true); - commonStatsFlags.optimizeNodeIndicesStatsOnLevel(true); + commonStatsFlags.aggregateNodeResponsesOnLevel(true); levelParams.forEach(levelParam -> { ArrayList level_arg = new ArrayList<>(); @@ -1236,7 +1236,7 @@ public void testNodeIndicesStatsToXContent() { commonStatsFlags.set(CommonStatsFlags.Flag.Docs, true); commonStatsFlags.set(CommonStatsFlags.Flag.Store, true); commonStatsFlags.set(CommonStatsFlags.Flag.Indexing, true); - commonStatsFlags.optimizeNodeIndicesStatsOnLevel(true); + commonStatsFlags.aggregateNodeResponsesOnLevel(true); levelParams.forEach(levelParam -> { ArrayList level_arg = new ArrayList<>(); @@ -1322,7 +1322,7 @@ public MockNodeIndicesStats generateMockNodeIndicesStats(CommonStats commonStats ClusterSettings clusterSettings = new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS); - if (commonStatsFlags.optimizeNodeIndicesStatsOnLevel()) { + if (commonStatsFlags.aggregateNodeResponsesOnLevel()) { return new MockNodeIndicesStats( new CommonStats(commonStatsFlags), statsByShard, From afeb38d25b66037e4110b8088989bedf763da8f8 Mon Sep 17 00:00:00 2001 From: Pranshu Shukla Date: Sat, 20 Jul 2024 16:02:34 +0530 Subject: [PATCH 15/25] Making Fields as enum and refacotring NodesStatsRequestBuilder Signed-off-by: Pranshu Shukla --- .../org/opensearch/nodestats/NodeStatsIT.java | 111 +++++++++++------- .../cluster/node/stats/NodesStatsRequest.java | 10 -- .../opensearch/indices/NodeIndicesStats.java | 72 ++++++++---- .../rest/action/cat/RestNodesAction.java | 2 +- .../cluster/node/stats/NodeStatsTests.java | 39 +++--- 5 files changed, 141 insertions(+), 93 deletions(-) diff --git a/server/src/internalClusterTest/java/org/opensearch/nodestats/NodeStatsIT.java b/server/src/internalClusterTest/java/org/opensearch/nodestats/NodeStatsIT.java index e7ce7006b10ab..f69ef11d2b1b2 100644 --- a/server/src/internalClusterTest/java/org/opensearch/nodestats/NodeStatsIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/nodestats/NodeStatsIT.java @@ -266,9 +266,9 @@ public void testNodeIndicesStatsDocStatusStatsCreateDeleteUpdate() { public void testNodeIndicesStatsWithoutAggregationOnNodes() { List testLevels = new ArrayList<>(); testLevels.add("null"); - testLevels.add(NodeIndicesStats.Fields.NODE); - testLevels.add(NodeIndicesStats.Fields.INDICES); - testLevels.add(NodeIndicesStats.Fields.SHARDS); + testLevels.add(NodeIndicesStats.Fields.NODE.getRestName()); + testLevels.add(NodeIndicesStats.Fields.INDICES.getRestName()); + testLevels.add(NodeIndicesStats.Fields.SHARDS.getRestName()); testLevels.add("unknown"); internalCluster().startNode(); @@ -307,7 +307,7 @@ public void testNodeIndicesStatsWithoutAggregationOnNodes() { builder.endObject(); Map xContentMap = xContentBuilderToMap(builder); - LinkedHashMap indicesStatsMap = (LinkedHashMap) xContentMap.get(NodeIndicesStats.Fields.INDICES); + LinkedHashMap indicesStatsMap = (LinkedHashMap) xContentMap.get(NodeIndicesStats.Fields.INDICES.getRestName()); assertFalse(indicesStatsMap.containsKey(NodeIndicesStats.Fields.INDICES)); assertFalse(indicesStatsMap.containsKey(NodeIndicesStats.Fields.SHARDS)); @@ -315,30 +315,36 @@ public void testNodeIndicesStatsWithoutAggregationOnNodes() { builder = XContentFactory.jsonBuilder(); builder.startObject(); builder = nodeStats.getIndices() - .toXContent(builder, new ToXContent.MapParams(Collections.singletonMap("level", NodeIndicesStats.Fields.INDICES))); + .toXContent( + builder, + new ToXContent.MapParams(Collections.singletonMap("level", NodeIndicesStats.Fields.INDICES.getRestName())) + ); builder.endObject(); xContentMap = xContentBuilderToMap(builder); - indicesStatsMap = (LinkedHashMap) xContentMap.get(NodeIndicesStats.Fields.INDICES); - assertTrue(indicesStatsMap.containsKey(NodeIndicesStats.Fields.INDICES)); - assertFalse(indicesStatsMap.containsKey(NodeIndicesStats.Fields.SHARDS)); + indicesStatsMap = (LinkedHashMap) xContentMap.get(NodeIndicesStats.Fields.INDICES.getRestName()); + assertTrue(indicesStatsMap.containsKey(NodeIndicesStats.Fields.INDICES.getRestName())); + assertFalse(indicesStatsMap.containsKey(NodeIndicesStats.Fields.SHARDS.getRestName())); - LinkedHashMap indexLevelStats = (LinkedHashMap) indicesStatsMap.get(NodeIndicesStats.Fields.INDICES); + LinkedHashMap indexLevelStats = (LinkedHashMap) indicesStatsMap.get(NodeIndicesStats.Fields.INDICES.getRestName()); assertTrue(indexLevelStats.containsKey(indexName)); // With param containing level as 'shards', the shard stats are returned builder = XContentFactory.jsonBuilder(); builder.startObject(); builder = nodeStats.getIndices() - .toXContent(builder, new ToXContent.MapParams(Collections.singletonMap("level", NodeIndicesStats.Fields.SHARDS))); + .toXContent( + builder, + new ToXContent.MapParams(Collections.singletonMap("level", NodeIndicesStats.Fields.SHARDS.getRestName())) + ); builder.endObject(); xContentMap = xContentBuilderToMap(builder); - indicesStatsMap = (LinkedHashMap) xContentMap.get(NodeIndicesStats.Fields.INDICES); - assertFalse(indicesStatsMap.containsKey(NodeIndicesStats.Fields.INDICES)); - assertTrue(indicesStatsMap.containsKey(NodeIndicesStats.Fields.SHARDS)); + indicesStatsMap = (LinkedHashMap) xContentMap.get(NodeIndicesStats.Fields.INDICES.getRestName()); + assertFalse(indicesStatsMap.containsKey(NodeIndicesStats.Fields.INDICES.getRestName())); + assertTrue(indicesStatsMap.containsKey(NodeIndicesStats.Fields.SHARDS.getRestName())); - LinkedHashMap shardLevelStats = (LinkedHashMap) indicesStatsMap.get(NodeIndicesStats.Fields.SHARDS); + LinkedHashMap shardLevelStats = (LinkedHashMap) indicesStatsMap.get(NodeIndicesStats.Fields.SHARDS.getRestName()); assertTrue(shardLevelStats.containsKey(indexName)); } catch (IOException e) { throw new RuntimeException(e); @@ -352,12 +358,13 @@ public void testNodeIndicesStatsWithoutAggregationOnNodes() { * individual data nodes instead and pre-compute information as required. */ public void testNodeIndicesStatsWithAggregationOnNodes() { - List testLevels = new ArrayList<>(); - testLevels.add("null"); - testLevels.add(NodeIndicesStats.Fields.NODE); - testLevels.add(NodeIndicesStats.Fields.INDICES); - testLevels.add(NodeIndicesStats.Fields.SHARDS); - testLevels.add("unknown"); + List testLevels = new ArrayList<>(); + + testLevels.add(MockFields.NULL); + testLevels.add(MockFields.NODE); + testLevels.add(MockFields.INDICES); + testLevels.add(MockFields.SHARDS); + testLevels.add(MockFields.UNKNOWN); internalCluster().startNode(); ensureGreen(); @@ -375,9 +382,9 @@ public void testNodeIndicesStatsWithAggregationOnNodes() { NodesStatsResponse response; CommonStatsFlags commonStatsFlags = new CommonStatsFlags(); commonStatsFlags.aggregateNodeResponsesOnLevel(true); - if (!testLevel.equals("null")) { + if (!testLevel.equals(MockFields.NULL)) { ArrayList level_arg = new ArrayList<>(); - level_arg.add(testLevel); + level_arg.add(testLevel.getRestName()); commonStatsFlags.setLevels(level_arg.toArray(new String[0])); } @@ -389,22 +396,25 @@ public void testNodeIndicesStatsWithAggregationOnNodes() { builder.startObject(); builder = nodeStats.getIndices() - .toXContent(builder, new ToXContent.MapParams(Collections.singletonMap("level", NodeIndicesStats.Fields.SHARDS))); + .toXContent( + builder, + new ToXContent.MapParams(Collections.singletonMap("level", NodeIndicesStats.Fields.SHARDS.getRestName())) + ); builder.endObject(); Map xContentMap = xContentBuilderToMap(builder); - LinkedHashMap indicesStatsMap = (LinkedHashMap) xContentMap.get(NodeIndicesStats.Fields.INDICES); - LinkedHashMap indicesStats = (LinkedHashMap) indicesStatsMap.get(NodeIndicesStats.Fields.INDICES); - LinkedHashMap shardStats = (LinkedHashMap) indicesStatsMap.get(NodeIndicesStats.Fields.SHARDS); + LinkedHashMap indicesStatsMap = (LinkedHashMap) xContentMap.get(NodeIndicesStats.Fields.INDICES.getRestName()); + LinkedHashMap indicesStats = (LinkedHashMap) indicesStatsMap.get(NodeIndicesStats.Fields.INDICES.getRestName()); + LinkedHashMap shardStats = (LinkedHashMap) indicesStatsMap.get(NodeIndicesStats.Fields.SHARDS.getRestName()); switch (testLevel) { - case NodeIndicesStats.Fields.SHARDS: + case SHARDS: assertFalse(shardStats.isEmpty()); assertNull(indicesStats); break; - case NodeIndicesStats.Fields.INDICES: - case NodeIndicesStats.Fields.NODE: - case "null": - case "unknown": + case INDICES: + case NODE: + case NULL: + case UNKNOWN: assertTrue(shardStats.isEmpty()); assertNull(indicesStats); break; @@ -414,22 +424,25 @@ public void testNodeIndicesStatsWithAggregationOnNodes() { builder.startObject(); builder = nodeStats.getIndices() - .toXContent(builder, new ToXContent.MapParams(Collections.singletonMap("level", NodeIndicesStats.Fields.INDICES))); + .toXContent( + builder, + new ToXContent.MapParams(Collections.singletonMap("level", NodeIndicesStats.Fields.INDICES.getRestName())) + ); builder.endObject(); xContentMap = xContentBuilderToMap(builder); - indicesStatsMap = (LinkedHashMap) xContentMap.get(NodeIndicesStats.Fields.INDICES); - indicesStats = (LinkedHashMap) indicesStatsMap.get(NodeIndicesStats.Fields.INDICES); - shardStats = (LinkedHashMap) indicesStatsMap.get(NodeIndicesStats.Fields.SHARDS); + indicesStatsMap = (LinkedHashMap) xContentMap.get(NodeIndicesStats.Fields.INDICES.getRestName()); + indicesStats = (LinkedHashMap) indicesStatsMap.get(NodeIndicesStats.Fields.INDICES.getRestName()); + shardStats = (LinkedHashMap) indicesStatsMap.get(NodeIndicesStats.Fields.SHARDS.getRestName()); switch (testLevel) { - case NodeIndicesStats.Fields.SHARDS: - case NodeIndicesStats.Fields.INDICES: + case SHARDS: + case INDICES: assertNull(shardStats); assertFalse(indicesStats.isEmpty()); break; - case NodeIndicesStats.Fields.NODE: - case "null": - case "unknown": + case NODE: + case NULL: + case UNKNOWN: assertNull(shardStats); assertTrue(indicesStats.isEmpty()); break; @@ -475,4 +488,22 @@ private void updateExpectedDocStatusCounter(Exception e) { expectedDocStatusStats.inc(ExceptionsHelper.status(e)); } + private enum MockFields { + INDICES(NodeIndicesStats.Fields.INDICES.getRestName()), + SHARDS(NodeIndicesStats.Fields.SHARDS.getRestName()), + NODE(NodeIndicesStats.Fields.NODE.getRestName()), + NULL("null"), + UNKNOWN("unknown"); + + private final String restName; + + MockFields(String restName) { + this.restName = restName; + } + + public String getRestName() { + return restName; + } + } + } diff --git a/server/src/main/java/org/opensearch/action/admin/cluster/node/stats/NodesStatsRequest.java b/server/src/main/java/org/opensearch/action/admin/cluster/node/stats/NodesStatsRequest.java index 27548673bca50..379836cf442e3 100644 --- a/server/src/main/java/org/opensearch/action/admin/cluster/node/stats/NodesStatsRequest.java +++ b/server/src/main/java/org/opensearch/action/admin/cluster/node/stats/NodesStatsRequest.java @@ -127,16 +127,6 @@ public NodesStatsRequest indices(boolean indices) { return this; } - /** - * Use Aggregated Response based on level passed - */ - public NodesStatsRequest aggregateResponsesOnLevel(boolean aggregateNodeResponsesOnLevel) { - if (this.indices != null) { - this.indices.aggregateNodeResponsesOnLevel(aggregateNodeResponsesOnLevel); - } - return this; - } - /** * Get the names of requested metrics, excluding indices, which are * handled separately. diff --git a/server/src/main/java/org/opensearch/indices/NodeIndicesStats.java b/server/src/main/java/org/opensearch/indices/NodeIndicesStats.java index cc285f882e606..0e8e323407311 100644 --- a/server/src/main/java/org/opensearch/indices/NodeIndicesStats.java +++ b/server/src/main/java/org/opensearch/indices/NodeIndicesStats.java @@ -68,6 +68,8 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Optional; +import java.util.stream.Collectors; /** * Global information on indices stats running on a specific node. @@ -133,19 +135,27 @@ public NodeIndicesStats( this.stats.search.setSearchRequestStats(searchRequestStats); } + Fields level = getFirstAcceptedLevel(levels); if (levels != null) { - Arrays.stream(levels).anyMatch(level -> { - switch (level) { - case Fields.INDICES: - this.statsByIndex = createStatsByIndex(statsByShard); - return true; - case Fields.SHARDS: - this.statsByShard = statsByShard; - return true; - } - return false; - }); + switch (level) { + case INDICES: + this.statsByIndex = createStatsByIndex(statsByShard); + break; + case SHARDS: + this.statsByShard = statsByShard; + break; + } + } + } + + public static Fields getFirstAcceptedLevel(String[] levels) { + if (levels != null) { + Optional level = Arrays.stream(levels) + .flatMap(passedLevel -> Arrays.stream(Fields.values()).filter(field -> field.getRestName().equals(passedLevel))) + .findFirst(); + return level.orElse(null); } + return null; } private void readStatsByIndex(StreamInput in) throws IOException { @@ -292,10 +302,10 @@ private void writeStatsByShards(StreamOutput out) throws IOException { @Override public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { - final String level = params.param("level", Fields.NODE); - final boolean isLevelValid = Fields.NODE.equalsIgnoreCase(level) - || Fields.INDICES.equalsIgnoreCase(level) - || Fields.SHARDS.equalsIgnoreCase(level); + final String level = params.param("level", Fields.NODE.getRestName()); + final boolean isLevelValid = Fields.NODE.getRestName().equalsIgnoreCase(level) + || Fields.INDICES.getRestName().equalsIgnoreCase(level) + || Fields.SHARDS.getRestName().equalsIgnoreCase(level); if (!isLevelValid) { throw new IllegalArgumentException( "level parameter must be one of [" @@ -312,14 +322,14 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws } // "node" level - builder.startObject(Fields.INDICES); + builder.startObject(Fields.INDICES.getRestName()); stats.toXContent(builder, params); - if (Fields.INDICES.equals(level)) { + if (Fields.INDICES.getRestName().equals(level)) { if (statsByIndex == null && statsByShard != null) { statsByIndex = createStatsByIndex(statsByShard); } - builder.startObject(Fields.INDICES); + builder.startObject(Fields.INDICES.getRestName()); if (statsByIndex != null) { for (Map.Entry entry : statsByIndex.entrySet()) { builder.startObject(entry.getKey().getName()); @@ -328,8 +338,8 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws } } builder.endObject(); - } else if (Fields.SHARDS.equals(level)) { - builder.startObject(Fields.SHARDS); + } else if (Fields.SHARDS.getRestName().equals(level)) { + builder.startObject(Fields.SHARDS.getRestName()); if (statsByShard != null) { for (Map.Entry> entry : statsByShard.entrySet()) { builder.startArray(entry.getKey().getName()); @@ -380,10 +390,24 @@ public List getShardStats(Index index) { * * @opensearch.internal */ - public static final class Fields { - public static final String INDICES = "indices"; - public static final String SHARDS = "shards"; - public static final String NODE = "node"; + @PublicApi(since = "2.0.0") + public enum Fields { + INDICES("indices"), + SHARDS("shards"), + NODE("node"); + private final String restName; + + Fields(String restName) { + this.restName = restName; + } + + public String getRestName() { + return restName; + } + + public static List acceptedValues() { + return Arrays.stream(Fields.values()).map(Fields::getRestName).collect(Collectors.toList()); + } } } diff --git a/server/src/main/java/org/opensearch/rest/action/cat/RestNodesAction.java b/server/src/main/java/org/opensearch/rest/action/cat/RestNodesAction.java index a45352326ce04..0452e3cb78292 100644 --- a/server/src/main/java/org/opensearch/rest/action/cat/RestNodesAction.java +++ b/server/src/main/java/org/opensearch/rest/action/cat/RestNodesAction.java @@ -139,7 +139,6 @@ public void processResponse(final NodesInfoResponse nodesInfoResponse) { nodesStatsRequest.timeout(request.param("timeout")); nodesStatsRequest.clear() .indices(true) - .aggregateResponsesOnLevel(true) .addMetrics( NodesStatsRequest.Metric.JVM.metricName(), NodesStatsRequest.Metric.OS.metricName(), @@ -147,6 +146,7 @@ public void processResponse(final NodesInfoResponse nodesInfoResponse) { NodesStatsRequest.Metric.PROCESS.metricName(), NodesStatsRequest.Metric.SCRIPT.metricName() ); + nodesStatsRequest.indices().aggregateNodeResponsesOnLevel(true); client.admin().cluster().nodesStats(nodesStatsRequest, new RestResponseListener(channel) { @Override public RestResponse buildResponse(NodesStatsResponse nodesStatsResponse) throws Exception { diff --git a/server/src/test/java/org/opensearch/action/admin/cluster/node/stats/NodeStatsTests.java b/server/src/test/java/org/opensearch/action/admin/cluster/node/stats/NodeStatsTests.java index 300d5dc990135..a78ad0d85cdbe 100644 --- a/server/src/test/java/org/opensearch/action/admin/cluster/node/stats/NodeStatsTests.java +++ b/server/src/test/java/org/opensearch/action/admin/cluster/node/stats/NodeStatsTests.java @@ -1165,7 +1165,7 @@ public void testNodeIndicesStatsSerializationWithOldESVersionNodesWithoutAggrega public void testNodeIndicesStatsSerialization() throws IOException { long numDocs = randomLongBetween(0, 10000); long numDeletedDocs = randomLongBetween(0, 100); - List levelParams = new ArrayList<>(); + List levelParams = new ArrayList<>(); levelParams.add(NodeIndicesStats.Fields.INDICES); levelParams.add(NodeIndicesStats.Fields.SHARDS); levelParams.add(NodeIndicesStats.Fields.NODE); @@ -1184,7 +1184,7 @@ public void testNodeIndicesStatsSerialization() throws IOException { levelParams.forEach(levelParam -> { ArrayList level_arg = new ArrayList<>(); - level_arg.add(levelParam); + level_arg.add(levelParam.getRestName()); commonStatsFlags.setLevels(level_arg.toArray(new String[0])); @@ -1198,15 +1198,15 @@ public void testNodeIndicesStatsSerialization() throws IOException { try (StreamInput in = out.bytes().streamInput()) { MockNodeIndicesStats newNodeIndicesStats = new MockNodeIndicesStats(in); switch (levelParam) { - case "node": + case NODE: assertNull(newNodeIndicesStats.getStatsByIndex()); assertNull(newNodeIndicesStats.getStatsByShard()); break; - case "indices": + case INDICES: assertNull(newNodeIndicesStats.getStatsByShard()); assertNotNull(newNodeIndicesStats.getStatsByIndex()); break; - case "shards": + case SHARDS: assertNull(newNodeIndicesStats.getStatsByIndex()); assertNotNull(newNodeIndicesStats.getStatsByShard()); break; @@ -1221,7 +1221,7 @@ public void testNodeIndicesStatsSerialization() throws IOException { public void testNodeIndicesStatsToXContent() { long numDocs = randomLongBetween(0, 10000); long numDeletedDocs = randomLongBetween(0, 100); - List levelParams = new ArrayList<>(); + List levelParams = new ArrayList<>(); levelParams.add(NodeIndicesStats.Fields.INDICES); levelParams.add(NodeIndicesStats.Fields.SHARDS); levelParams.add(NodeIndicesStats.Fields.NODE); @@ -1240,7 +1240,7 @@ public void testNodeIndicesStatsToXContent() { levelParams.forEach(levelParam -> { ArrayList level_arg = new ArrayList<>(); - level_arg.add(levelParam); + level_arg.add(levelParam.getRestName()); commonStatsFlags.setLevels(level_arg.toArray(new String[0])); @@ -1252,24 +1252,27 @@ public void testNodeIndicesStatsToXContent() { try { builder = XContentFactory.jsonBuilder(); builder.startObject(); - builder = mockNodeIndicesStats.toXContent(builder, new ToXContent.MapParams(Collections.singletonMap("level", levelParam))); + builder = mockNodeIndicesStats.toXContent( + builder, + new ToXContent.MapParams(Collections.singletonMap("level", levelParam.getRestName())) + ); builder.endObject(); Map xContentMap = xContentBuilderToMap(builder); - LinkedHashMap indicesStatsMap = (LinkedHashMap) xContentMap.get(NodeIndicesStats.Fields.INDICES); + LinkedHashMap indicesStatsMap = (LinkedHashMap) xContentMap.get(NodeIndicesStats.Fields.INDICES.getRestName()); switch (levelParam) { - case NodeIndicesStats.Fields.NODE: - assertFalse(indicesStatsMap.containsKey(NodeIndicesStats.Fields.INDICES)); - assertFalse(indicesStatsMap.containsKey(NodeIndicesStats.Fields.SHARDS)); + case NODE: + assertFalse(indicesStatsMap.containsKey(NodeIndicesStats.Fields.INDICES.getRestName())); + assertFalse(indicesStatsMap.containsKey(NodeIndicesStats.Fields.SHARDS.getRestName())); break; - case NodeIndicesStats.Fields.INDICES: - assertTrue(indicesStatsMap.containsKey(NodeIndicesStats.Fields.INDICES)); - assertFalse(indicesStatsMap.containsKey(NodeIndicesStats.Fields.SHARDS)); + case INDICES: + assertTrue(indicesStatsMap.containsKey(NodeIndicesStats.Fields.INDICES.getRestName())); + assertFalse(indicesStatsMap.containsKey(NodeIndicesStats.Fields.SHARDS.getRestName())); break; - case NodeIndicesStats.Fields.SHARDS: - assertFalse(indicesStatsMap.containsKey(NodeIndicesStats.Fields.INDICES)); - assertTrue(indicesStatsMap.containsKey(NodeIndicesStats.Fields.SHARDS)); + case SHARDS: + assertFalse(indicesStatsMap.containsKey(NodeIndicesStats.Fields.INDICES.getRestName())); + assertTrue(indicesStatsMap.containsKey(NodeIndicesStats.Fields.SHARDS.getRestName())); break; } } catch (IOException e) { From b65707255cd077120b330e4570bced3d5b60c21e Mon Sep 17 00:00:00 2001 From: Pranshu Shukla Date: Sat, 20 Jul 2024 16:47:42 +0530 Subject: [PATCH 16/25] Fix level selection Signed-off-by: Pranshu Shukla --- .../java/org/opensearch/indices/NodeIndicesStats.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/server/src/main/java/org/opensearch/indices/NodeIndicesStats.java b/server/src/main/java/org/opensearch/indices/NodeIndicesStats.java index 0e8e323407311..612dacbc20b70 100644 --- a/server/src/main/java/org/opensearch/indices/NodeIndicesStats.java +++ b/server/src/main/java/org/opensearch/indices/NodeIndicesStats.java @@ -136,7 +136,7 @@ public NodeIndicesStats( } Fields level = getFirstAcceptedLevel(levels); - if (levels != null) { + if (level != null) { switch (level) { case INDICES: this.statsByIndex = createStatsByIndex(statsByShard); @@ -309,12 +309,12 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws if (!isLevelValid) { throw new IllegalArgumentException( "level parameter must be one of [" - + Fields.INDICES + + Fields.INDICES.getRestName() + "] or " + "[" - + Fields.NODE + + Fields.NODE.getRestName() + "] or [" - + Fields.SHARDS + + Fields.SHARDS.getRestName() + "] but was [" + level + "]" From 67749bd1f3c498ec1340c19481e05aa8cc5f837b Mon Sep 17 00:00:00 2001 From: Pranshu Shukla Date: Mon, 22 Jul 2024 16:12:30 +0530 Subject: [PATCH 17/25] Add comments and variable refactoring Signed-off-by: Pranshu Shukla --- .../org/opensearch/nodestats/NodeStatsIT.java | 2 +- .../admin/indices/stats/CommonStatsFlags.java | 14 +++++++------- .../org/opensearch/indices/IndicesService.java | 2 +- .../opensearch/indices/NodeIndicesStats.java | 18 +++++++++++++++++- .../admin/cluster/RestNodesStatsAction.java | 2 +- .../rest/action/cat/RestNodesAction.java | 2 +- .../cluster/node/stats/NodeStatsTests.java | 6 +++--- 7 files changed, 31 insertions(+), 15 deletions(-) diff --git a/server/src/internalClusterTest/java/org/opensearch/nodestats/NodeStatsIT.java b/server/src/internalClusterTest/java/org/opensearch/nodestats/NodeStatsIT.java index f69ef11d2b1b2..28abe4a10c724 100644 --- a/server/src/internalClusterTest/java/org/opensearch/nodestats/NodeStatsIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/nodestats/NodeStatsIT.java @@ -381,7 +381,7 @@ public void testNodeIndicesStatsWithAggregationOnNodes() { testLevels.forEach(testLevel -> { NodesStatsResponse response; CommonStatsFlags commonStatsFlags = new CommonStatsFlags(); - commonStatsFlags.aggregateNodeResponsesOnLevel(true); + commonStatsFlags.setAggregateNodeIndicesStatsResponsesOnLevel(true); if (!testLevel.equals(MockFields.NULL)) { ArrayList level_arg = new ArrayList<>(); level_arg.add(testLevel.getRestName()); 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 0a28be5e17dfe..39880b8abaa81 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 @@ -67,7 +67,7 @@ public class CommonStatsFlags implements Writeable, Cloneable { // Used for metric CACHE_STATS, to determine which caches to report stats for private EnumSet includeCaches = EnumSet.noneOf(CacheType.class); private String[] levels = new String[0]; - private boolean aggregateNodeResponsesOnLevel = false; + private boolean aggregateNodeIndicesStatsResponsesOnLevel = false; /** * @param flags flags to set. If no flags are supplied, default flags will be set. @@ -102,7 +102,7 @@ public CommonStatsFlags(StreamInput in) throws IOException { levels = in.readStringArray(); } if (in.getVersion().onOrAfter(Version.V_3_0_0)) { - aggregateNodeResponsesOnLevel = in.readBoolean(); + aggregateNodeIndicesStatsResponsesOnLevel = in.readBoolean(); } } @@ -129,7 +129,7 @@ public void writeTo(StreamOutput out) throws IOException { out.writeStringArrayNullable(levels); } if (out.getVersion().onOrAfter(Version.V_3_0_0)) { - out.writeBoolean(aggregateNodeResponsesOnLevel); + out.writeBoolean(aggregateNodeIndicesStatsResponsesOnLevel); } } @@ -269,12 +269,12 @@ public boolean includeSegmentFileSizes() { return this.includeSegmentFileSizes; } - public void aggregateNodeResponsesOnLevel(boolean aggregateNodeResponsesOnLevel) { - this.aggregateNodeResponsesOnLevel = aggregateNodeResponsesOnLevel; + public void setAggregateNodeIndicesStatsResponsesOnLevel(boolean aggregateNodeIndicesStatsResponsesOnLevel) { + this.aggregateNodeIndicesStatsResponsesOnLevel = aggregateNodeIndicesStatsResponsesOnLevel; } - public boolean aggregateNodeResponsesOnLevel() { - return this.aggregateNodeResponsesOnLevel; + public boolean getAggregateNodeIndicesStatsResponsesOnLevel() { + return this.aggregateNodeIndicesStatsResponsesOnLevel; } public boolean isSet(Flag flag) { diff --git a/server/src/main/java/org/opensearch/indices/IndicesService.java b/server/src/main/java/org/opensearch/indices/IndicesService.java index 3ca39b0c5cb6a..3f4da71bec9c7 100644 --- a/server/src/main/java/org/opensearch/indices/IndicesService.java +++ b/server/src/main/java/org/opensearch/indices/IndicesService.java @@ -692,7 +692,7 @@ public NodeIndicesStats stats(CommonStatsFlags flags) { break; } } - if (flags.aggregateNodeResponsesOnLevel()) { + if (flags.getAggregateNodeIndicesStatsResponsesOnLevel()) { return new NodeIndicesStats(commonStats, statsByShard(this, flags), searchRequestStats, flags.getLevels()); } return new NodeIndicesStats(commonStats, statsByShard(this, flags), searchRequestStats); diff --git a/server/src/main/java/org/opensearch/indices/NodeIndicesStats.java b/server/src/main/java/org/opensearch/indices/NodeIndicesStats.java index 612dacbc20b70..41045e3702b80 100644 --- a/server/src/main/java/org/opensearch/indices/NodeIndicesStats.java +++ b/server/src/main/java/org/opensearch/indices/NodeIndicesStats.java @@ -97,6 +97,10 @@ public NodeIndicesStats(StreamInput in) throws IOException { } } + /** + * Without passing the information of the levels to the constructor, we return the Node-level aggregated stats as + * {@link CommonStats} along with a hash-map containing Index to List of Shard Stats. + */ public NodeIndicesStats(CommonStats oldStats, Map> statsByShard, SearchRequestStats searchRequestStats) { // this.stats = stats; this.statsByShard = statsByShard; @@ -115,6 +119,12 @@ public NodeIndicesStats(CommonStats oldStats, Map> } } + /** + * Passing the level information to the nodes allows us to aggregate the stats based on the level passed. This + * allows us to aggregate based on NodeLevel (default - if no level is passed) or Index level if `indices` level is + * passed and finally return the statsByShards map if `shards` level is passed. This allows us to reduce ser/de of + * stats and return only the information that is required while returning to the client. + */ public NodeIndicesStats( CommonStats oldStats, Map> statsByShard, @@ -148,7 +158,13 @@ public NodeIndicesStats( } } - public static Fields getFirstAcceptedLevel(String[] levels) { + /** + * By default, the levels passed from the transport action will be a list of strings, since NodeIndicesStats can + * only aggregate on one level, we pick the first accepted level else we ignore if no known level is passed. + * @param levels - levels sent in the request. + * @return Corresponding identified enum {@link Fields} + */ + private static Fields getFirstAcceptedLevel(String[] levels) { if (levels != null) { Optional level = Arrays.stream(levels) .flatMap(passedLevel -> Arrays.stream(Fields.values()).filter(field -> field.getRestName().equals(passedLevel))) 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 2caba1a1376e1..c26f0376a8bed 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 @@ -232,7 +232,7 @@ public RestChannelConsumer prepareRequest(final RestRequest request, final NodeC // If no levels are passed in this results in an empty array. String[] levels = Strings.splitStringByCommaToArray(request.param("level")); nodesStatsRequest.indices().setLevels(levels); - nodesStatsRequest.indices().aggregateNodeResponsesOnLevel(true); + nodesStatsRequest.indices().setAggregateNodeIndicesStatsResponsesOnLevel(true); return channel -> client.admin().cluster().nodesStats(nodesStatsRequest, new NodesResponseRestListener<>(channel)); } diff --git a/server/src/main/java/org/opensearch/rest/action/cat/RestNodesAction.java b/server/src/main/java/org/opensearch/rest/action/cat/RestNodesAction.java index 0452e3cb78292..c73a8c6798514 100644 --- a/server/src/main/java/org/opensearch/rest/action/cat/RestNodesAction.java +++ b/server/src/main/java/org/opensearch/rest/action/cat/RestNodesAction.java @@ -146,7 +146,7 @@ public void processResponse(final NodesInfoResponse nodesInfoResponse) { NodesStatsRequest.Metric.PROCESS.metricName(), NodesStatsRequest.Metric.SCRIPT.metricName() ); - nodesStatsRequest.indices().aggregateNodeResponsesOnLevel(true); + nodesStatsRequest.indices().setAggregateNodeIndicesStatsResponsesOnLevel(true); client.admin().cluster().nodesStats(nodesStatsRequest, new RestResponseListener(channel) { @Override public RestResponse buildResponse(NodesStatsResponse nodesStatsResponse) throws Exception { diff --git a/server/src/test/java/org/opensearch/action/admin/cluster/node/stats/NodeStatsTests.java b/server/src/test/java/org/opensearch/action/admin/cluster/node/stats/NodeStatsTests.java index a78ad0d85cdbe..d3eda069c16b0 100644 --- a/server/src/test/java/org/opensearch/action/admin/cluster/node/stats/NodeStatsTests.java +++ b/server/src/test/java/org/opensearch/action/admin/cluster/node/stats/NodeStatsTests.java @@ -1180,7 +1180,7 @@ public void testNodeIndicesStatsSerialization() throws IOException { commonStatsFlags.set(CommonStatsFlags.Flag.Docs, true); commonStatsFlags.set(CommonStatsFlags.Flag.Store, true); commonStatsFlags.set(CommonStatsFlags.Flag.Indexing, true); - commonStatsFlags.aggregateNodeResponsesOnLevel(true); + commonStatsFlags.setAggregateNodeIndicesStatsResponsesOnLevel(true); levelParams.forEach(levelParam -> { ArrayList level_arg = new ArrayList<>(); @@ -1236,7 +1236,7 @@ public void testNodeIndicesStatsToXContent() { commonStatsFlags.set(CommonStatsFlags.Flag.Docs, true); commonStatsFlags.set(CommonStatsFlags.Flag.Store, true); commonStatsFlags.set(CommonStatsFlags.Flag.Indexing, true); - commonStatsFlags.aggregateNodeResponsesOnLevel(true); + commonStatsFlags.setAggregateNodeIndicesStatsResponsesOnLevel(true); levelParams.forEach(levelParam -> { ArrayList level_arg = new ArrayList<>(); @@ -1325,7 +1325,7 @@ public MockNodeIndicesStats generateMockNodeIndicesStats(CommonStats commonStats ClusterSettings clusterSettings = new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS); - if (commonStatsFlags.aggregateNodeResponsesOnLevel()) { + if (commonStatsFlags.getAggregateNodeIndicesStatsResponsesOnLevel()) { return new MockNodeIndicesStats( new CommonStats(commonStatsFlags), statsByShard, From aef96d9e7e0f495f7df346091f26d667414557a1 Mon Sep 17 00:00:00 2001 From: Pranshu Shukla Date: Mon, 22 Jul 2024 16:14:19 +0530 Subject: [PATCH 18/25] Fix ChangeLogs Signed-off-by: Pranshu Shukla --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e15ac2d7f1913..da1ba16830111 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,7 +5,6 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), ## [Unreleased 2.x] ### Added -- Optimize NodeIndicesStats output behind flag ([#14454](https://github.com/opensearch-project/OpenSearch/pull/14454)) - Add fingerprint ingest processor ([#13724](https://github.com/opensearch-project/OpenSearch/pull/13724)) - [Remote Store] Rate limiter for remote store low priority uploads ([#14374](https://github.com/opensearch-project/OpenSearch/pull/14374/)) - Apply the date histogram rewrite optimization to range aggregation ([#13865](https://github.com/opensearch-project/OpenSearch/pull/13865)) @@ -22,6 +21,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - Add Plugin interface for loading application based configuration templates (([#14659](https://github.com/opensearch-project/OpenSearch/issues/14659))) - Refactor remote-routing-table service inline with remote state interfaces([#14668](https://github.com/opensearch-project/OpenSearch/pull/14668)) - Add prefix mode verification setting for repository verification (([#14790](https://github.com/opensearch-project/OpenSearch/pull/14790))) +- Optimize NodeIndicesStats output behind flag ([#14454](https://github.com/opensearch-project/OpenSearch/pull/14454)) ### Dependencies - Bump `org.gradle.test-retry` from 1.5.8 to 1.5.9 ([#13442](https://github.com/opensearch-project/OpenSearch/pull/13442)) From 11280062476e1f672fd03ad9c37c26871993040e Mon Sep 17 00:00:00 2001 From: Pranshu Shukla Date: Mon, 22 Jul 2024 18:17:45 +0530 Subject: [PATCH 19/25] Retry Build Signed-off-by: Pranshu Shukla From 5203097bead4ebff0925b0eef61231d03b75bff2 Mon Sep 17 00:00:00 2001 From: Pranshu Shukla Date: Tue, 23 Jul 2024 08:47:48 +0530 Subject: [PATCH 20/25] Adding tests and addressing nit comments Signed-off-by: Pranshu Shukla --- .../org/opensearch/nodestats/NodeStatsIT.java | 74 ++++---- .../admin/indices/stats/CommonStatsFlags.java | 14 +- .../opensearch/indices/IndicesService.java | 5 +- .../opensearch/indices/NodeIndicesStats.java | 42 ++--- .../admin/cluster/RestNodesStatsAction.java | 2 +- .../rest/action/cat/RestNodesAction.java | 2 +- .../cluster/node/stats/NodeStatsTests.java | 174 ++++++++++++++++-- 7 files changed, 225 insertions(+), 88 deletions(-) diff --git a/server/src/internalClusterTest/java/org/opensearch/nodestats/NodeStatsIT.java b/server/src/internalClusterTest/java/org/opensearch/nodestats/NodeStatsIT.java index 28abe4a10c724..dd20d12946201 100644 --- a/server/src/internalClusterTest/java/org/opensearch/nodestats/NodeStatsIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/nodestats/NodeStatsIT.java @@ -266,9 +266,9 @@ public void testNodeIndicesStatsDocStatusStatsCreateDeleteUpdate() { public void testNodeIndicesStatsWithoutAggregationOnNodes() { List testLevels = new ArrayList<>(); testLevels.add("null"); - testLevels.add(NodeIndicesStats.Fields.NODE.getRestName()); - testLevels.add(NodeIndicesStats.Fields.INDICES.getRestName()); - testLevels.add(NodeIndicesStats.Fields.SHARDS.getRestName()); + testLevels.add(NodeIndicesStats.StatsLevel.NODE.getRestName()); + testLevels.add(NodeIndicesStats.StatsLevel.INDICES.getRestName()); + testLevels.add(NodeIndicesStats.StatsLevel.SHARDS.getRestName()); testLevels.add("unknown"); internalCluster().startNode(); @@ -307,9 +307,9 @@ public void testNodeIndicesStatsWithoutAggregationOnNodes() { builder.endObject(); Map xContentMap = xContentBuilderToMap(builder); - LinkedHashMap indicesStatsMap = (LinkedHashMap) xContentMap.get(NodeIndicesStats.Fields.INDICES.getRestName()); - assertFalse(indicesStatsMap.containsKey(NodeIndicesStats.Fields.INDICES)); - assertFalse(indicesStatsMap.containsKey(NodeIndicesStats.Fields.SHARDS)); + LinkedHashMap indicesStatsMap = (LinkedHashMap) xContentMap.get(NodeIndicesStats.StatsLevel.INDICES.getRestName()); + assertFalse(indicesStatsMap.containsKey(NodeIndicesStats.StatsLevel.INDICES)); + assertFalse(indicesStatsMap.containsKey(NodeIndicesStats.StatsLevel.SHARDS)); // With param containing level as 'indices', the indices stats are returned builder = XContentFactory.jsonBuilder(); @@ -317,16 +317,16 @@ public void testNodeIndicesStatsWithoutAggregationOnNodes() { builder = nodeStats.getIndices() .toXContent( builder, - new ToXContent.MapParams(Collections.singletonMap("level", NodeIndicesStats.Fields.INDICES.getRestName())) + new ToXContent.MapParams(Collections.singletonMap("level", NodeIndicesStats.StatsLevel.INDICES.getRestName())) ); builder.endObject(); xContentMap = xContentBuilderToMap(builder); - indicesStatsMap = (LinkedHashMap) xContentMap.get(NodeIndicesStats.Fields.INDICES.getRestName()); - assertTrue(indicesStatsMap.containsKey(NodeIndicesStats.Fields.INDICES.getRestName())); - assertFalse(indicesStatsMap.containsKey(NodeIndicesStats.Fields.SHARDS.getRestName())); + indicesStatsMap = (LinkedHashMap) xContentMap.get(NodeIndicesStats.StatsLevel.INDICES.getRestName()); + assertTrue(indicesStatsMap.containsKey(NodeIndicesStats.StatsLevel.INDICES.getRestName())); + assertFalse(indicesStatsMap.containsKey(NodeIndicesStats.StatsLevel.SHARDS.getRestName())); - LinkedHashMap indexLevelStats = (LinkedHashMap) indicesStatsMap.get(NodeIndicesStats.Fields.INDICES.getRestName()); + LinkedHashMap indexLevelStats = (LinkedHashMap) indicesStatsMap.get(NodeIndicesStats.StatsLevel.INDICES.getRestName()); assertTrue(indexLevelStats.containsKey(indexName)); // With param containing level as 'shards', the shard stats are returned @@ -335,16 +335,16 @@ public void testNodeIndicesStatsWithoutAggregationOnNodes() { builder = nodeStats.getIndices() .toXContent( builder, - new ToXContent.MapParams(Collections.singletonMap("level", NodeIndicesStats.Fields.SHARDS.getRestName())) + new ToXContent.MapParams(Collections.singletonMap("level", NodeIndicesStats.StatsLevel.SHARDS.getRestName())) ); builder.endObject(); xContentMap = xContentBuilderToMap(builder); - indicesStatsMap = (LinkedHashMap) xContentMap.get(NodeIndicesStats.Fields.INDICES.getRestName()); - assertFalse(indicesStatsMap.containsKey(NodeIndicesStats.Fields.INDICES.getRestName())); - assertTrue(indicesStatsMap.containsKey(NodeIndicesStats.Fields.SHARDS.getRestName())); + indicesStatsMap = (LinkedHashMap) xContentMap.get(NodeIndicesStats.StatsLevel.INDICES.getRestName()); + assertFalse(indicesStatsMap.containsKey(NodeIndicesStats.StatsLevel.INDICES.getRestName())); + assertTrue(indicesStatsMap.containsKey(NodeIndicesStats.StatsLevel.SHARDS.getRestName())); - LinkedHashMap shardLevelStats = (LinkedHashMap) indicesStatsMap.get(NodeIndicesStats.Fields.SHARDS.getRestName()); + LinkedHashMap shardLevelStats = (LinkedHashMap) indicesStatsMap.get(NodeIndicesStats.StatsLevel.SHARDS.getRestName()); assertTrue(shardLevelStats.containsKey(indexName)); } catch (IOException e) { throw new RuntimeException(e); @@ -358,13 +358,13 @@ public void testNodeIndicesStatsWithoutAggregationOnNodes() { * individual data nodes instead and pre-compute information as required. */ public void testNodeIndicesStatsWithAggregationOnNodes() { - List testLevels = new ArrayList<>(); + List testLevels = new ArrayList<>(); - testLevels.add(MockFields.NULL); - testLevels.add(MockFields.NODE); - testLevels.add(MockFields.INDICES); - testLevels.add(MockFields.SHARDS); - testLevels.add(MockFields.UNKNOWN); + testLevels.add(MockStatsLevel.NULL); + testLevels.add(MockStatsLevel.NODE); + testLevels.add(MockStatsLevel.INDICES); + testLevels.add(MockStatsLevel.SHARDS); + testLevels.add(MockStatsLevel.UNKNOWN); internalCluster().startNode(); ensureGreen(); @@ -381,8 +381,8 @@ public void testNodeIndicesStatsWithAggregationOnNodes() { testLevels.forEach(testLevel -> { NodesStatsResponse response; CommonStatsFlags commonStatsFlags = new CommonStatsFlags(); - commonStatsFlags.setAggregateNodeIndicesStatsResponsesOnLevel(true); - if (!testLevel.equals(MockFields.NULL)) { + commonStatsFlags.setIncludeIndicesStatsByLevel(true); + if (!testLevel.equals(MockStatsLevel.NULL)) { ArrayList level_arg = new ArrayList<>(); level_arg.add(testLevel.getRestName()); @@ -398,14 +398,14 @@ public void testNodeIndicesStatsWithAggregationOnNodes() { builder = nodeStats.getIndices() .toXContent( builder, - new ToXContent.MapParams(Collections.singletonMap("level", NodeIndicesStats.Fields.SHARDS.getRestName())) + new ToXContent.MapParams(Collections.singletonMap("level", NodeIndicesStats.StatsLevel.SHARDS.getRestName())) ); builder.endObject(); Map xContentMap = xContentBuilderToMap(builder); - LinkedHashMap indicesStatsMap = (LinkedHashMap) xContentMap.get(NodeIndicesStats.Fields.INDICES.getRestName()); - LinkedHashMap indicesStats = (LinkedHashMap) indicesStatsMap.get(NodeIndicesStats.Fields.INDICES.getRestName()); - LinkedHashMap shardStats = (LinkedHashMap) indicesStatsMap.get(NodeIndicesStats.Fields.SHARDS.getRestName()); + LinkedHashMap indicesStatsMap = (LinkedHashMap) xContentMap.get(NodeIndicesStats.StatsLevel.INDICES.getRestName()); + LinkedHashMap indicesStats = (LinkedHashMap) indicesStatsMap.get(NodeIndicesStats.StatsLevel.INDICES.getRestName()); + LinkedHashMap shardStats = (LinkedHashMap) indicesStatsMap.get(NodeIndicesStats.StatsLevel.SHARDS.getRestName()); switch (testLevel) { case SHARDS: assertFalse(shardStats.isEmpty()); @@ -426,14 +426,14 @@ public void testNodeIndicesStatsWithAggregationOnNodes() { builder = nodeStats.getIndices() .toXContent( builder, - new ToXContent.MapParams(Collections.singletonMap("level", NodeIndicesStats.Fields.INDICES.getRestName())) + new ToXContent.MapParams(Collections.singletonMap("level", NodeIndicesStats.StatsLevel.INDICES.getRestName())) ); builder.endObject(); xContentMap = xContentBuilderToMap(builder); - indicesStatsMap = (LinkedHashMap) xContentMap.get(NodeIndicesStats.Fields.INDICES.getRestName()); - indicesStats = (LinkedHashMap) indicesStatsMap.get(NodeIndicesStats.Fields.INDICES.getRestName()); - shardStats = (LinkedHashMap) indicesStatsMap.get(NodeIndicesStats.Fields.SHARDS.getRestName()); + indicesStatsMap = (LinkedHashMap) xContentMap.get(NodeIndicesStats.StatsLevel.INDICES.getRestName()); + indicesStats = (LinkedHashMap) indicesStatsMap.get(NodeIndicesStats.StatsLevel.INDICES.getRestName()); + shardStats = (LinkedHashMap) indicesStatsMap.get(NodeIndicesStats.StatsLevel.SHARDS.getRestName()); switch (testLevel) { case SHARDS: case INDICES: @@ -488,16 +488,16 @@ private void updateExpectedDocStatusCounter(Exception e) { expectedDocStatusStats.inc(ExceptionsHelper.status(e)); } - private enum MockFields { - INDICES(NodeIndicesStats.Fields.INDICES.getRestName()), - SHARDS(NodeIndicesStats.Fields.SHARDS.getRestName()), - NODE(NodeIndicesStats.Fields.NODE.getRestName()), + private enum MockStatsLevel { + INDICES(NodeIndicesStats.StatsLevel.INDICES.getRestName()), + SHARDS(NodeIndicesStats.StatsLevel.SHARDS.getRestName()), + NODE(NodeIndicesStats.StatsLevel.NODE.getRestName()), NULL("null"), UNKNOWN("unknown"); private final String restName; - MockFields(String restName) { + MockStatsLevel(String restName) { this.restName = restName; } 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 39880b8abaa81..04f39d77ce6c8 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 @@ -67,7 +67,7 @@ public class CommonStatsFlags implements Writeable, Cloneable { // Used for metric CACHE_STATS, to determine which caches to report stats for private EnumSet includeCaches = EnumSet.noneOf(CacheType.class); private String[] levels = new String[0]; - private boolean aggregateNodeIndicesStatsResponsesOnLevel = false; + private boolean includeIndicesStatsByLevel = false; /** * @param flags flags to set. If no flags are supplied, default flags will be set. @@ -102,7 +102,7 @@ public CommonStatsFlags(StreamInput in) throws IOException { levels = in.readStringArray(); } if (in.getVersion().onOrAfter(Version.V_3_0_0)) { - aggregateNodeIndicesStatsResponsesOnLevel = in.readBoolean(); + includeIndicesStatsByLevel = in.readBoolean(); } } @@ -129,7 +129,7 @@ public void writeTo(StreamOutput out) throws IOException { out.writeStringArrayNullable(levels); } if (out.getVersion().onOrAfter(Version.V_3_0_0)) { - out.writeBoolean(aggregateNodeIndicesStatsResponsesOnLevel); + out.writeBoolean(includeIndicesStatsByLevel); } } @@ -269,12 +269,12 @@ public boolean includeSegmentFileSizes() { return this.includeSegmentFileSizes; } - public void setAggregateNodeIndicesStatsResponsesOnLevel(boolean aggregateNodeIndicesStatsResponsesOnLevel) { - this.aggregateNodeIndicesStatsResponsesOnLevel = aggregateNodeIndicesStatsResponsesOnLevel; + public void setIncludeIndicesStatsByLevel(boolean includeIndicesStatsByLevel) { + this.includeIndicesStatsByLevel = includeIndicesStatsByLevel; } - public boolean getAggregateNodeIndicesStatsResponsesOnLevel() { - return this.aggregateNodeIndicesStatsResponsesOnLevel; + public boolean getIncludeIndicesStatsByLevel() { + return this.includeIndicesStatsByLevel; } public boolean isSet(Flag flag) { diff --git a/server/src/main/java/org/opensearch/indices/IndicesService.java b/server/src/main/java/org/opensearch/indices/IndicesService.java index 3f4da71bec9c7..845fed095065c 100644 --- a/server/src/main/java/org/opensearch/indices/IndicesService.java +++ b/server/src/main/java/org/opensearch/indices/IndicesService.java @@ -692,10 +692,11 @@ public NodeIndicesStats stats(CommonStatsFlags flags) { break; } } - if (flags.getAggregateNodeIndicesStatsResponsesOnLevel()) { + if (flags.getIncludeIndicesStatsByLevel()) { return new NodeIndicesStats(commonStats, statsByShard(this, flags), searchRequestStats, flags.getLevels()); + } else { + return new NodeIndicesStats(commonStats, statsByShard(this, flags), searchRequestStats); } - return new NodeIndicesStats(commonStats, statsByShard(this, flags), searchRequestStats); } Map> statsByShard(final IndicesService indicesService, final CommonStatsFlags flags) { diff --git a/server/src/main/java/org/opensearch/indices/NodeIndicesStats.java b/server/src/main/java/org/opensearch/indices/NodeIndicesStats.java index 41045e3702b80..5ffec76eaf1c7 100644 --- a/server/src/main/java/org/opensearch/indices/NodeIndicesStats.java +++ b/server/src/main/java/org/opensearch/indices/NodeIndicesStats.java @@ -145,7 +145,7 @@ public NodeIndicesStats( this.stats.search.setSearchRequestStats(searchRequestStats); } - Fields level = getFirstAcceptedLevel(levels); + StatsLevel level = getAcceptedLevel(levels); if (level != null) { switch (level) { case INDICES: @@ -162,12 +162,12 @@ public NodeIndicesStats( * By default, the levels passed from the transport action will be a list of strings, since NodeIndicesStats can * only aggregate on one level, we pick the first accepted level else we ignore if no known level is passed. * @param levels - levels sent in the request. - * @return Corresponding identified enum {@link Fields} + * @return Corresponding identified enum {@link StatsLevel} */ - private static Fields getFirstAcceptedLevel(String[] levels) { - if (levels != null) { - Optional level = Arrays.stream(levels) - .flatMap(passedLevel -> Arrays.stream(Fields.values()).filter(field -> field.getRestName().equals(passedLevel))) + private static StatsLevel getAcceptedLevel(String[] levels) { + if (levels != null && levels.length > 0) { + Optional level = Arrays.stream(StatsLevel.values()) + .filter(field -> field.getRestName().equals(levels[0])) .findFirst(); return level.orElse(null); } @@ -318,19 +318,19 @@ private void writeStatsByShards(StreamOutput out) throws IOException { @Override public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { - final String level = params.param("level", Fields.NODE.getRestName()); - final boolean isLevelValid = Fields.NODE.getRestName().equalsIgnoreCase(level) - || Fields.INDICES.getRestName().equalsIgnoreCase(level) - || Fields.SHARDS.getRestName().equalsIgnoreCase(level); + final String level = params.param("level", StatsLevel.NODE.getRestName()); + final boolean isLevelValid = StatsLevel.NODE.getRestName().equalsIgnoreCase(level) + || StatsLevel.INDICES.getRestName().equalsIgnoreCase(level) + || StatsLevel.SHARDS.getRestName().equalsIgnoreCase(level); if (!isLevelValid) { throw new IllegalArgumentException( "level parameter must be one of [" - + Fields.INDICES.getRestName() + + StatsLevel.INDICES.getRestName() + "] or " + "[" - + Fields.NODE.getRestName() + + StatsLevel.NODE.getRestName() + "] or [" - + Fields.SHARDS.getRestName() + + StatsLevel.SHARDS.getRestName() + "] but was [" + level + "]" @@ -338,14 +338,14 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws } // "node" level - builder.startObject(Fields.INDICES.getRestName()); + builder.startObject(StatsLevel.INDICES.getRestName()); stats.toXContent(builder, params); - if (Fields.INDICES.getRestName().equals(level)) { + if (StatsLevel.INDICES.getRestName().equals(level)) { if (statsByIndex == null && statsByShard != null) { statsByIndex = createStatsByIndex(statsByShard); } - builder.startObject(Fields.INDICES.getRestName()); + builder.startObject(StatsLevel.INDICES.getRestName()); if (statsByIndex != null) { for (Map.Entry entry : statsByIndex.entrySet()) { builder.startObject(entry.getKey().getName()); @@ -354,8 +354,8 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws } } builder.endObject(); - } else if (Fields.SHARDS.getRestName().equals(level)) { - builder.startObject(Fields.SHARDS.getRestName()); + } else if (StatsLevel.SHARDS.getRestName().equals(level)) { + builder.startObject(StatsLevel.SHARDS.getRestName()); if (statsByShard != null) { for (Map.Entry> entry : statsByShard.entrySet()) { builder.startArray(entry.getKey().getName()); @@ -407,14 +407,14 @@ public List getShardStats(Index index) { * @opensearch.internal */ @PublicApi(since = "2.0.0") - public enum Fields { + public enum StatsLevel { INDICES("indices"), SHARDS("shards"), NODE("node"); private final String restName; - Fields(String restName) { + StatsLevel(String restName) { this.restName = restName; } @@ -423,7 +423,7 @@ public String getRestName() { } public static List acceptedValues() { - return Arrays.stream(Fields.values()).map(Fields::getRestName).collect(Collectors.toList()); + return Arrays.stream(StatsLevel.values()).map(StatsLevel::getRestName).collect(Collectors.toList()); } } } 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 b6850f8ab824c..0119731e4a0d7 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 @@ -233,7 +233,7 @@ public RestChannelConsumer prepareRequest(final RestRequest request, final NodeC String[] levels = Strings.splitStringByCommaToArray(request.param("level")); nodesStatsRequest.indices().setLevels(levels); nodesStatsRequest.setIncludeDiscoveryNodes(false); - nodesStatsRequest.indices().setAggregateNodeIndicesStatsResponsesOnLevel(true); + nodesStatsRequest.indices().setIncludeIndicesStatsByLevel(true); return channel -> client.admin().cluster().nodesStats(nodesStatsRequest, new NodesResponseRestListener<>(channel)); } diff --git a/server/src/main/java/org/opensearch/rest/action/cat/RestNodesAction.java b/server/src/main/java/org/opensearch/rest/action/cat/RestNodesAction.java index c9b20a0b22f03..1aa40b50290cd 100644 --- a/server/src/main/java/org/opensearch/rest/action/cat/RestNodesAction.java +++ b/server/src/main/java/org/opensearch/rest/action/cat/RestNodesAction.java @@ -148,7 +148,7 @@ public void processResponse(final NodesInfoResponse nodesInfoResponse) { NodesStatsRequest.Metric.PROCESS.metricName(), NodesStatsRequest.Metric.SCRIPT.metricName() ); - nodesStatsRequest.indices().setAggregateNodeIndicesStatsResponsesOnLevel(true); + nodesStatsRequest.indices().setIncludeIndicesStatsByLevel(true); client.admin().cluster().nodesStats(nodesStatsRequest, new RestResponseListener(channel) { @Override public RestResponse buildResponse(NodesStatsResponse nodesStatsResponse) throws Exception { diff --git a/server/src/test/java/org/opensearch/action/admin/cluster/node/stats/NodeStatsTests.java b/server/src/test/java/org/opensearch/action/admin/cluster/node/stats/NodeStatsTests.java index d3eda069c16b0..d60945524c16d 100644 --- a/server/src/test/java/org/opensearch/action/admin/cluster/node/stats/NodeStatsTests.java +++ b/server/src/test/java/org/opensearch/action/admin/cluster/node/stats/NodeStatsTests.java @@ -73,6 +73,10 @@ import org.opensearch.http.HttpStats; import org.opensearch.index.ReplicationStats; import org.opensearch.index.SegmentReplicationRejectionStats; +import org.opensearch.index.cache.query.QueryCacheStats; +import org.opensearch.index.engine.SegmentsStats; +import org.opensearch.index.fielddata.FieldDataStats; +import org.opensearch.index.flush.FlushStats; import org.opensearch.index.remote.RemoteSegmentStats; import org.opensearch.index.remote.RemoteTranslogTransferTracker; import org.opensearch.index.shard.DocsStats; @@ -98,6 +102,7 @@ import org.opensearch.ratelimitting.admissioncontrol.stats.AdmissionControllerStats; import org.opensearch.script.ScriptCacheStats; import org.opensearch.script.ScriptStats; +import org.opensearch.search.suggest.completion.CompletionStats; import org.opensearch.test.OpenSearchTestCase; import org.opensearch.test.VersionUtils; import org.opensearch.threadpool.ThreadPoolStats; @@ -1128,6 +1133,7 @@ public void testNodeIndicesStatsSerializationWithOldESVersionNodesWithoutAggrega commonStats.docs = new DocsStats(numDocs, numDeletedDocs, 0); commonStats.store = new StoreStats(100, 0L); commonStats.indexing = new IndexingStats(); + DocsStats hostDocStats = new DocsStats(numDocs, numDeletedDocs, 0); CommonStatsFlags commonStatsFlags = new CommonStatsFlags(); commonStatsFlags.clear(); @@ -1152,7 +1158,6 @@ public void testNodeIndicesStatsSerializationWithOldESVersionNodesWithoutAggrega ShardStats shardStats = Arrays.stream(indexShardStats.getShards()).findFirst().get(); DocsStats incomingDocStats = shardStats.getStats().docs; - DocsStats hostDocStats = new DocsStats(numDocs, numDeletedDocs, 0); assertEquals(incomingDocStats.getCount(), hostDocStats.getCount()); assertEquals(incomingDocStats.getTotalSizeInBytes(), hostDocStats.getTotalSizeInBytes()); assertEquals(incomingDocStats.getAverageSizeInBytes(), hostDocStats.getAverageSizeInBytes()); @@ -1165,10 +1170,10 @@ public void testNodeIndicesStatsSerializationWithOldESVersionNodesWithoutAggrega public void testNodeIndicesStatsSerialization() throws IOException { long numDocs = randomLongBetween(0, 10000); long numDeletedDocs = randomLongBetween(0, 100); - List levelParams = new ArrayList<>(); - levelParams.add(NodeIndicesStats.Fields.INDICES); - levelParams.add(NodeIndicesStats.Fields.SHARDS); - levelParams.add(NodeIndicesStats.Fields.NODE); + List levelParams = new ArrayList<>(); + levelParams.add(NodeIndicesStats.StatsLevel.INDICES); + levelParams.add(NodeIndicesStats.StatsLevel.SHARDS); + levelParams.add(NodeIndicesStats.StatsLevel.NODE); CommonStats commonStats = new CommonStats(CommonStatsFlags.NONE); commonStats.docs = new DocsStats(numDocs, numDeletedDocs, 0); @@ -1180,7 +1185,7 @@ public void testNodeIndicesStatsSerialization() throws IOException { commonStatsFlags.set(CommonStatsFlags.Flag.Docs, true); commonStatsFlags.set(CommonStatsFlags.Flag.Store, true); commonStatsFlags.set(CommonStatsFlags.Flag.Indexing, true); - commonStatsFlags.setAggregateNodeIndicesStatsResponsesOnLevel(true); + commonStatsFlags.setIncludeIndicesStatsByLevel(true); levelParams.forEach(levelParam -> { ArrayList level_arg = new ArrayList<>(); @@ -1221,10 +1226,10 @@ public void testNodeIndicesStatsSerialization() throws IOException { public void testNodeIndicesStatsToXContent() { long numDocs = randomLongBetween(0, 10000); long numDeletedDocs = randomLongBetween(0, 100); - List levelParams = new ArrayList<>(); - levelParams.add(NodeIndicesStats.Fields.INDICES); - levelParams.add(NodeIndicesStats.Fields.SHARDS); - levelParams.add(NodeIndicesStats.Fields.NODE); + List levelParams = new ArrayList<>(); + levelParams.add(NodeIndicesStats.StatsLevel.INDICES); + levelParams.add(NodeIndicesStats.StatsLevel.SHARDS); + levelParams.add(NodeIndicesStats.StatsLevel.NODE); CommonStats commonStats = new CommonStats(CommonStatsFlags.NONE); commonStats.docs = new DocsStats(numDocs, numDeletedDocs, 0); @@ -1236,7 +1241,7 @@ public void testNodeIndicesStatsToXContent() { commonStatsFlags.set(CommonStatsFlags.Flag.Docs, true); commonStatsFlags.set(CommonStatsFlags.Flag.Store, true); commonStatsFlags.set(CommonStatsFlags.Flag.Indexing, true); - commonStatsFlags.setAggregateNodeIndicesStatsResponsesOnLevel(true); + commonStatsFlags.setIncludeIndicesStatsByLevel(true); levelParams.forEach(levelParam -> { ArrayList level_arg = new ArrayList<>(); @@ -1259,20 +1264,20 @@ public void testNodeIndicesStatsToXContent() { builder.endObject(); Map xContentMap = xContentBuilderToMap(builder); - LinkedHashMap indicesStatsMap = (LinkedHashMap) xContentMap.get(NodeIndicesStats.Fields.INDICES.getRestName()); + LinkedHashMap indicesStatsMap = (LinkedHashMap) xContentMap.get(NodeIndicesStats.StatsLevel.INDICES.getRestName()); switch (levelParam) { case NODE: - assertFalse(indicesStatsMap.containsKey(NodeIndicesStats.Fields.INDICES.getRestName())); - assertFalse(indicesStatsMap.containsKey(NodeIndicesStats.Fields.SHARDS.getRestName())); + assertFalse(indicesStatsMap.containsKey(NodeIndicesStats.StatsLevel.INDICES.getRestName())); + assertFalse(indicesStatsMap.containsKey(NodeIndicesStats.StatsLevel.SHARDS.getRestName())); break; case INDICES: - assertTrue(indicesStatsMap.containsKey(NodeIndicesStats.Fields.INDICES.getRestName())); - assertFalse(indicesStatsMap.containsKey(NodeIndicesStats.Fields.SHARDS.getRestName())); + assertTrue(indicesStatsMap.containsKey(NodeIndicesStats.StatsLevel.INDICES.getRestName())); + assertFalse(indicesStatsMap.containsKey(NodeIndicesStats.StatsLevel.SHARDS.getRestName())); break; case SHARDS: - assertFalse(indicesStatsMap.containsKey(NodeIndicesStats.Fields.INDICES.getRestName())); - assertTrue(indicesStatsMap.containsKey(NodeIndicesStats.Fields.SHARDS.getRestName())); + assertFalse(indicesStatsMap.containsKey(NodeIndicesStats.StatsLevel.INDICES.getRestName())); + assertTrue(indicesStatsMap.containsKey(NodeIndicesStats.StatsLevel.SHARDS.getRestName())); break; } } catch (IOException e) { @@ -1282,6 +1287,137 @@ public void testNodeIndicesStatsToXContent() { }); } + public void testNodeIndicesStatsWithAndWithoutAggregations() throws IOException { + + CommonStatsFlags commonStatsFlags = new CommonStatsFlags( + CommonStatsFlags.Flag.Docs, + CommonStatsFlags.Flag.Store, + CommonStatsFlags.Flag.Indexing, + CommonStatsFlags.Flag.Completion, + CommonStatsFlags.Flag.Flush, + CommonStatsFlags.Flag.FieldData, + CommonStatsFlags.Flag.QueryCache, + CommonStatsFlags.Flag.Segments + ); + + int numberOfIndexes = randomIntBetween(1, 3); + List indexList = new ArrayList<>(); + for (int i = 0; i < numberOfIndexes; i++) { + Index index = new Index("test-index-" + i, "_na_"); + indexList.add(index); + } + + ClusterSettings clusterSettings = new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS); + HashMap> statsByShards = createRandomShardByStats(indexList); + + final MockNodeIndicesStats nonAggregatedNodeIndicesStats = new MockNodeIndicesStats( + new CommonStats(commonStatsFlags), + statsByShards, + new SearchRequestStats(clusterSettings) + ); + + commonStatsFlags.setIncludeIndicesStatsByLevel(true); + + NodeIndicesStats.StatsLevel.acceptedValues().forEach(level -> { + List levelList = new ArrayList<>(); + levelList.add(level); + + MockNodeIndicesStats aggregatedNodeIndicesStats = new MockNodeIndicesStats( + new CommonStats(commonStatsFlags), + statsByShards, + new SearchRequestStats(clusterSettings), + levelList.toArray(new String[0]) + ); + + XContentBuilder nonAggregatedBuilder = null; + XContentBuilder aggregatedBuilder = null; + try { + nonAggregatedBuilder = XContentFactory.jsonBuilder(); + nonAggregatedBuilder.startObject(); + nonAggregatedBuilder = nonAggregatedNodeIndicesStats.toXContent( + nonAggregatedBuilder, + new ToXContent.MapParams(Collections.singletonMap("level", level)) + ); + nonAggregatedBuilder.endObject(); + Map nonAggregatedContentMap = xContentBuilderToMap(nonAggregatedBuilder); + + aggregatedBuilder = XContentFactory.jsonBuilder(); + aggregatedBuilder.startObject(); + aggregatedBuilder = aggregatedNodeIndicesStats.toXContent( + aggregatedBuilder, + new ToXContent.MapParams(Collections.singletonMap("level", level)) + ); + aggregatedBuilder.endObject(); + Map aggregatedContentMap = xContentBuilderToMap(aggregatedBuilder); + + assertEquals(aggregatedContentMap, nonAggregatedContentMap); + } catch (IOException e) { + throw new RuntimeException(e); + } + }); + } + + private CommonStats createRandomCommonStats() { + CommonStats commonStats = new CommonStats(CommonStatsFlags.NONE); + commonStats.docs = new DocsStats(randomLongBetween(0, 10000), randomLongBetween(0, 100), randomLongBetween(0, 1000)); + commonStats.store = new StoreStats(randomLongBetween(0, 100), randomLongBetween(0, 1000)); + commonStats.indexing = new IndexingStats(); + commonStats.completion = new CompletionStats(); + commonStats.flush = new FlushStats(randomLongBetween(0, 100), randomLongBetween(0, 100), randomLongBetween(0, 100)); + commonStats.fieldData = new FieldDataStats(randomLongBetween(0, 100), randomLongBetween(0, 100), null); + commonStats.queryCache = new QueryCacheStats( + randomLongBetween(0, 100), + randomLongBetween(0, 100), + randomLongBetween(0, 100), + randomLongBetween(0, 100), + randomLongBetween(0, 100) + ); + commonStats.segments = new SegmentsStats(); + + return commonStats; + } + + private HashMap> createRandomShardByStats(List indexes) { + DiscoveryNode localNode = new DiscoveryNode("node", buildNewFakeTransportAddress(), Version.CURRENT); + HashMap> statsByShards = new HashMap<>(); + indexes.forEach(index -> { + List indexShardStatsList = new ArrayList<>(); + + int numberOfShards = randomIntBetween(1, 4); + for (int i = 0; i < numberOfShards; i++) { + ShardRoutingState shardRoutingState = ShardRoutingState.fromValue((byte) randomIntBetween(2, 3)); + + ShardRouting shardRouting = TestShardRouting.newShardRouting( + index.getName(), + i, + localNode.getId(), + randomBoolean(), + shardRoutingState + ); + + Path path = createTempDir().resolve("indices") + .resolve(shardRouting.shardId().getIndex().getUUID()) + .resolve(String.valueOf(shardRouting.shardId().id())); + + ShardStats shardStats = new ShardStats( + shardRouting, + new ShardPath(false, path, path, shardRouting.shardId()), + createRandomCommonStats(), + null, + null, + null + ); + List shardStatsList = new ArrayList<>(); + shardStatsList.add(shardStats); + IndexShardStats indexShardStats = new IndexShardStats(shardRouting.shardId(), shardStatsList.toArray(new ShardStats[0])); + indexShardStatsList.add(indexShardStats); + } + statsByShards.put(index, indexShardStatsList); + }); + + return statsByShards; + } + private Map xContentBuilderToMap(XContentBuilder xContentBuilder) { return XContentHelper.convertToMap(BytesReference.bytes(xContentBuilder), true, xContentBuilder.contentType()).v2(); } @@ -1325,7 +1461,7 @@ public MockNodeIndicesStats generateMockNodeIndicesStats(CommonStats commonStats ClusterSettings clusterSettings = new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS); - if (commonStatsFlags.getAggregateNodeIndicesStatsResponsesOnLevel()) { + if (commonStatsFlags.getIncludeIndicesStatsByLevel()) { return new MockNodeIndicesStats( new CommonStats(commonStatsFlags), statsByShard, From a0dff3fecd8525ad7af8f6f9328e280aa9256792 Mon Sep 17 00:00:00 2001 From: Pranshu Shukla Date: Tue, 23 Jul 2024 15:15:25 +0530 Subject: [PATCH 21/25] Addressing comments Signed-off-by: Pranshu Shukla --- .../org/opensearch/nodestats/NodeStatsIT.java | 222 +++++++++++------- .../opensearch/indices/NodeIndicesStats.java | 28 +-- .../cluster/node/stats/NodeStatsTests.java | 5 +- 3 files changed, 151 insertions(+), 104 deletions(-) diff --git a/server/src/internalClusterTest/java/org/opensearch/nodestats/NodeStatsIT.java b/server/src/internalClusterTest/java/org/opensearch/nodestats/NodeStatsIT.java index dd20d12946201..cf40b73d5178d 100644 --- a/server/src/internalClusterTest/java/org/opensearch/nodestats/NodeStatsIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/nodestats/NodeStatsIT.java @@ -10,6 +10,7 @@ import org.opensearch.ExceptionsHelper; import org.opensearch.action.DocWriteResponse; +import org.opensearch.action.admin.cluster.node.stats.NodeStats; import org.opensearch.action.admin.cluster.node.stats.NodesStatsResponse; import org.opensearch.action.admin.indices.stats.CommonStatsFlags; import org.opensearch.action.bulk.BulkItemResponse; @@ -259,11 +260,46 @@ public void testNodeIndicesStatsDocStatusStatsCreateDeleteUpdate() { } } + public void testNodeIndicesStatsDocStatsWithAggregations() { + { // Testing Create + final String INDEX = "create_index"; + final String ID = "id"; + DocStatusStats expectedDocStatusStats = new DocStatusStats(); + + IndexResponse response = client().index(new IndexRequest(INDEX).id(ID).source(SOURCE).create(true)).actionGet(); + expectedDocStatusStats.inc(response.status()); + + CommonStatsFlags commonStatsFlags = new CommonStatsFlags(); + commonStatsFlags.setIncludeIndicesStatsByLevel(true); + + DocStatusStats docStatusStats = client().admin() + .cluster() + .prepareNodesStats() + .setIndices(commonStatsFlags) + .execute() + .actionGet() + .getNodes() + .get(0) + .getIndices() + .getIndexing() + .getTotal() + .getDocStatusStats(); + + assertTrue( + Arrays.equals( + docStatusStats.getDocStatusCounter(), + expectedDocStatusStats.getDocStatusCounter(), + Comparator.comparingLong(AtomicLong::longValue) + ) + ); + } + } + /** * Default behavior - without consideration of request level param on level, the NodeStatsRequest always * returns ShardStats which is aggregated on the coordinator node when creating the XContent. */ - public void testNodeIndicesStatsWithoutAggregationOnNodes() { + public void testNodeIndicesStatsXContentWithoutAggregationOnNodes() { List testLevels = new ArrayList<>(); testLevels.add("null"); testLevels.add(NodeIndicesStats.StatsLevel.NODE.getRestName()); @@ -297,59 +333,58 @@ public void testNodeIndicesStatsWithoutAggregationOnNodes() { response = client().admin().cluster().prepareNodesStats().get(); } - response.getNodes().forEach(nodeStats -> { - assertNotNull(nodeStats.getIndices().getShardStats(clusterState.metadata().index(indexName).getIndex())); - try { - // Without any param - default is level = nodes - XContentBuilder builder = XContentFactory.jsonBuilder(); - builder.startObject(); - builder = nodeStats.getIndices().toXContent(builder, ToXContent.EMPTY_PARAMS); - builder.endObject(); - - Map xContentMap = xContentBuilderToMap(builder); - LinkedHashMap indicesStatsMap = (LinkedHashMap) xContentMap.get(NodeIndicesStats.StatsLevel.INDICES.getRestName()); - assertFalse(indicesStatsMap.containsKey(NodeIndicesStats.StatsLevel.INDICES)); - assertFalse(indicesStatsMap.containsKey(NodeIndicesStats.StatsLevel.SHARDS)); - - // With param containing level as 'indices', the indices stats are returned - builder = XContentFactory.jsonBuilder(); - builder.startObject(); - builder = nodeStats.getIndices() - .toXContent( - builder, - new ToXContent.MapParams(Collections.singletonMap("level", NodeIndicesStats.StatsLevel.INDICES.getRestName())) - ); - builder.endObject(); - - xContentMap = xContentBuilderToMap(builder); - indicesStatsMap = (LinkedHashMap) xContentMap.get(NodeIndicesStats.StatsLevel.INDICES.getRestName()); - assertTrue(indicesStatsMap.containsKey(NodeIndicesStats.StatsLevel.INDICES.getRestName())); - assertFalse(indicesStatsMap.containsKey(NodeIndicesStats.StatsLevel.SHARDS.getRestName())); - - LinkedHashMap indexLevelStats = (LinkedHashMap) indicesStatsMap.get(NodeIndicesStats.StatsLevel.INDICES.getRestName()); - assertTrue(indexLevelStats.containsKey(indexName)); - - // With param containing level as 'shards', the shard stats are returned - builder = XContentFactory.jsonBuilder(); - builder.startObject(); - builder = nodeStats.getIndices() - .toXContent( - builder, - new ToXContent.MapParams(Collections.singletonMap("level", NodeIndicesStats.StatsLevel.SHARDS.getRestName())) - ); - builder.endObject(); - - xContentMap = xContentBuilderToMap(builder); - indicesStatsMap = (LinkedHashMap) xContentMap.get(NodeIndicesStats.StatsLevel.INDICES.getRestName()); - assertFalse(indicesStatsMap.containsKey(NodeIndicesStats.StatsLevel.INDICES.getRestName())); - assertTrue(indicesStatsMap.containsKey(NodeIndicesStats.StatsLevel.SHARDS.getRestName())); - - LinkedHashMap shardLevelStats = (LinkedHashMap) indicesStatsMap.get(NodeIndicesStats.StatsLevel.SHARDS.getRestName()); - assertTrue(shardLevelStats.containsKey(indexName)); - } catch (IOException e) { - throw new RuntimeException(e); - } - }); + NodeStats nodeStats = response.getNodes().get(0); + assertNotNull(nodeStats.getIndices().getShardStats(clusterState.metadata().index(indexName).getIndex())); + try { + // Without any param - default is level = nodes + XContentBuilder builder = XContentFactory.jsonBuilder(); + builder.startObject(); + builder = nodeStats.getIndices().toXContent(builder, ToXContent.EMPTY_PARAMS); + builder.endObject(); + + Map xContentMap = xContentBuilderToMap(builder); + LinkedHashMap indicesStatsMap = (LinkedHashMap) xContentMap.get(NodeIndicesStats.StatsLevel.INDICES.getRestName()); + assertFalse(indicesStatsMap.containsKey(NodeIndicesStats.StatsLevel.INDICES)); + assertFalse(indicesStatsMap.containsKey(NodeIndicesStats.StatsLevel.SHARDS)); + + // With param containing level as 'indices', the indices stats are returned + builder = XContentFactory.jsonBuilder(); + builder.startObject(); + builder = nodeStats.getIndices() + .toXContent( + builder, + new ToXContent.MapParams(Collections.singletonMap("level", NodeIndicesStats.StatsLevel.INDICES.getRestName())) + ); + builder.endObject(); + + xContentMap = xContentBuilderToMap(builder); + indicesStatsMap = (LinkedHashMap) xContentMap.get(NodeIndicesStats.StatsLevel.INDICES.getRestName()); + assertTrue(indicesStatsMap.containsKey(NodeIndicesStats.StatsLevel.INDICES.getRestName())); + assertFalse(indicesStatsMap.containsKey(NodeIndicesStats.StatsLevel.SHARDS.getRestName())); + + LinkedHashMap indexLevelStats = (LinkedHashMap) indicesStatsMap.get(NodeIndicesStats.StatsLevel.INDICES.getRestName()); + assertTrue(indexLevelStats.containsKey(indexName)); + + // With param containing level as 'shards', the shard stats are returned + builder = XContentFactory.jsonBuilder(); + builder.startObject(); + builder = nodeStats.getIndices() + .toXContent( + builder, + new ToXContent.MapParams(Collections.singletonMap("level", NodeIndicesStats.StatsLevel.SHARDS.getRestName())) + ); + builder.endObject(); + + xContentMap = xContentBuilderToMap(builder); + indicesStatsMap = (LinkedHashMap) xContentMap.get(NodeIndicesStats.StatsLevel.INDICES.getRestName()); + assertFalse(indicesStatsMap.containsKey(NodeIndicesStats.StatsLevel.INDICES.getRestName())); + assertTrue(indicesStatsMap.containsKey(NodeIndicesStats.StatsLevel.SHARDS.getRestName())); + + LinkedHashMap shardLevelStats = (LinkedHashMap) indicesStatsMap.get(NodeIndicesStats.StatsLevel.SHARDS.getRestName()); + assertTrue(shardLevelStats.containsKey(indexName)); + } catch (IOException e) { + throw new RuntimeException(e); + } }); } @@ -357,7 +392,7 @@ public void testNodeIndicesStatsWithoutAggregationOnNodes() { * Aggregated behavior - to avoid unnecessary IO in the form of shard-stats when not required, we not honor the levels on the * individual data nodes instead and pre-compute information as required. */ - public void testNodeIndicesStatsWithAggregationOnNodes() { + public void testNodeIndicesStatsXContentWithAggregationOnNodes() { List testLevels = new ArrayList<>(); testLevels.add(MockStatsLevel.NULL); @@ -390,11 +425,24 @@ public void testNodeIndicesStatsWithAggregationOnNodes() { } response = client().admin().cluster().prepareNodesStats().setIndices(commonStatsFlags).get(); - response.getNodes().forEach(nodeStats -> { - try { - XContentBuilder builder = XContentFactory.jsonBuilder(); - - builder.startObject(); + NodeStats nodeStats = response.getNodes().get(0); + try { + XContentBuilder builder = XContentFactory.jsonBuilder(); + + builder.startObject(); + + if (!testLevel.equals(MockStatsLevel.SHARDS)){ + final XContentBuilder failedBuilder = builder; + assertThrows( + "Expected shard stats in response for generating [SHARDS] field", + AssertionError.class, + () -> nodeStats.getIndices() + .toXContent( + failedBuilder, + new ToXContent.MapParams(Collections.singletonMap("level", NodeIndicesStats.StatsLevel.SHARDS.getRestName())) + ) + ); + } else { builder = nodeStats.getIndices() .toXContent( builder, @@ -406,23 +454,26 @@ public void testNodeIndicesStatsWithAggregationOnNodes() { LinkedHashMap indicesStatsMap = (LinkedHashMap) xContentMap.get(NodeIndicesStats.StatsLevel.INDICES.getRestName()); LinkedHashMap indicesStats = (LinkedHashMap) indicesStatsMap.get(NodeIndicesStats.StatsLevel.INDICES.getRestName()); LinkedHashMap shardStats = (LinkedHashMap) indicesStatsMap.get(NodeIndicesStats.StatsLevel.SHARDS.getRestName()); - switch (testLevel) { - case SHARDS: - assertFalse(shardStats.isEmpty()); - assertNull(indicesStats); - break; - case INDICES: - case NODE: - case NULL: - case UNKNOWN: - assertTrue(shardStats.isEmpty()); - assertNull(indicesStats); - break; - } - builder = XContentFactory.jsonBuilder(); + assertFalse(shardStats.isEmpty()); + assertNull(indicesStats); + } - builder.startObject(); + builder = XContentFactory.jsonBuilder(); + builder.startObject(); + + if (!(testLevel.equals(MockStatsLevel.SHARDS) || testLevel.equals(MockStatsLevel.INDICES))) { + final XContentBuilder failedBuilder = builder; + assertThrows( + "Expected shard stats or index stats in response for generating INDICES field", + AssertionError.class, + () -> nodeStats.getIndices() + .toXContent( + failedBuilder, + new ToXContent.MapParams(Collections.singletonMap("level", NodeIndicesStats.StatsLevel.INDICES.getRestName())) + ) + ); + } else { builder = nodeStats.getIndices() .toXContent( builder, @@ -430,27 +481,22 @@ public void testNodeIndicesStatsWithAggregationOnNodes() { ); builder.endObject(); - xContentMap = xContentBuilderToMap(builder); - indicesStatsMap = (LinkedHashMap) xContentMap.get(NodeIndicesStats.StatsLevel.INDICES.getRestName()); - indicesStats = (LinkedHashMap) indicesStatsMap.get(NodeIndicesStats.StatsLevel.INDICES.getRestName()); - shardStats = (LinkedHashMap) indicesStatsMap.get(NodeIndicesStats.StatsLevel.SHARDS.getRestName()); + Map xContentMap = xContentBuilderToMap(builder); + LinkedHashMap indicesStatsMap = (LinkedHashMap) xContentMap.get(NodeIndicesStats.StatsLevel.INDICES.getRestName()); + LinkedHashMap indicesStats = (LinkedHashMap) indicesStatsMap.get(NodeIndicesStats.StatsLevel.INDICES.getRestName()); + LinkedHashMap shardStats = (LinkedHashMap) indicesStatsMap.get(NodeIndicesStats.StatsLevel.SHARDS.getRestName()); + switch (testLevel) { case SHARDS: case INDICES: assertNull(shardStats); assertFalse(indicesStats.isEmpty()); break; - case NODE: - case NULL: - case UNKNOWN: - assertNull(shardStats); - assertTrue(indicesStats.isEmpty()); - break; } - } catch (IOException e) { - throw new RuntimeException(e); } - }); + } catch (IOException e) { + throw new RuntimeException(e); + } }); } diff --git a/server/src/main/java/org/opensearch/indices/NodeIndicesStats.java b/server/src/main/java/org/opensearch/indices/NodeIndicesStats.java index 5ffec76eaf1c7..ad786d236f3ce 100644 --- a/server/src/main/java/org/opensearch/indices/NodeIndicesStats.java +++ b/server/src/main/java/org/opensearch/indices/NodeIndicesStats.java @@ -61,8 +61,10 @@ import org.opensearch.index.translog.TranslogStats; import org.opensearch.index.warmer.WarmerStats; import org.opensearch.search.suggest.completion.CompletionStats; +import org.opensearch.transport.TransportException; import java.io.IOException; +import java.rmi.UnexpectedException; import java.util.ArrayList; import java.util.Arrays; import java.util.HashMap; @@ -342,9 +344,11 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws stats.toXContent(builder, params); if (StatsLevel.INDICES.getRestName().equals(level)) { - if (statsByIndex == null && statsByShard != null) { + assert statsByIndex!=null || statsByShard!=null: "Expected shard stats or index stats in response for generating [" + StatsLevel.INDICES + "] field"; + if (statsByIndex == null) { statsByIndex = createStatsByIndex(statsByShard); } + builder.startObject(StatsLevel.INDICES.getRestName()); if (statsByIndex != null) { for (Map.Entry entry : statsByIndex.entrySet()) { @@ -356,18 +360,17 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws builder.endObject(); } else if (StatsLevel.SHARDS.getRestName().equals(level)) { builder.startObject(StatsLevel.SHARDS.getRestName()); - if (statsByShard != null) { - for (Map.Entry> entry : statsByShard.entrySet()) { - builder.startArray(entry.getKey().getName()); - for (IndexShardStats indexShardStats : entry.getValue()) { - builder.startObject().startObject(String.valueOf(indexShardStats.getShardId().getId())); - for (ShardStats shardStats : indexShardStats.getShards()) { - shardStats.toXContent(builder, params); - } - builder.endObject().endObject(); + assert statsByShard != null: "Expected shard stats in response for generating [" + StatsLevel.SHARDS + "] field"; + for (Map.Entry> entry : statsByShard.entrySet()) { + builder.startArray(entry.getKey().getName()); + for (IndexShardStats indexShardStats : entry.getValue()) { + builder.startObject().startObject(String.valueOf(indexShardStats.getShardId().getId())); + for (ShardStats shardStats : indexShardStats.getShards()) { + shardStats.toXContent(builder, params); } - builder.endArray(); + builder.endObject().endObject(); } + builder.endArray(); } builder.endObject(); } @@ -422,8 +425,5 @@ public String getRestName() { return restName; } - public static List acceptedValues() { - return Arrays.stream(StatsLevel.values()).map(StatsLevel::getRestName).collect(Collectors.toList()); - } } } diff --git a/server/src/test/java/org/opensearch/action/admin/cluster/node/stats/NodeStatsTests.java b/server/src/test/java/org/opensearch/action/admin/cluster/node/stats/NodeStatsTests.java index d60945524c16d..a22e2fbb914b8 100644 --- a/server/src/test/java/org/opensearch/action/admin/cluster/node/stats/NodeStatsTests.java +++ b/server/src/test/java/org/opensearch/action/admin/cluster/node/stats/NodeStatsTests.java @@ -1125,7 +1125,7 @@ public Map> getStatsByShard() { } } - public void testNodeIndicesStatsSerializationWithOldESVersionNodesWithoutAggregations() throws IOException { + public void testOldVersionNodes() throws IOException { long numDocs = randomLongBetween(0, 10000); long numDeletedDocs = randomLongBetween(0, 100); CommonStats commonStats = new CommonStats(CommonStatsFlags.NONE); @@ -1318,7 +1318,8 @@ public void testNodeIndicesStatsWithAndWithoutAggregations() throws IOException commonStatsFlags.setIncludeIndicesStatsByLevel(true); - NodeIndicesStats.StatsLevel.acceptedValues().forEach(level -> { + Arrays.stream(NodeIndicesStats.StatsLevel.values()).forEach(enumLevel -> { + String level = enumLevel.getRestName(); List levelList = new ArrayList<>(); levelList.add(level); From ff664317b46a2afae743e8ab0ca17bf478e81698 Mon Sep 17 00:00:00 2001 From: Pranshu Shukla Date: Tue, 23 Jul 2024 16:47:51 +0530 Subject: [PATCH 22/25] Add Node Level Validations Signed-off-by: Pranshu Shukla --- .../org/opensearch/nodestats/NodeStatsIT.java | 47 +++++++++++++++++-- .../opensearch/indices/NodeIndicesStats.java | 11 ++--- 2 files changed, 48 insertions(+), 10 deletions(-) diff --git a/server/src/internalClusterTest/java/org/opensearch/nodestats/NodeStatsIT.java b/server/src/internalClusterTest/java/org/opensearch/nodestats/NodeStatsIT.java index cf40b73d5178d..b2c834a10a13d 100644 --- a/server/src/internalClusterTest/java/org/opensearch/nodestats/NodeStatsIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/nodestats/NodeStatsIT.java @@ -399,7 +399,6 @@ public void testNodeIndicesStatsXContentWithAggregationOnNodes() { testLevels.add(MockStatsLevel.NODE); testLevels.add(MockStatsLevel.INDICES); testLevels.add(MockStatsLevel.SHARDS); - testLevels.add(MockStatsLevel.UNKNOWN); internalCluster().startNode(); ensureGreen(); @@ -431,7 +430,7 @@ public void testNodeIndicesStatsXContentWithAggregationOnNodes() { builder.startObject(); - if (!testLevel.equals(MockStatsLevel.SHARDS)){ + if (!testLevel.equals(MockStatsLevel.SHARDS)) { final XContentBuilder failedBuilder = builder; assertThrows( "Expected shard stats in response for generating [SHARDS] field", @@ -439,7 +438,9 @@ public void testNodeIndicesStatsXContentWithAggregationOnNodes() { () -> nodeStats.getIndices() .toXContent( failedBuilder, - new ToXContent.MapParams(Collections.singletonMap("level", NodeIndicesStats.StatsLevel.SHARDS.getRestName())) + new ToXContent.MapParams( + Collections.singletonMap("level", NodeIndicesStats.StatsLevel.SHARDS.getRestName()) + ) ) ); } else { @@ -470,7 +471,9 @@ public void testNodeIndicesStatsXContentWithAggregationOnNodes() { () -> nodeStats.getIndices() .toXContent( failedBuilder, - new ToXContent.MapParams(Collections.singletonMap("level", NodeIndicesStats.StatsLevel.INDICES.getRestName())) + new ToXContent.MapParams( + Collections.singletonMap("level", NodeIndicesStats.StatsLevel.INDICES.getRestName()) + ) ) ); } else { @@ -500,6 +503,42 @@ public void testNodeIndicesStatsXContentWithAggregationOnNodes() { }); } + public void testNodeIndicesStatsUnknownLevelThrowsException() { + List testLevels = new ArrayList<>(); + + testLevels.add(MockStatsLevel.UNKNOWN); + internalCluster().startNode(); + ensureGreen(); + String indexName = "test1"; + assertAcked( + prepareCreate( + indexName, + clusterService().state().getNodes().getSize(), + Settings.builder().put("number_of_shards", 2).put("number_of_replicas", clusterService().state().getNodes().getSize() - 1) + ) + ); + ensureGreen(); + + testLevels.forEach(testLevel -> { + NodesStatsResponse response; + CommonStatsFlags commonStatsFlags = new CommonStatsFlags(); + commonStatsFlags.setIncludeIndicesStatsByLevel(true); + if (!testLevel.equals(MockStatsLevel.NULL)) { + ArrayList level_arg = new ArrayList<>(); + level_arg.add(testLevel.getRestName()); + + commonStatsFlags.setLevels(level_arg.toArray(new String[0])); + } + response = client().admin().cluster().prepareNodesStats().setIndices(commonStatsFlags).get(); + + assertTrue(response.hasFailures()); + assertEquals( + "Level provided is not supported by NodeIndicesStats", + response.failures().get(0).getCause().getCause().getMessage() + ); + }); + } + private Map xContentBuilderToMap(XContentBuilder xContentBuilder) { return XContentHelper.convertToMap(BytesReference.bytes(xContentBuilder), true, xContentBuilder.contentType()).v2(); } diff --git a/server/src/main/java/org/opensearch/indices/NodeIndicesStats.java b/server/src/main/java/org/opensearch/indices/NodeIndicesStats.java index ad786d236f3ce..4a5e216b514ae 100644 --- a/server/src/main/java/org/opensearch/indices/NodeIndicesStats.java +++ b/server/src/main/java/org/opensearch/indices/NodeIndicesStats.java @@ -61,17 +61,14 @@ import org.opensearch.index.translog.TranslogStats; import org.opensearch.index.warmer.WarmerStats; import org.opensearch.search.suggest.completion.CompletionStats; -import org.opensearch.transport.TransportException; import java.io.IOException; -import java.rmi.UnexpectedException; import java.util.ArrayList; import java.util.Arrays; import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.Optional; -import java.util.stream.Collectors; /** * Global information on indices stats running on a specific node. @@ -171,7 +168,7 @@ private static StatsLevel getAcceptedLevel(String[] levels) { Optional level = Arrays.stream(StatsLevel.values()) .filter(field -> field.getRestName().equals(levels[0])) .findFirst(); - return level.orElse(null); + return level.orElseThrow(() -> new IllegalArgumentException("Level provided is not supported by NodeIndicesStats")); } return null; } @@ -344,7 +341,9 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws stats.toXContent(builder, params); if (StatsLevel.INDICES.getRestName().equals(level)) { - assert statsByIndex!=null || statsByShard!=null: "Expected shard stats or index stats in response for generating [" + StatsLevel.INDICES + "] field"; + assert statsByIndex != null || statsByShard != null : "Expected shard stats or index stats in response for generating [" + + StatsLevel.INDICES + + "] field"; if (statsByIndex == null) { statsByIndex = createStatsByIndex(statsByShard); } @@ -360,7 +359,7 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws builder.endObject(); } else if (StatsLevel.SHARDS.getRestName().equals(level)) { builder.startObject(StatsLevel.SHARDS.getRestName()); - assert statsByShard != null: "Expected shard stats in response for generating [" + StatsLevel.SHARDS + "] field"; + assert statsByShard != null : "Expected shard stats in response for generating [" + StatsLevel.SHARDS + "] field"; for (Map.Entry> entry : statsByShard.entrySet()) { builder.startArray(entry.getKey().getName()); for (IndexShardStats indexShardStats : entry.getValue()) { From d78c9488aede3d01c615ec4ad3346e7fee9e17d3 Mon Sep 17 00:00:00 2001 From: Pranshu Shukla Date: Tue, 23 Jul 2024 19:01:37 +0530 Subject: [PATCH 23/25] Refactor validation and simplify tests Signed-off-by: Pranshu Shukla --- .../org/opensearch/nodestats/NodeStatsIT.java | 29 ++++------- .../opensearch/indices/IndicesService.java | 3 +- .../opensearch/indices/NodeIndicesStats.java | 15 +++--- .../cluster/node/stats/NodeStatsTests.java | 50 ++++++++----------- 4 files changed, 39 insertions(+), 58 deletions(-) diff --git a/server/src/internalClusterTest/java/org/opensearch/nodestats/NodeStatsIT.java b/server/src/internalClusterTest/java/org/opensearch/nodestats/NodeStatsIT.java index b2c834a10a13d..22c1679babb52 100644 --- a/server/src/internalClusterTest/java/org/opensearch/nodestats/NodeStatsIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/nodestats/NodeStatsIT.java @@ -504,9 +504,7 @@ public void testNodeIndicesStatsXContentWithAggregationOnNodes() { } public void testNodeIndicesStatsUnknownLevelThrowsException() { - List testLevels = new ArrayList<>(); - - testLevels.add(MockStatsLevel.UNKNOWN); + MockStatsLevel testLevel = MockStatsLevel.UNKNOWN; internalCluster().startNode(); ensureGreen(); String indexName = "test1"; @@ -519,24 +517,17 @@ public void testNodeIndicesStatsUnknownLevelThrowsException() { ); ensureGreen(); - testLevels.forEach(testLevel -> { - NodesStatsResponse response; - CommonStatsFlags commonStatsFlags = new CommonStatsFlags(); - commonStatsFlags.setIncludeIndicesStatsByLevel(true); - if (!testLevel.equals(MockStatsLevel.NULL)) { - ArrayList level_arg = new ArrayList<>(); - level_arg.add(testLevel.getRestName()); + NodesStatsResponse response; + CommonStatsFlags commonStatsFlags = new CommonStatsFlags(); + commonStatsFlags.setIncludeIndicesStatsByLevel(true); + ArrayList level_arg = new ArrayList<>(); + level_arg.add(testLevel.getRestName()); - commonStatsFlags.setLevels(level_arg.toArray(new String[0])); - } - response = client().admin().cluster().prepareNodesStats().setIndices(commonStatsFlags).get(); + commonStatsFlags.setLevels(level_arg.toArray(new String[0])); + response = client().admin().cluster().prepareNodesStats().setIndices(commonStatsFlags).get(); - assertTrue(response.hasFailures()); - assertEquals( - "Level provided is not supported by NodeIndicesStats", - response.failures().get(0).getCause().getCause().getMessage() - ); - }); + assertTrue(response.hasFailures()); + assertEquals("Level provided is not supported by NodeIndicesStats", response.failures().get(0).getCause().getCause().getMessage()); } private Map xContentBuilderToMap(XContentBuilder xContentBuilder) { diff --git a/server/src/main/java/org/opensearch/indices/IndicesService.java b/server/src/main/java/org/opensearch/indices/IndicesService.java index 845fed095065c..8a05d868eca4b 100644 --- a/server/src/main/java/org/opensearch/indices/IndicesService.java +++ b/server/src/main/java/org/opensearch/indices/IndicesService.java @@ -693,7 +693,8 @@ public NodeIndicesStats stats(CommonStatsFlags flags) { } } if (flags.getIncludeIndicesStatsByLevel()) { - return new NodeIndicesStats(commonStats, statsByShard(this, flags), searchRequestStats, flags.getLevels()); + NodeIndicesStats.StatsLevel statsLevel = NodeIndicesStats.getAcceptedLevel(flags.getLevels()); + return new NodeIndicesStats(commonStats, statsByShard(this, flags), searchRequestStats, statsLevel); } else { return new NodeIndicesStats(commonStats, statsByShard(this, flags), searchRequestStats); } diff --git a/server/src/main/java/org/opensearch/indices/NodeIndicesStats.java b/server/src/main/java/org/opensearch/indices/NodeIndicesStats.java index 4a5e216b514ae..c629eaf867fc8 100644 --- a/server/src/main/java/org/opensearch/indices/NodeIndicesStats.java +++ b/server/src/main/java/org/opensearch/indices/NodeIndicesStats.java @@ -128,7 +128,7 @@ public NodeIndicesStats( CommonStats oldStats, Map> statsByShard, SearchRequestStats searchRequestStats, - String[] levels + StatsLevel level ) { // make a total common stats from old ones and current ones this.stats = oldStats; @@ -144,7 +144,6 @@ public NodeIndicesStats( this.stats.search.setSearchRequestStats(searchRequestStats); } - StatsLevel level = getAcceptedLevel(levels); if (level != null) { switch (level) { case INDICES: @@ -163,7 +162,7 @@ public NodeIndicesStats( * @param levels - levels sent in the request. * @return Corresponding identified enum {@link StatsLevel} */ - private static StatsLevel getAcceptedLevel(String[] levels) { + public static StatsLevel getAcceptedLevel(String[] levels) { if (levels != null && levels.length > 0) { Optional level = Arrays.stream(StatsLevel.values()) .filter(field -> field.getRestName().equals(levels[0])) @@ -349,12 +348,10 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws } builder.startObject(StatsLevel.INDICES.getRestName()); - if (statsByIndex != null) { - for (Map.Entry entry : statsByIndex.entrySet()) { - builder.startObject(entry.getKey().getName()); - entry.getValue().toXContent(builder, params); - builder.endObject(); - } + for (Map.Entry entry : statsByIndex.entrySet()) { + builder.startObject(entry.getKey().getName()); + entry.getValue().toXContent(builder, params); + builder.endObject(); } builder.endObject(); } else if (StatsLevel.SHARDS.getRestName().equals(level)) { diff --git a/server/src/test/java/org/opensearch/action/admin/cluster/node/stats/NodeStatsTests.java b/server/src/test/java/org/opensearch/action/admin/cluster/node/stats/NodeStatsTests.java index a22e2fbb914b8..a0225a0bf6193 100644 --- a/server/src/test/java/org/opensearch/action/admin/cluster/node/stats/NodeStatsTests.java +++ b/server/src/test/java/org/opensearch/action/admin/cluster/node/stats/NodeStatsTests.java @@ -1107,9 +1107,9 @@ public MockNodeIndicesStats( CommonStats oldStats, Map> statsByShard, SearchRequestStats searchRequestStats, - String[] levels + StatsLevel level ) { - super(oldStats, statsByShard, searchRequestStats, levels); + super(oldStats, statsByShard, searchRequestStats, level); } public CommonStats getStats() { @@ -1143,7 +1143,7 @@ public void testOldVersionNodes() throws IOException { Index newIndex = new Index("index", "_na_"); - MockNodeIndicesStats mockNodeIndicesStats = generateMockNodeIndicesStats(commonStats, newIndex, commonStatsFlags); + MockNodeIndicesStats mockNodeIndicesStats = generateMockNodeIndicesStats(commonStats, newIndex, commonStatsFlags, null); // To test out scenario when the incoming node stats response is from a node with an older ES Version. try (BytesStreamOutput out = new BytesStreamOutput()) { @@ -1187,22 +1187,17 @@ public void testNodeIndicesStatsSerialization() throws IOException { commonStatsFlags.set(CommonStatsFlags.Flag.Indexing, true); commonStatsFlags.setIncludeIndicesStatsByLevel(true); - levelParams.forEach(levelParam -> { - ArrayList level_arg = new ArrayList<>(); - level_arg.add(levelParam.getRestName()); - - commonStatsFlags.setLevels(level_arg.toArray(new String[0])); - + levelParams.forEach(level -> { Index newIndex = new Index("index", "_na_"); - MockNodeIndicesStats mockNodeIndicesStats = generateMockNodeIndicesStats(commonStats, newIndex, commonStatsFlags); + MockNodeIndicesStats mockNodeIndicesStats = generateMockNodeIndicesStats(commonStats, newIndex, commonStatsFlags, level); // To test out scenario when the incoming node stats response is from a node with an older ES Version. try (BytesStreamOutput out = new BytesStreamOutput()) { mockNodeIndicesStats.writeTo(out); try (StreamInput in = out.bytes().streamInput()) { MockNodeIndicesStats newNodeIndicesStats = new MockNodeIndicesStats(in); - switch (levelParam) { + switch (level) { case NODE: assertNull(newNodeIndicesStats.getStatsByIndex()); assertNull(newNodeIndicesStats.getStatsByShard()); @@ -1243,15 +1238,11 @@ public void testNodeIndicesStatsToXContent() { commonStatsFlags.set(CommonStatsFlags.Flag.Indexing, true); commonStatsFlags.setIncludeIndicesStatsByLevel(true); - levelParams.forEach(levelParam -> { - ArrayList level_arg = new ArrayList<>(); - level_arg.add(levelParam.getRestName()); - - commonStatsFlags.setLevels(level_arg.toArray(new String[0])); + levelParams.forEach(level -> { Index newIndex = new Index("index", "_na_"); - MockNodeIndicesStats mockNodeIndicesStats = generateMockNodeIndicesStats(commonStats, newIndex, commonStatsFlags); + MockNodeIndicesStats mockNodeIndicesStats = generateMockNodeIndicesStats(commonStats, newIndex, commonStatsFlags, level); XContentBuilder builder = null; try { @@ -1259,14 +1250,14 @@ public void testNodeIndicesStatsToXContent() { builder.startObject(); builder = mockNodeIndicesStats.toXContent( builder, - new ToXContent.MapParams(Collections.singletonMap("level", levelParam.getRestName())) + new ToXContent.MapParams(Collections.singletonMap("level", level.getRestName())) ); builder.endObject(); Map xContentMap = xContentBuilderToMap(builder); LinkedHashMap indicesStatsMap = (LinkedHashMap) xContentMap.get(NodeIndicesStats.StatsLevel.INDICES.getRestName()); - switch (levelParam) { + switch (level) { case NODE: assertFalse(indicesStatsMap.containsKey(NodeIndicesStats.StatsLevel.INDICES.getRestName())); assertFalse(indicesStatsMap.containsKey(NodeIndicesStats.StatsLevel.SHARDS.getRestName())); @@ -1318,16 +1309,12 @@ public void testNodeIndicesStatsWithAndWithoutAggregations() throws IOException commonStatsFlags.setIncludeIndicesStatsByLevel(true); - Arrays.stream(NodeIndicesStats.StatsLevel.values()).forEach(enumLevel -> { - String level = enumLevel.getRestName(); - List levelList = new ArrayList<>(); - levelList.add(level); - + Arrays.stream(NodeIndicesStats.StatsLevel.values()).forEach(level -> { MockNodeIndicesStats aggregatedNodeIndicesStats = new MockNodeIndicesStats( new CommonStats(commonStatsFlags), statsByShards, new SearchRequestStats(clusterSettings), - levelList.toArray(new String[0]) + level ); XContentBuilder nonAggregatedBuilder = null; @@ -1337,7 +1324,7 @@ public void testNodeIndicesStatsWithAndWithoutAggregations() throws IOException nonAggregatedBuilder.startObject(); nonAggregatedBuilder = nonAggregatedNodeIndicesStats.toXContent( nonAggregatedBuilder, - new ToXContent.MapParams(Collections.singletonMap("level", level)) + new ToXContent.MapParams(Collections.singletonMap("level", level.getRestName())) ); nonAggregatedBuilder.endObject(); Map nonAggregatedContentMap = xContentBuilderToMap(nonAggregatedBuilder); @@ -1346,7 +1333,7 @@ public void testNodeIndicesStatsWithAndWithoutAggregations() throws IOException aggregatedBuilder.startObject(); aggregatedBuilder = aggregatedNodeIndicesStats.toXContent( aggregatedBuilder, - new ToXContent.MapParams(Collections.singletonMap("level", level)) + new ToXContent.MapParams(Collections.singletonMap("level", level.getRestName())) ); aggregatedBuilder.endObject(); Map aggregatedContentMap = xContentBuilderToMap(aggregatedBuilder); @@ -1423,7 +1410,12 @@ private Map xContentBuilderToMap(XContentBuilder xContentBuilder return XContentHelper.convertToMap(BytesReference.bytes(xContentBuilder), true, xContentBuilder.contentType()).v2(); } - public MockNodeIndicesStats generateMockNodeIndicesStats(CommonStats commonStats, Index index, CommonStatsFlags commonStatsFlags) { + public MockNodeIndicesStats generateMockNodeIndicesStats( + CommonStats commonStats, + Index index, + CommonStatsFlags commonStatsFlags, + NodeIndicesStats.StatsLevel level + ) { DiscoveryNode localNode = new DiscoveryNode("local", buildNewFakeTransportAddress(), Version.CURRENT); Map> statsByShard = new HashMap<>(); List indexShardStatsList = new ArrayList<>(); @@ -1467,7 +1459,7 @@ public MockNodeIndicesStats generateMockNodeIndicesStats(CommonStats commonStats new CommonStats(commonStatsFlags), statsByShard, new SearchRequestStats(clusterSettings), - commonStatsFlags.getLevels() + level ); } else { return new MockNodeIndicesStats(new CommonStats(commonStatsFlags), statsByShard, new SearchRequestStats(clusterSettings)); From bcceb86d8d4611531a4a5a4ffc902a413b1e3d88 Mon Sep 17 00:00:00 2001 From: Pranshu Shukla Date: Wed, 28 Aug 2024 10:26:29 +0530 Subject: [PATCH 24/25] Addressing comments Signed-off-by: Pranshu Shukla --- CHANGELOG.md | 1 + .../opensearch/indices/NodeIndicesStats.java | 25 +++++++++++-------- 2 files changed, 16 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3dff44ed96dfd..39706af949416 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -27,6 +27,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - Adding access to noSubMatches and noOverlappingMatches in Hyphenation ([#13895](https://github.com/opensearch-project/OpenSearch/pull/13895)) - Add support for index level max slice count setting for concurrent segment search ([#15336](https://github.com/opensearch-project/OpenSearch/pull/15336)) - Add concurrent search support for Derived Fields ([#15326](https://github.com/opensearch-project/OpenSearch/pull/15326)) +- Optimize NodeIndicesStats output behind flag ([#14454](https://github.com/opensearch-project/OpenSearch/pull/14454)) ### Dependencies - Bump `netty` from 4.1.111.Final to 4.1.112.Final ([#15081](https://github.com/opensearch-project/OpenSearch/pull/15081)) diff --git a/server/src/main/java/org/opensearch/indices/NodeIndicesStats.java b/server/src/main/java/org/opensearch/indices/NodeIndicesStats.java index c629eaf867fc8..83a759cdb71c5 100644 --- a/server/src/main/java/org/opensearch/indices/NodeIndicesStats.java +++ b/server/src/main/java/org/opensearch/indices/NodeIndicesStats.java @@ -86,13 +86,11 @@ public NodeIndicesStats(StreamInput in) throws IOException { if (in.getVersion().onOrAfter(Version.V_3_0_0)) { // contains statsByIndex if (in.readBoolean()) { - statsByIndex = new HashMap<>(); - readStatsByIndex(in); + statsByIndex = readStatsByIndex(in); } } if (in.readBoolean()) { - statsByShard = new HashMap<>(); - readStatsByShards(in); + statsByShard = readStatsByShard(in); } } @@ -158,8 +156,12 @@ public NodeIndicesStats( /** * By default, the levels passed from the transport action will be a list of strings, since NodeIndicesStats can - * only aggregate on one level, we pick the first accepted level else we ignore if no known level is passed. + * only aggregate on one level, we pick the first accepted level else we ignore if no known level is passed. Level is + * selected based on enum defined in {@link StatsLevel} + * + * Note - we are picking the first level as multiple levels are not supported in the previous versions. * @param levels - levels sent in the request. + * * @return Corresponding identified enum {@link StatsLevel} */ public static StatsLevel getAcceptedLevel(String[] levels) { @@ -172,16 +174,19 @@ public static StatsLevel getAcceptedLevel(String[] levels) { return null; } - private void readStatsByIndex(StreamInput in) throws IOException { + private Map readStatsByIndex(StreamInput in) throws IOException { + Map statsByIndex = new HashMap<>(); int indexEntries = in.readVInt(); for (int i = 0; i < indexEntries; i++) { Index index = new Index(in); CommonStats commonStats = new CommonStats(in); statsByIndex.put(index, commonStats); } + return statsByIndex; } - private void readStatsByShards(StreamInput in) throws IOException { + private Map> readStatsByShard(StreamInput in) throws IOException { + Map> statsByShard = new HashMap<>(); int entries = in.readVInt(); for (int i = 0; i < entries; i++) { Index index = new Index(in); @@ -192,6 +197,7 @@ private void readStatsByShards(StreamInput in) throws IOException { } statsByShard.put(index, indexShardStats); } + return statsByShard; } @Nullable @@ -324,8 +330,7 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws throw new IllegalArgumentException( "level parameter must be one of [" + StatsLevel.INDICES.getRestName() - + "] or " - + "[" + + "] or [" + StatsLevel.NODE.getRestName() + "] or [" + StatsLevel.SHARDS.getRestName() @@ -405,7 +410,7 @@ public List getShardStats(Index index) { * * @opensearch.internal */ - @PublicApi(since = "2.0.0") + @PublicApi(since = "3.0.0") public enum StatsLevel { INDICES("indices"), SHARDS("shards"), From 40e0fc92fe19eadf69101a17b79c488340ef9ecd Mon Sep 17 00:00:00 2001 From: Pranshu Shukla Date: Thu, 29 Aug 2024 18:55:59 +0530 Subject: [PATCH 25/25] Retry Build Signed-off-by: Pranshu Shukla