Skip to content

Commit

Permalink
Remove indices stats consumers.
Browse files Browse the repository at this point in the history 
Signed-off-by: Swetha Guptha <[email protected]>
  • Loading branch information
Swetha Guptha committed Oct 7, 2024
1 parent 1e78e1e commit 304973e
Show file tree
Hide file tree
Showing 4 changed files with 91 additions and 64 deletions.
36 changes: 36 additions & 0 deletions ...rc/internalClusterTest/java/org/opensearch/action/admin/cluster/stats/ClusterStatsIT.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@
import org.opensearch.common.Priority;
import org.opensearch.common.settings.Settings;
import org.opensearch.common.util.concurrent.OpenSearchExecutors;
import org.opensearch.core.common.unit.ByteSizeValue;
import org.opensearch.monitor.os.OsStats;
import org.opensearch.node.NodeRoleSettings;
import org.opensearch.test.OpenSearchIntegTestCase;
Expand Down Expand Up @@ -772,6 +773,41 @@ public void testClusterStatsWithIndexMetricWithDocsFilter() throws IOException {
assertTrue(statsResponseWithAllIndicesMetrics.getIndicesStats().getDocs().getAverageSizeInBytes() > 0);
}

public void testClusterStatsWithSelectiveMetricsFilterAndNoIndex() {
internalCluster().startNode();
ensureGreen();
ClusterStatsResponse statsResponseWithAllIndicesMetrics = client().admin()
.cluster()
.prepareClusterStats()
.useAggregatedNodeLevelResponses(randomBoolean())
.requestMetrics(Set.of(Metric.OS, Metric.FS, Metric.INDICES))
.indexMetrics(Set.of(IndexMetric.FIELDDATA, IndexMetric.SHARDS, IndexMetric.SEGMENTS, IndexMetric.DOCS, IndexMetric.STORE))
.computeAllMetrics(false)
.get();
assertNotNull(statsResponseWithAllIndicesMetrics);
assertNotNull(statsResponseWithAllIndicesMetrics.getNodesStats());
assertNotNull(statsResponseWithAllIndicesMetrics.getIndicesStats());
validateNodeStatsOutput(Set.of(Metric.FS, Metric.OS), statsResponseWithAllIndicesMetrics);
validateIndicesStatsOutput(
Set.of(IndexMetric.FIELDDATA, IndexMetric.SHARDS, IndexMetric.SEGMENTS, IndexMetric.DOCS, IndexMetric.STORE),
statsResponseWithAllIndicesMetrics
);
assertEquals(0, statsResponseWithAllIndicesMetrics.getIndicesStats().getShards().getIndices());
assertEquals(0, statsResponseWithAllIndicesMetrics.getIndicesStats().getShards().getTotal());
assertEquals(0, statsResponseWithAllIndicesMetrics.getIndicesStats().getShards().getPrimaries());
assertEquals(0, statsResponseWithAllIndicesMetrics.getIndicesStats().getDocs().getCount());
assertEquals(0, statsResponseWithAllIndicesMetrics.getIndicesStats().getDocs().getDeleted());
assertEquals(0, statsResponseWithAllIndicesMetrics.getIndicesStats().getDocs().getTotalSizeInBytes());
assertEquals(0, statsResponseWithAllIndicesMetrics.getIndicesStats().getStore().getSizeInBytes());
assertEquals(new ByteSizeValue(0), statsResponseWithAllIndicesMetrics.getIndicesStats().getStore().getReservedSize());
assertEquals(new ByteSizeValue(0), statsResponseWithAllIndicesMetrics.getIndicesStats().getFieldData().getMemorySize());
assertEquals(0, statsResponseWithAllIndicesMetrics.getIndicesStats().getFieldData().getEvictions());
assertNull(statsResponseWithAllIndicesMetrics.getIndicesStats().getFieldData().getFields());
assertEquals(0, statsResponseWithAllIndicesMetrics.getIndicesStats().getSegments().getCount());
assertEquals(0, statsResponseWithAllIndicesMetrics.getIndicesStats().getSegments().getIndexWriterMemoryInBytes());
assertEquals(0, statsResponseWithAllIndicesMetrics.getIndicesStats().getSegments().getVersionMapMemoryInBytes());
}

