From 67749bd1f3c498ec1340c19481e05aa8cc5f837b Mon Sep 17 00:00:00 2001 From: Pranshu Shukla Date: Mon, 22 Jul 2024 16:12:30 +0530 Subject: [PATCH] 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,