Skip to content

Commit 5fb7d51

Browse files
Addressing comments
Signed-off-by: Bharathwaj G <bharath78910@gmail.com>
1 parent f861207 commit 5fb7d51

9 files changed

Lines changed: 39 additions & 37 deletions

File tree

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

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@
5656
import org.opensearch.monitor.os.OsStats;
5757
import org.opensearch.monitor.process.ProcessStats;
5858
import org.opensearch.node.AdaptiveSelectionStats;
59-
import org.opensearch.node.DownstreamNodesPerfStats;
59+
import org.opensearch.node.NodesPerformanceStats;
6060
import org.opensearch.script.ScriptCacheStats;
6161
import org.opensearch.script.ScriptStats;
6262
import org.opensearch.search.backpressure.stats.SearchBackpressureStats;
@@ -144,7 +144,7 @@ public class NodeStats extends BaseNodeResponse implements ToXContentFragment {
144144
private SearchPipelineStats searchPipelineStats;
145145

146146
@Nullable
147-
private DownstreamNodesPerfStats downstreamNodesPerfStats;
147+
private NodesPerformanceStats nodesPerformanceStats;
148148

149149
public NodeStats(StreamInput in) throws IOException {
150150
super(in);
@@ -203,9 +203,9 @@ public NodeStats(StreamInput in) throws IOException {
203203
searchPipelineStats = null;
204204
}
205205
if (in.getVersion().onOrAfter(Version.V_3_0_0)) { // make it 2.11 when we backport
206-
downstreamNodesPerfStats = in.readOptionalWriteable(DownstreamNodesPerfStats::new);
206+
nodesPerformanceStats = in.readOptionalWriteable(NodesPerformanceStats::new);
207207
} else {
208-
downstreamNodesPerfStats = null;
208+
nodesPerformanceStats = null;
209209
}
210210
}
211211

@@ -225,7 +225,7 @@ public NodeStats(
225225
@Nullable DiscoveryStats discoveryStats,
226226
@Nullable IngestStats ingestStats,
227227
@Nullable AdaptiveSelectionStats adaptiveSelectionStats,
228-
@Nullable DownstreamNodesPerfStats downstreamNodesPerfStats,
228+
@Nullable NodesPerformanceStats nodesPerformanceStats,
229229
@Nullable ScriptCacheStats scriptCacheStats,
230230
@Nullable IndexingPressureStats indexingPressureStats,
231231
@Nullable ShardIndexingPressureStats shardIndexingPressureStats,
@@ -250,7 +250,7 @@ public NodeStats(
250250
this.scriptStats = scriptStats;
251251
this.discoveryStats = discoveryStats;
252252
this.ingestStats = ingestStats;
253-
this.downstreamNodesPerfStats = downstreamNodesPerfStats;
253+
this.nodesPerformanceStats = nodesPerformanceStats;
254254
this.scriptCacheStats = scriptCacheStats;
255255
this.indexingPressureStats = indexingPressureStats;
256256
this.shardIndexingPressureStats = shardIndexingPressureStats;
@@ -356,8 +356,8 @@ public AdaptiveSelectionStats getAdaptiveSelectionStats() {
356356
}
357357

358358
@Nullable
359-
public DownstreamNodesPerfStats getNodesPerformanceStats() {
360-
return downstreamNodesPerfStats;
359+
public NodesPerformanceStats getNodesPerformanceStats() {
360+
return nodesPerformanceStats;
361361
}
362362

363363
@Nullable
@@ -447,7 +447,7 @@ public void writeTo(StreamOutput out) throws IOException {
447447
out.writeOptionalWriteable(searchPipelineStats);
448448
}
449449
if (out.getVersion().onOrAfter(Version.V_3_0_0)) { // TODO : make it 2.11 when we backport
450-
out.writeOptionalWriteable(downstreamNodesPerfStats);
450+
out.writeOptionalWriteable(nodesPerformanceStats);
451451
}
452452
}
453453

server/src/main/java/org/opensearch/node/Node.java

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1069,7 +1069,6 @@ protected Node(
10691069
searchPipelineService,
10701070
fileCache,
10711071
taskCancellationMonitoringService,
1072-
nodePerformanceTracker,
10731072
performanceCollectorService
10741073
);
10751074

server/src/main/java/org/opensearch/node/NodePerformanceStatistics.java

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -53,13 +53,18 @@ public String toString() {
5353
sb.append(nodeId).append("](");
5454
sb.append("CPU utilization percent: ").append(String.format(Locale.ROOT, "%.1f", cpuUtilizationPercent));
5555
sb.append(", Memory utilization percent: ").append(String.format(Locale.ROOT, "%.1f", memoryUtilizationPercent));
56-
sb.append(", Timestamp: ").append(memoryUtilizationPercent);
56+
sb.append(", Timestamp: ").append(timestamp);
5757
sb.append(")");
5858
return sb.toString();
5959
}
6060

61-
NodePerformanceStatistics(NodePerformanceStatistics nodeStats) {
62-
this(nodeStats.nodeId, nodeStats.cpuUtilizationPercent, nodeStats.memoryUtilizationPercent, nodeStats.timestamp);
61+
NodePerformanceStatistics(NodePerformanceStatistics nodePerformanceStatistics) {
62+
this(
63+
nodePerformanceStatistics.nodeId,
64+
nodePerformanceStatistics.cpuUtilizationPercent,
65+
nodePerformanceStatistics.memoryUtilizationPercent,
66+
nodePerformanceStatistics.timestamp
67+
);
6368
}
6469

6570
public double getMemoryUtilizationPercent() {

server/src/main/java/org/opensearch/node/NodeService.java

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,6 @@
5959
import org.opensearch.search.pipeline.SearchPipelineService;
6060
import org.opensearch.tasks.TaskCancellationMonitoringService;
6161
import org.opensearch.threadpool.ThreadPool;
62-
import org.opensearch.throttling.tracker.NodePerformanceTracker;
6362
import org.opensearch.transport.TransportService;
6463

6564
import java.io.Closeable;
@@ -117,7 +116,6 @@ public class NodeService implements Closeable {
117116
SearchPipelineService searchPipelineService,
118117
FileCache fileCache,
119118
TaskCancellationMonitoringService taskCancellationMonitoringService,
120-
NodePerformanceTracker nodePerformanceTracker,
121119
PerformanceCollectorService performanceCollectorService
122120
) {
123121
this.settings = settings;

server/src/main/java/org/opensearch/node/DownstreamNodesPerfStats.java renamed to server/src/main/java/org/opensearch/node/NodesPerformanceStats.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,14 +23,14 @@
2323
/**
2424
* This class represents collected performance stats of all downstream nodes and the local node
2525
*/
26-
public class DownstreamNodesPerfStats implements Writeable, ToXContentFragment {
26+
public class NodesPerformanceStats implements Writeable, ToXContentFragment {
2727
private final Map<String, NodePerformanceStatistics> nodePerfStats;
2828

29-
public DownstreamNodesPerfStats(Map<String, NodePerformanceStatistics> nodePerfStats) {
29+
public NodesPerformanceStats(Map<String, NodePerformanceStatistics> nodePerfStats) {
3030
this.nodePerfStats = nodePerfStats;
3131
}
3232

33-
public DownstreamNodesPerfStats(StreamInput in) throws IOException {
33+
public NodesPerformanceStats(StreamInput in) throws IOException {
3434
this.nodePerfStats = in.readMap(StreamInput::readString, NodePerformanceStatistics::new);
3535
}
3636

server/src/main/java/org/opensearch/node/PerformanceCollectorService.java

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ public void addNodePerfStatistics(String nodeId, double cpuUtilizationPercent, d
6161
*/
6262
public Map<String, NodePerformanceStatistics> getAllNodeStatistics() {
6363
Map<String, NodePerformanceStatistics> nodeStats = new HashMap<>(nodeIdToPerfStats.size());
64-
nodeIdToPerfStats.forEach((k, v) -> { nodeStats.put(k, new NodePerformanceStatistics(v)); });
64+
nodeIdToPerfStats.forEach((nodeId, nodePerfStats) -> { nodeStats.put(nodeId, new NodePerformanceStatistics(nodePerfStats)); });
6565
return nodeStats;
6666
}
6767

@@ -71,11 +71,11 @@ public Map<String, NodePerformanceStatistics> getAllNodeStatistics() {
7171
* {@code Optional} if the node was not found.
7272
*/
7373
public Optional<NodePerformanceStatistics> getNodeStatistics(final String nodeId) {
74-
return Optional.ofNullable(nodeIdToPerfStats.get(nodeId)).map(ns -> new NodePerformanceStatistics(ns));
74+
return Optional.ofNullable(nodeIdToPerfStats.get(nodeId)).map(perfStats -> new NodePerformanceStatistics(perfStats));
7575
}
7676

77-
public DownstreamNodesPerfStats stats() {
78-
return new DownstreamNodesPerfStats(getAllNodeStatistics());
77+
public NodesPerformanceStats stats() {
78+
return new NodesPerformanceStats(getAllNodeStatistics());
7979
}
8080

8181
}

server/src/main/java/org/opensearch/throttling/tracker/NodePerformanceTracker.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,7 @@ protected void doStart() {
110110
try {
111111
doRun();
112112
} catch (Exception e) {
113-
logger.debug("failure in search search backpressure", e);
113+
logger.debug("failure in node performance tracker : {}", e);
114114
}
115115
}, interval, ThreadPool.Names.GENERIC);
116116
cpuUsageTracker.doStart();

server/src/main/java/org/opensearch/throttling/tracker/PerformanceTrackerSettings.java

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -28,25 +28,25 @@ private static class Defaults {
2828
Setting.Property.NodeScope
2929
);
3030
public static final Setting<TimeValue> GLOBAL_CPU_USAGE_AC_POLLING_INTERVAL_SETTING = Setting.positiveTimeSetting(
31-
"node.global_cpu_usage.polling_interval",
31+
"node.perf_tracker.global_cpu_usage.polling_interval",
3232
TimeValue.timeValueMillis(Defaults.POLLING_INTERVAL),
3333
Setting.Property.NodeScope
3434
);
3535
public static final Setting<TimeValue> GLOBAL_CPU_USAGE_AC_WINDOW_DURATION_SETTING = Setting.positiveTimeSetting(
36-
"node.global_cpu_usage.window_duration",
36+
"node.perf_tracker.global_cpu_usage.window_duration",
3737
TimeValue.timeValueSeconds(Defaults.WINDOW_DURATION),
3838
Setting.Property.Dynamic,
3939
Setting.Property.NodeScope
4040
);
4141

4242
public static final Setting<TimeValue> GLOBAL_JVM_USAGE_AC_POLLING_INTERVAL_SETTING = Setting.positiveTimeSetting(
43-
"node.global_jvmmp.polling_interval",
43+
"node.perf_tracker.global_jvmmp.polling_interval",
4444
TimeValue.timeValueMillis(Defaults.POLLING_INTERVAL),
4545
Setting.Property.NodeScope
4646
);
4747

4848
public static final Setting<TimeValue> GLOBAL_JVM_USAGE_AC_WINDOW_DURATION_SETTING = Setting.positiveTimeSetting(
49-
"node.global_jvmmp.window_duration",
49+
"node.perf_tracker.global_jvmmp.window_duration",
5050
TimeValue.timeValueSeconds(Defaults.WINDOW_DURATION),
5151
Setting.Property.Dynamic,
5252
Setting.Property.NodeScope

server/src/test/java/org/opensearch/action/admin/cluster/node/stats/NodeStatsTests.java

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -58,8 +58,8 @@
5858
import org.opensearch.monitor.os.OsStats;
5959
import org.opensearch.monitor.process.ProcessStats;
6060
import org.opensearch.node.AdaptiveSelectionStats;
61-
import org.opensearch.node.DownstreamNodesPerfStats;
6261
import org.opensearch.node.NodePerformanceStatistics;
62+
import org.opensearch.node.NodesPerformanceStats;
6363
import org.opensearch.node.ResponseCollectorService;
6464
import org.opensearch.script.ScriptCacheStats;
6565
import org.opensearch.script.ScriptStats;
@@ -394,14 +394,14 @@ public void testSerialization() throws IOException {
394394
assertEquals(aStats.responseTime, bStats.responseTime, 0.01);
395395
});
396396
}
397-
DownstreamNodesPerfStats downstreamNodesPerfStats = nodeStats.getNodesPerformanceStats();
398-
DownstreamNodesPerfStats deserializedNodePerfStats = deserializedNodeStats.getNodesPerformanceStats();
399-
if (downstreamNodesPerfStats == null) {
397+
NodesPerformanceStats nodesPerformanceStats = nodeStats.getNodesPerformanceStats();
398+
NodesPerformanceStats deserializedNodePerfStats = deserializedNodeStats.getNodesPerformanceStats();
399+
if (nodesPerformanceStats == null) {
400400
assertNull(deserializedNodePerfStats);
401401
} else {
402-
downstreamNodesPerfStats.getNodePerfStats().forEach((k, v) -> {
403-
NodePerformanceStatistics aPerfStats = downstreamNodesPerfStats.getNodePerfStats().get(k);
404-
NodePerformanceStatistics bPerfStats = downstreamNodesPerfStats.getNodePerfStats().get(k);
402+
nodesPerformanceStats.getNodePerfStats().forEach((k, v) -> {
403+
NodePerformanceStatistics aPerfStats = nodesPerformanceStats.getNodePerfStats().get(k);
404+
NodePerformanceStatistics bPerfStats = nodesPerformanceStats.getNodePerfStats().get(k);
405405
assertEquals(aPerfStats.getMemoryUtilizationPercent(), bPerfStats.getMemoryUtilizationPercent(), 0.0);
406406
assertEquals(aPerfStats.getCpuUtilizationPercent(), bPerfStats.getCpuUtilizationPercent(), 0.0);
407407
assertEquals(aPerfStats.getTimestamp(), bPerfStats.getTimestamp());
@@ -769,7 +769,7 @@ public static NodeStats createNodeStats(boolean remoteStoreStats) {
769769
}
770770
adaptiveSelectionStats = new AdaptiveSelectionStats(nodeConnections, nodeStats);
771771
}
772-
DownstreamNodesPerfStats downstreamNodesPerfStats = null;
772+
NodesPerformanceStats nodesPerformanceStats = null;
773773
if (frequently()) {
774774
int numNodes = randomIntBetween(0, 10);
775775
Map<String, Long> nodeConnections = new HashMap<>();
@@ -791,7 +791,7 @@ public static NodeStats createNodeStats(boolean remoteStoreStats) {
791791
nodePerfStats.put(nodeId, stats);
792792
}
793793
}
794-
downstreamNodesPerfStats = new DownstreamNodesPerfStats(nodePerfStats);
794+
nodesPerformanceStats = new NodesPerformanceStats(nodePerfStats);
795795
}
796796
ClusterManagerThrottlingStats clusterManagerThrottlingStats = null;
797797
if (frequently()) {
@@ -824,7 +824,7 @@ public static NodeStats createNodeStats(boolean remoteStoreStats) {
824824
discoveryStats,
825825
ingestStats,
826826
adaptiveSelectionStats,
827-
downstreamNodesPerfStats,
827+
nodesPerformanceStats,
828828
scriptCacheStats,
829829
null,
830830
null,

0 commit comments

Comments
 (0)