Skip to content

Commit

Permalink
Addressing comments
Browse files Browse the repository at this point in the history 
Signed-off-by: Pranshu Shukla <[email protected]>
  • Loading branch information
@Pranshu-S
Pranshu-S committed Jul 23, 2024
1 parent 5203097 commit a0dff3f
Show file tree
Hide file tree
Showing 3 changed files with 151 additions and 104 deletions.
222 changes: 134 additions & 88 deletions server/src/internalClusterTest/java/org/opensearch/nodestats/NodeStatsIT.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<String> testLevels = new ArrayList<>();
testLevels.add("null");
testLevels.add(NodeIndicesStats.StatsLevel.NODE.getRestName());
Expand Down Expand Up @@ -297,67 +333,66 @@ 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<String, Object> 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<String, Object> 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);
}
});
}

/**
* 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<MockStatsLevel> testLevels = new ArrayList<>();

testLevels.add(MockStatsLevel.NULL);
Expand Down Expand Up @@ -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,
Expand All @@ -406,51 +454,49 @@ 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,
new ToXContent.MapParams(Collections.singletonMap("level", NodeIndicesStats.StatsLevel.INDICES.getRestName()))
);
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<String, Object> 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);
}
});
}

Expand Down
28 changes: 14 additions & 14 deletions server/src/main/java/org/opensearch/indices/NodeIndicesStats.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<Index, CommonStats> entry : statsByIndex.entrySet()) {
Expand All @@ -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<Index, List<IndexShardStats>> 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<Index, List<IndexShardStats>> 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();
}
Expand Down Expand Up @@ -422,8 +425,5 @@ public String getRestName() {
return restName;
}

public static List<String> acceptedValues() {
return Arrays.stream(StatsLevel.values()).map(StatsLevel::getRestName).collect(Collectors.toList());
}
}
}
5 changes: 3 additions & 2 deletions server/src/test/java/org/opensearch/action/admin/cluster/node/stats/NodeStatsTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1125,7 +1125,7 @@ public Map<Index, List<IndexShardStats>> 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);
Expand Down Expand Up @@ -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<String> levelList = new ArrayList<>();
levelList.add(level);

Expand Down

0 comments on commit a0dff3f

Please sign in to comment.