Skip to content

Commit b741868

Browse files
Pranshu-SPranshu-S
committed
Modify optimisation approach without Cluster Settings
Signed-off-by: Pranshu Shukla <[email protected]>
1 parent 52ba11d commit b741868

File tree

7 files changed

+360
-92
lines changed

7 files changed

+360
-92
lines changed

‎server/src/internalClusterTest/java/org/opensearch/nodestats/NodeStatsIT.java

+124-23
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,11 @@
2222
import org.opensearch.action.update.UpdateRequest;
2323
import org.opensearch.action.update.UpdateResponse;
2424
import org.opensearch.cluster.ClusterState;
25+
import org.opensearch.common.xcontent.XContentFactory;
26+
import org.opensearch.common.xcontent.XContentHelper;
27+
import org.opensearch.core.common.bytes.BytesReference;
28+
import org.opensearch.core.xcontent.ToXContent;
29+
import org.opensearch.core.xcontent.XContentBuilder;
2530
import org.opensearch.index.IndexNotFoundException;
2631
import org.opensearch.index.engine.DocumentMissingException;
2732
import org.opensearch.index.engine.VersionConflictEngineException;
@@ -31,10 +36,13 @@
3136
import org.opensearch.test.OpenSearchIntegTestCase.Scope;
3237
import org.hamcrest.MatcherAssert;
3338

39+
import java.io.IOException;
40+
import java.util.ArrayList;
3441
import java.util.Arrays;
42+
import java.util.Collections;
3543
import java.util.Comparator;
44+
import java.util.LinkedHashMap;
3645
import java.util.Map;
37-
import java.util.ArrayList;
3846
import java.util.concurrent.atomic.AtomicLong;
3947