private void validateNodeStatsOutput(Set<ClusterStatsRequest.Metric> expectedMetrics, ClusterStatsResponse clusterStatsResponse) {
// Ingest, network types, discovery types and packaging types stats are not included here as they don't have a get method exposed.
Set<Metric> NodeMetrics = Set.of(Metric.OS, Metric.JVM, Metric.FS, Metric.PROCESS, Metric.PLUGINS);
Expand Down
84 changes: 36 additions & 48 deletions server/src/main/java/org/opensearch/action/admin/cluster/stats/ClusterStatsIndices.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,6 @@
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.function.Consumer;

/**
* Cluster Stats per index
Expand Down Expand Up @@ -83,46 +82,11 @@ public ClusterStatsIndices(
) {
Map<String, ShardStats> countsPerIndex = new HashMap<>();
this.docs = indicesMetrics.contains(IndexMetric.DOCS) ? new DocsStats() : null;
Consumer<DocsStats> docsStatsConsumer = (docs) -> {
if (this.docs != null) {
this.docs.add(docs);
}
};

this.store = indicesMetrics.contains(IndexMetric.STORE) ? new StoreStats() : null;
Consumer<StoreStats> storeStatsConsumer = (store) -> {
if (this.store != null) {
this.store.add(store);
}
};

this.fieldData = indicesMetrics.contains(IndexMetric.FIELDDATA) ? new FieldDataStats() : null;
Consumer<FieldDataStats> fieldDataConsumer = (fieldDataStats) -> {
if (this.fieldData != null) {
this.fieldData.add(fieldDataStats);
}
};

this.queryCache = indicesMetrics.contains(IndexMetric.QUERY_CACHE) ? new QueryCacheStats() : null;
Consumer<QueryCacheStats> queryCacheStatsConsumer = (queryCacheStats) -> {
if (this.queryCache != null) {
this.queryCache.add(queryCacheStats);
}
};

this.completion = indicesMetrics.contains(IndexMetric.COMPLETION) ? new CompletionStats() : null;
Consumer<CompletionStats> completionStatsConsumer = (completionStats) -> {
if (this.completion != null) {
this.completion.add(completionStats);
}
};

this.segments = indicesMetrics.contains(IndexMetric.SEGMENTS) ? new SegmentsStats() : null;
Consumer<SegmentsStats> segmentsStatsConsumer = (segmentsStats) -> {
if (this.segments != null) {
this.segments.add(segmentsStats);
}
};

for (ClusterStatsNodeResponse r : nodeResponses) {
// Aggregated response from the node
Expand All @@ -139,12 +103,24 @@ public ClusterStatsIndices(
}
}

docsStatsConsumer.accept(r.getAggregatedNodeLevelStats().commonStats.docs);
storeStatsConsumer.accept(r.getAggregatedNodeLevelStats().commonStats.store);
fieldDataConsumer.accept(r.getAggregatedNodeLevelStats().commonStats.fieldData);
queryCacheStatsConsumer.accept(r.getAggregatedNodeLevelStats().commonStats.queryCache);
completionStatsConsumer.accept(r.getAggregatedNodeLevelStats().commonStats.completion);
segmentsStatsConsumer.accept(r.getAggregatedNodeLevelStats().commonStats.segments);
if (docs != null) {
docs.add(r.getAggregatedNodeLevelStats().commonStats.docs);
}
if (store != null) {
store.add(r.getAggregatedNodeLevelStats().commonStats.store);
}
if (fieldData != null) {
fieldData.add(r.getAggregatedNodeLevelStats().commonStats.fieldData);
}
if (queryCache != null) {
queryCache.add(r.getAggregatedNodeLevelStats().commonStats.queryCache);
}
if (completion != null) {
completion.add(r.getAggregatedNodeLevelStats().commonStats.completion);
}
if (segments != null) {
segments.add(r.getAggregatedNodeLevelStats().commonStats.segments);
}
} else {
// Default response from the node
for (org.opensearch.action.admin.indices.stats.ShardStats shardStats : r.shardsStats()) {
Expand All @@ -160,13 +136,25 @@ public ClusterStatsIndices(

if (shardStats.getShardRouting().primary()) {
indexShardStats.primaries++;
docsStatsConsumer.accept(shardCommonStats.docs);
if (docs != null) {
docs.add(shardCommonStats.docs);
}
}
if (store != null) {
store.add(shardCommonStats.store);
}
if (fieldData != null) {
fieldData.add(shardCommonStats.fieldData);
}
if (queryCache != null) {
queryCache.add(shardCommonStats.queryCache);
}
if (completion != null) {
completion.add(shardCommonStats.completion);
}
if (segments != null) {
segments.add(shardCommonStats.segments);
}
storeStatsConsumer.accept(shardCommonStats.store);
fieldDataConsumer.accept(shardCommonStats.fieldData);
queryCacheStatsConsumer.accept(shardCommonStats.queryCache);
completionStatsConsumer.accept(shardCommonStats.completion);
segmentsStatsConsumer.accept(shardCommonStats.segments);
}
}
}
Expand Down
17 changes: 1 addition & 16 deletions .../src/main/java/org/opensearch/action/admin/cluster/stats/TransportClusterStatsAction.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -80,21 +80,6 @@ public class TransportClusterStatsAction extends TransportNodesAction<
TransportClusterStatsAction.ClusterStatsNodeRequest,
ClusterStatsNodeResponse> {

private static final Map<ClusterStatsRequest.IndexMetric, CommonStatsFlags.Flag> INDEX_METRIC_TO_SHARDS_STATS_FLAG_MAP = Map.of(
ClusterStatsRequest.IndexMetric.DOCS,
CommonStatsFlags.Flag.Docs,
ClusterStatsRequest.IndexMetric.STORE,
CommonStatsFlags.Flag.Store,
ClusterStatsRequest.IndexMetric.FIELDDATA,
CommonStatsFlags.Flag.FieldData,
ClusterStatsRequest.IndexMetric.QUERY_CACHE,
CommonStatsFlags.Flag.QueryCache,
ClusterStatsRequest.IndexMetric.COMPLETION,
CommonStatsFlags.Flag.Completion,
ClusterStatsRequest.IndexMetric.SEGMENTS,
CommonStatsFlags.Flag.Segments
);

private static final Map<CommonStatsFlags.Flag, ClusterStatsRequest.IndexMetric> SHARDS_STATS_FLAG_MAP_TO_INDEX_METRIC = Map.of(
CommonStatsFlags.Flag.Docs,
ClusterStatsRequest.IndexMetric.DOCS,
Expand Down Expand Up @@ -278,7 +263,7 @@ private boolean isMetricRequired(Metric metric, ClusterStatsRequest clusterStats
private static CommonStatsFlags getCommonStatsFlags(ClusterStatsNodeRequest nodeRequest) {
Set<CommonStatsFlags.Flag> requestedCommonStatsFlags = new HashSet<>();
if (nodeRequest.request.computeAllMetrics()) {
requestedCommonStatsFlags.addAll(INDEX_METRIC_TO_SHARDS_STATS_FLAG_MAP.values());
requestedCommonStatsFlags.addAll(SHARDS_STATS_FLAG_MAP_TO_INDEX_METRIC.keySet());
} else {
for (Map.Entry<CommonStatsFlags.Flag, ClusterStatsRequest.IndexMetric> entry : SHARDS_STATS_FLAG_MAP_TO_INDEX_METRIC
.entrySet()) {

Check warning on line 269 in server/src/main/java/org/opensearch/action/admin/cluster/stats/TransportClusterStatsAction.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/org/opensearch/action/admin/cluster/stats/TransportClusterStatsAction.java#L269 

Added line #L269 was not covered by tests
Expand Down
18 changes: 18 additions & 0 deletions ...r/src/test/java/org/opensearch/rest/action/admin/cluster/RestClusterStatsActionTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -150,4 +150,22 @@ public void testAllIndexMetricsRequestWithOtherIndicesMetric() {
);
}

public void testIndexMetricsRequestWithoutMetricIndices() {
final HashMap<String, String> params = new HashMap<>();
params.put("metric", "os");
final String indexMetric = randomSubsetOf(1, RestClusterStatsAction.INDEX_METRIC_TO_REQUEST_CONSUMER_MAP.keySet()).get(0);
params.put("index_metric", indexMetric);
final RestRequest request = new FakeRestRequest.Builder(xContentRegistry()).withPath("/_cluster/stats").withParams(params).build();
final IllegalArgumentException e = expectThrows(
IllegalArgumentException.class,
() -> action.prepareRequest(request, mock(NodeClient.class))
);
assertThat(
e,
hasToString(
containsString("request [/_cluster/stats] contains index metrics [" + indexMetric + "] but indices stats not requested")
)
);
}

}

0 comments on commit 304973e

Please sign in to comment.