Optimize TransportNodesAction to selectively avoid sending list of Di… by Pranshu-S · Pull Request #14531 · opensearch-project/OpenSearch

Pranshu-S · 2024-06-25T09:21:29Z

Description

In the current implementation, every transport action extending TransportNodesAction includes all discovery nodes in the transport request sent to each node in the cluster. This approach leads to performance bottlenecks in large clusters due to redundant data transmission. Specifically:

Increased Network Traffic: The same list of discovery nodes is written n^2 times (where n is the number of nodes), causing unnecessary network traffic and increased IO.
Write/Read Latency: The excessive data transmission contributes to higher overall latency for both write and read operations.
Netty Buffer Bottleneck: When using plugins like Netty for inter-node communication, the buffer becomes overloaded with redundant discovery node information, increasing the size of the request and correspondingly reducing the amount of requests which can fit in the Netty buffer.

This PR proposes a solution to address these bottlenecks by selectively skipping the resolution of concrete nodes at the transport layer. This optimization will significantly reduce redundant data transmission and improve overall performance in large clusters.

To do this, flag was introduced on the BaseNodeRequest ClasspopulateDiscoveryNodesInTransportRequest. Requests in this flag overriden to False will skip adding the concrete nodes data into the request. Currently it is only implemented in NodeStats, ClusterStats and NodeInfo Call from the rest layer.

The perf gains are significant (tested in a large 1k node domain)

API	Stats	Default (s)	Optimized (s)	% Reduction
_nodes/stats	Average	10.03409	1.41749	-86%
	P90	12.54658	5.80648	-54%
	Max	16.39159	11.61396	-30%
	Min	6.41173	0.35198	-95%
_nodes	Average	8.61528	3.66873	-58%
	P90	13.19499	8.30744	-38%
	Max	19.39874	15.91021	-18%
	Min	6.56645	1.17164	-83%
_cat/nodes	Average	21.45014	1.0651	-96%
	P90	26.95066	1.79513	-94%
	Max	36.11789	17.46319	-52%
	Min	15.26925	0.20656	-99%
_cluster/stats	Average	9.82926	2.01671	-80%
	P90	12.31955	6.52061	-48%
	Max	17.9214	12.50545	-31%
	Min	6.22867	0.27218	-96%

Related Issues

Resolves #[Issue number to be closed when this PR is merged]

Check List

Functionality includes testing.
API changes companion pull request created, if applicable.
Public documentation issue/PR created, if applicable.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

github-actions · 2024-06-25T09:52:20Z

❌ Gradle check result for 9e66ed2: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

vikasvb90 · 2024-07-01T14:08:02Z

server/src/main/java/org/opensearch/action/support/nodes/TransportNodesAction.java


        void start() {
-            final DiscoveryNode[] nodes = request.concreteNodes();
+            logger.info(request.getClass().getSimpleName());


This would lead to heavy logging. Please remove this.

Sure Vikas, I intended to work on the logs for testing. Will remove them prior to raising the PR.

vikasvb90 · 2024-07-01T14:14:27Z

server/src/main/java/org/opensearch/action/support/nodes/TransportNodesAction.java

            if (request.concreteNodes() == null) {
-                resolveRequest(request, clusterService.state());
-                assert request.concreteNodes() != null;
+                if (optimizedTransportActionNames.contains(request.getClass().getSimpleName())) {


Can you add some tests for this? Also, have you executed all existing tests and made sure that they are passing after your change. If yes, then please confirm this in the PR description.

Please make sure that node left and node added scenarios are covered properly.

Yes, I have modified the draft PR approach. Will make sure I add all the necessary tests covering different scenarios.

github-actions · 2024-07-11T14:38:28Z

❌ Gradle check result for 4f6b6ed: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

…s, NodesInfo and ClusterStats call Signed-off-by: Pranshu Shukla <pranshushukla06@gmail.com>

github-actions · 2024-07-12T13:47:29Z

❌ Gradle check result for d2ef37d: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

Signed-off-by: Pranshu Shukla <pranshushukla06@gmail.com>

github-actions · 2024-07-15T07:10:57Z

❌ Gradle check result for 469cd6a: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

Signed-off-by: Pranshu Shukla <pranshushukla06@gmail.com>

github-actions · 2024-07-15T09:24:32Z

❌ Gradle check result for 03c35df: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

Signed-off-by: Pranshu Shukla <pranshushukla06@gmail.com>

github-actions · 2024-07-15T11:19:36Z

❌ Gradle check result for b7b8754: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

Signed-off-by: Pranshu Shukla <pranshushukla06@gmail.com>

github-actions · 2024-07-15T15:22:52Z

❌ Gradle check result for 31f030c: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

Pranshu-S · 2024-07-15T15:46:18Z

Closing this PR in virtue of clone PR - #14749

vikasvb90 reviewed Jul 1, 2024

View reviewed changes

Pranshu-S force-pushed the TransportNodesActionOptimisation-DiscoveryNodeFix branch from 9e66ed2 to 4f6b6ed Compare July 11, 2024 14:27

Optimize TransportNodesAction to not send DiscoveryNodes for NodeStat…

d2ef37d

…s, NodesInfo and ClusterStats call Signed-off-by: Pranshu Shukla <pranshushukla06@gmail.com>

Pranshu-S force-pushed the TransportNodesActionOptimisation-DiscoveryNodeFix branch from 4f6b6ed to d2ef37d Compare July 12, 2024 13:18

Retry Build

469cd6a

Signed-off-by: Pranshu Shukla <pranshushukla06@gmail.com>

Retry Build

03c35df

Signed-off-by: Pranshu Shukla <pranshushukla06@gmail.com>

Retry Build

b7b8754

Signed-off-by: Pranshu Shukla <pranshushukla06@gmail.com>

Retry Build

31f030c

Signed-off-by: Pranshu Shukla <pranshushukla06@gmail.com>

Pranshu-S closed this Jul 15, 2024

opensearch-ci-bot mentioned this pull request Sep 9, 2024

[AUTOCUT] Gradle Check Flaky Test Report for RemoteSegmentTransferTrackerTests #14325

Closed

Conversation

Pranshu-S commented Jun 25, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Related Issues

Check List

Uh oh!

github-actions bot commented Jun 25, 2024

Uh oh!

vikasvb90 Jul 1, 2024

Choose a reason for hiding this comment

Uh oh!

Pranshu-S Jul 11, 2024

Choose a reason for hiding this comment

Uh oh!

vikasvb90 Jul 1, 2024

Choose a reason for hiding this comment

Uh oh!

vikasvb90 Jul 1, 2024

Choose a reason for hiding this comment

Uh oh!

Pranshu-S Jul 11, 2024

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Jul 11, 2024

Uh oh!

github-actions bot commented Jul 12, 2024

Uh oh!

github-actions bot commented Jul 15, 2024

Uh oh!

github-actions bot commented Jul 15, 2024

Uh oh!

github-actions bot commented Jul 15, 2024

Uh oh!

github-actions bot commented Jul 15, 2024

Uh oh!

Pranshu-S commented Jul 15, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Pranshu-S commented Jun 25, 2024 •

edited

Loading