4048
import static java.util.Collections.singletonMap;
@@ -246,47 +254,140 @@ public void testNodeIndicesStatsDocStatusStatsCreateDeleteUpdate() {
246254
}
247255
}
248256
}
249-
public void testNodeIndicesStatsOptimisedResponse() {
257+
258+
/**
259+
* Default behavior - without consideration of request level param on level, the NodeStatsRequest always
260+
* returns ShardStats which is aggregated on the coordinator node when creating the XContent.
261+
*/
262+
public void testNodeIndicesStatsDefaultResponse() {
263+
String testLevel = randomFrom("null", "node", "indices", "shards", "unknown");
250264
internalCluster().startNode();
251265
ensureGreen();
252266
String indexName = "test1";
253267
index(indexName, "type", "1", "f", "f");
254268
refresh();
255269
ClusterState clusterState = client().admin().cluster().prepareState().get().getState();
256270

257-
NodesStatsResponse response = client().admin().cluster().prepareNodesStats().get();
271+
NodesStatsResponse response;
272+
if (!testLevel.equals("null")) {
273+
ArrayList<String> level_arg = new ArrayList<>();
274+
level_arg.add(testLevel);
275+
276+
CommonStatsFlags commonStatsFlags = new CommonStatsFlags();
277+
commonStatsFlags.setLevels(level_arg.toArray(new String[0]));
278+
response = client().admin().cluster().prepareNodesStats().setIndices(commonStatsFlags).get();
279+
} else {
280+
response = client().admin().cluster().prepareNodesStats().get();
281+
}
282+
258283
response.getNodes().forEach(nodeStats -> {
259284
assertNotNull(nodeStats.getIndices().getShardStats(clusterState.metadata().index(indexName).getIndex()));
260-
assertNull(nodeStats.getIndices().getIndexStats(clusterState.metadata().index(indexName).getIndex()));
285+
try {
286+
// Without any param - default is level = nodes
287+
XContentBuilder builder = XContentFactory.jsonBuilder();
288+
builder.startObject();
289+
builder = nodeStats.getIndices().toXContent(builder, ToXContent.EMPTY_PARAMS);
290+
builder.endObject();
291+
292+
Map<String, Object> xContentMap = xContentBuilderToMap(builder);
293+
LinkedHashMap indicesStatsMap = (LinkedHashMap) xContentMap.get("indices");
294+
assertFalse(indicesStatsMap.containsKey("indices"));
295+
assertFalse(indicesStatsMap.containsKey("shards"));
296+
297+
// With param containing level as 'indices', the indices stats are returned
298+
builder = XContentFactory.jsonBuilder();
299+
builder.startObject();
300+
builder = nodeStats.getIndices()
301+
.toXContent(builder, new ToXContent.MapParams(Collections.singletonMap("level", "indices")));
302+
builder.endObject();
303+
304+
xContentMap = xContentBuilderToMap(builder);
305+
indicesStatsMap = (LinkedHashMap) xContentMap.get("indices");
306+
assertTrue(indicesStatsMap.containsKey("indices"));
307+
assertFalse(indicesStatsMap.containsKey("shards"));
308+
309+
LinkedHashMap indexLevelStats = (LinkedHashMap) indicesStatsMap.get("indices");
310+
assertTrue(indexLevelStats.containsKey(indexName));
311+
312+
// With param containing level as 'shards', the shard stats are returned
313+
builder = XContentFactory.jsonBuilder();
314+
builder.startObject();
315+
builder = nodeStats.getIndices().toXContent(builder, new ToXContent.MapParams(Collections.singletonMap("level", "shards")));
316+
builder.endObject();
317+
318+
xContentMap = xContentBuilderToMap(builder);
319+
indicesStatsMap = (LinkedHashMap) xContentMap.get("indices");
320+
assertFalse(indicesStatsMap.containsKey("indices"));
321+
assertTrue(indicesStatsMap.containsKey("shards"));
322+
323+
LinkedHashMap shardLevelStats = (LinkedHashMap) indicesStatsMap.get("shards");
324+
assertTrue(shardLevelStats.containsKey(indexName));
325+
} catch (IOException e) {
326+
throw new RuntimeException(e);
327+
}
261328
});
329+
}
330+
331+
/**
332+
* Optimized behavior - to avoid unnecessary IO in the form of shard-stats when not required, we not honor the levels on the
333+
* individual data nodes instead and pre-compute information as required.
334+
*/
335+
public void testNodeIndicesStatsOptimizedResponse() {
336+
String testLevel = randomFrom("null", "node", "indices", "shards", "unknown");
337+
internalCluster().startNode();
338+
ensureGreen();
339+
String indexName = "test1";
340+
index(indexName, "type", "1", "f", "f");
341+
refresh();
342+
343+
NodesStatsResponse response;
262344
CommonStatsFlags commonStatsFlags = new CommonStatsFlags();
263345
commonStatsFlags.optimizeNodeIndicesStatsOnLevel(true);
264-
response = client().admin().cluster().prepareNodesStats().setIndices(commonStatsFlags).get();
265-
response.getNodes().forEach(nodeStats -> {
266-
assertNull(nodeStats.getIndices().getShardStats(clusterState.metadata().index(indexName).getIndex()));
267-
assertNull(nodeStats.getIndices().getIndexStats(clusterState.metadata().index(indexName).getIndex()));
346+
if (!testLevel.equals("null")) {
347+
ArrayList<String> level_arg = new ArrayList<>();
348+
level_arg.add(testLevel);
268349

269-
});
270-
ArrayList<String> level_arg = new ArrayList<>();
271-
level_arg.add("indices");
272-
commonStatsFlags.setLevels(level_arg.toArray(new String[0]));
350+
commonStatsFlags.setLevels(level_arg.toArray(new String[0]));
351+
}
273352
response = client().admin().cluster().prepareNodesStats().setIndices(commonStatsFlags).get();
274-
response.getNodes().forEach(nodeStats -> {
275-
assertNotNull(nodeStats.getIndices().getIndexStats(clusterState.metadata().index(indexName).getIndex()));
276-
assertNull(nodeStats.getIndices().getShardStats(clusterState.metadata().index(indexName).getIndex()));
277-
});
278353

279-
level_arg.clear();
280-
level_arg.add("shards");
281-
commonStatsFlags.setLevels(level_arg.toArray(new String[0]));
282-
response = client().admin().cluster().prepareNodesStats().setIndices(commonStatsFlags).get();
283354
response.getNodes().forEach(nodeStats -> {
284-
assertNotNull(nodeStats.getIndices().getShardStats(clusterState.metadata().index(indexName).getIndex()));
285-
assertNull(nodeStats.getIndices().getIndexStats(clusterState.metadata().index(indexName).getIndex()));
355+
try {
356+
XContentBuilder builder = XContentFactory.jsonBuilder();
357+
builder.startObject();
358+
builder = nodeStats.getIndices().toXContent(builder, new ToXContent.MapParams(Collections.singletonMap("level", "shards")));
359+
builder.endObject();
360+
361+
Map<String, Object> xContentMap = xContentBuilderToMap(builder);
362+
LinkedHashMap indicesStatsMap = (LinkedHashMap) xContentMap.get("indices");
363+
LinkedHashMap indicesStats = (LinkedHashMap) indicesStatsMap.get("indices");
364+
LinkedHashMap shardStats = (LinkedHashMap) indicesStatsMap.get("shards");
365+
366+
switch (testLevel) {
367+
case "shards":
368+
assertFalse(shardStats.isEmpty());
369+
assertFalse(indicesStats.isEmpty());
370+
break;
371+
case "indices":
372+
assertTrue(shardStats.isEmpty());
373+
assertFalse(indicesStats.isEmpty());
374+
break;
375+
case "node":
376+
case "null":
377+
case "unknown":
378+
assertTrue(shardStats.isEmpty());
379+
assertTrue(indicesStats.isEmpty());
380+
break;
381+
}
382+
} catch (IOException e) {
383+
throw new RuntimeException(e);
384+
}
286385
});
287386
}
288387

289-
388+
private Map<String, Object> xContentBuilderToMap(XContentBuilder xContentBuilder) {
389+
return XContentHelper.convertToMap(BytesReference.bytes(xContentBuilder), true, xContentBuilder.contentType()).v2();
390+
}
290391

291392
private void assertDocStatusStats() {
292393
DocStatusStats docStatusStats = client().admin()

‎server/src/main/java/org/opensearch/action/admin/cluster/node/stats/NodesStatsRequest.java

+2-2
Original file line numberDiff line numberDiff line change
@@ -130,8 +130,8 @@ public NodesStatsRequest indices(boolean indices) {
130130
/**
131131
* Use Optimized Response filtered based on level
132132
*/
133-
public NodesStatsRequest useOptimizedNodeIndicesStats(boolean useOptimizedNodeIndicesStats){
134-
if (this.indices!=null) {
133+
public NodesStatsRequest useOptimizedNodeIndicesStats(boolean useOptimizedNodeIndicesStats) {
134+
if (this.indices != null) {
135135
this.indices.optimizeNodeIndicesStatsOnLevel(true);
136136
}
137137
return this;

‎server/src/main/java/org/opensearch/action/admin/indices/stats/CommonStatsFlags.java

+2-3
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,6 @@ public class CommonStatsFlags implements Writeable, Cloneable {
6969
private String[] levels = new String[0];
7070
private boolean optimizeNodeIndicesStatsOnLevel = false;
7171

72-
7372
/**
7473
* @param flags flags to set. If no flags are supplied, default flags will be set.
7574
*/
@@ -102,7 +101,7 @@ public CommonStatsFlags(StreamInput in) throws IOException {
102101
includeCaches = in.readEnumSet(CacheType.class);
103102
levels = in.readStringArray();
104103
}
105-
if (in.getVersion().onOrAfter(Version.V_2_15_0)) {
104+
if (in.getVersion().onOrAfter(Version.V_2_16_0)) {
106105
optimizeNodeIndicesStatsOnLevel = in.readBoolean();
107106
}
108107
}
@@ -129,7 +128,7 @@ public void writeTo(StreamOutput out) throws IOException {
129128
out.writeEnumSet(includeCaches);
130129
out.writeStringArrayNullable(levels);
131130
}
132-
if (out.getVersion().onOrAfter(Version.V_2_15_0)){
131+
if (out.getVersion().onOrAfter(Version.V_2_16_0)) {
133132
out.writeBoolean(optimizeNodeIndicesStatsOnLevel);
134133
}
135134
}

‎server/src/main/java/org/opensearch/indices/IndicesService.java

-1
Original file line numberDiff line numberDiff line change
@@ -693,7 +693,6 @@ public NodeIndicesStats stats(CommonStatsFlags flags) {
693693
}
694694
}
695695
if (flags.optimizeNodeIndicesStatsOnLevel()) {
696-
logger.info("Picked NodeIndicesStats");
697696
return new NodeIndicesStats(commonStats, statsByShard(this, flags), searchRequestStats, flags.getLevels());
698697
}
699698
return new NodeIndicesStats(commonStats, statsByShard(this, flags), searchRequestStats);

‎server/src/main/java/org/opensearch/indices/NodeIndicesStats.java

+39-62
Original file line numberDiff line numberDiff line change
@@ -32,8 +32,6 @@
3232

3333
package org.opensearch.indices;
3434

35-
import org.apache.logging.log4j.LogManager;
36-
import org.apache.logging.log4j.Logger;
3735
import org.opensearch.Version;
3836
import org.opensearch.action.admin.indices.stats.CommonStats;
3937
import org.opensearch.action.admin.indices.stats.IndexShardStats;
@@ -78,13 +76,13 @@
7876
*/
7977
@PublicApi(since = "1.0.0")
8078
public class NodeIndicesStats implements Writeable, ToXContentFragment {
81-
private CommonStats stats;
82-
private Map<Index, CommonStats> statsByIndex;
83-
private Map<Index, List<IndexShardStats>> statsByShard;
79+
protected CommonStats stats;
80+
protected Map<Index, CommonStats> statsByIndex;
81+
protected Map<Index, List<IndexShardStats>> statsByShard;
8482

8583
public NodeIndicesStats(StreamInput in) throws IOException {
8684
stats = new CommonStats(in);
87-
if (in.getVersion().onOrAfter(Version.V_2_15_0)) {
85+
if (in.getVersion().onOrAfter(Version.V_2_16_0)) {
8886
// contains statsByIndex
8987
if (in.readBoolean()) {
9088
statsByIndex = new HashMap<>();
@@ -136,11 +134,17 @@ public NodeIndicesStats(
136134
}
137135

138136
if (levels != null) {
139-
if (Arrays.stream(levels).anyMatch(NodeIndicesStats.levels.indices::equals)) {
140-
this.statsByIndex = createStatsByIndex(statsByShard);
141-
} else if (Arrays.stream(levels).anyMatch(NodeIndicesStats.levels.shards::equals)) {
142-
this.statsByShard = statsByShard;
143-
}
137+
Arrays.stream(levels).anyMatch(level -> {
138+
switch (level) {
139+
case Fields.INDICES:
140+
this.statsByIndex = createStatsByIndex(statsByShard);
141+
return true;
142+
case Fields.SHARDS:
143+
this.statsByShard = statsByShard;
144+
return true;
145+
}
146+
return false;
147+
});
144148
}
145149
}
146150

@@ -250,7 +254,7 @@ public RecoveryStats getRecoveryStats() {
250254
public void writeTo(StreamOutput out) throws IOException {
251255
stats.writeTo(out);
252256

253-
if (out.getVersion().onOrAfter(Version.V_2_15_0)) {
257+
if (out.getVersion().onOrAfter(Version.V_2_16_0)) {
254258
out.writeBoolean(statsByIndex != null);
255259
if (statsByIndex != null) {
256260
writeStatsByIndex(out);
@@ -300,29 +304,33 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws
300304
builder.startObject(Fields.INDICES);
301305
stats.toXContent(builder, params);
302306

303-
if (levels.indices.equals(level)) {
304-
builder.startObject(Fields.INDICES);
305-
if (statsByIndex == null && statsByShard!=null) {
307+
if (Fields.INDICES.equals(level)) {
308+
if (statsByIndex == null && statsByShard != null) {
306309
statsByIndex = createStatsByIndex(statsByShard);
307310
}
308-
for (Map.Entry<Index, CommonStats> entry : statsByIndex.entrySet()) {
309-
builder.startObject(entry.getKey().getName());
310-
entry.getValue().toXContent(builder, params);
311-
builder.endObject();
311+
builder.startObject(Fields.INDICES);
312+
if (statsByIndex != null) {
313+
for (Map.Entry<Index, CommonStats> entry : statsByIndex.entrySet()) {
314+
builder.startObject(entry.getKey().getName());
315+
entry.getValue().toXContent(builder, params);
316+
builder.endObject();
317+
}
312318
}
313319
builder.endObject();
314-
} else if (levels.shards.equals(level)) {
320+
} else if (Fields.SHARDS.equals(level)) {
315321
builder.startObject("shards");
316-
for (Map.Entry<Index, List<IndexShardStats>> entry : statsByShard.entrySet()) {
317-
builder.startArray(entry.getKey().getName());
318-
for (IndexShardStats indexShardStats : entry.getValue()) {
319-
builder.startObject().startObject(String.valueOf(indexShardStats.getShardId().getId()));
320-
for (ShardStats shardStats : indexShardStats.getShards()) {
321-
shardStats.toXContent(builder, params);
322+
if (statsByShard != null) {
323+
for (Map.Entry<Index, List<IndexShardStats>> entry : statsByShard.entrySet()) {
324+
builder.startArray(entry.getKey().getName());
325+
for (IndexShardStats indexShardStats : entry.getValue()) {
326+
builder.startObject().startObject(String.valueOf(indexShardStats.getShardId().getId()));
327+
for (ShardStats shardStats : indexShardStats.getShards()) {
328+
shardStats.toXContent(builder, params);
329+
}
330+
builder.endObject().endObject();
322331
}
323-
builder.endObject().endObject();
332+
builder.endArray();
324333
}
325-
builder.endArray();
326334
}
327335
builder.endObject();
328336
}
@@ -356,44 +364,13 @@ public List<IndexShardStats> getShardStats(Index index) {
356364
}
357365
}
358366

359-
public CommonStats getIndexStats(Index index) {
360-
if (statsByIndex == null) {
361-
return null;
362-
} else {
363-
return statsByIndex.get(index);
364-
}
365-
}
366-
367367
/**
368368
* Fields used for parsing and toXContent
369369
*
370370
* @opensearch.internal
371371
*/
372-
static final class Fields {
373-
static final String INDICES = "indices";
374-
}
375-
376-
/**
377-
* Levels for the NodeIndicesStats
378-
*/
379-
public enum levels {
380-
node("node"),
381-
indices("indices"),
382-
shards("shards");
383-
384-
private final String name;
385-
386-
levels(String name) {
387-
this.name = name;
388-
}
389-
390-
@Override
391-
public String toString() {
392-
return name;
393-
}
394-
395-
public boolean equals(String value) {
396-
return this.name.equals(value);
397-
}
372+
public static final class Fields {
373+
public static final String INDICES = "indices";
374+
public static final String SHARDS = "shards";
398375
}
399376
}

0 commit comments

Comments
 (0)