Improve performance of Cat Nodes API

[NEUpanning]

Description

We found that executing the Cat Nodes API (query parameters do not matter) on the coordinate node of a large cluster can require a huge amount of CPU.This could have a significant impact on cluster stability.I reproduced this problem in the cluster with 200 data nodes and 140k shards.
When I used the 'top' command to query, the result showed that CPU usage fluctuated between 726% and 1173% for 3 seconds.
The most CPU usage comes from ShardStats.<init> and DiscoveryNode.writeTo. ShardStats.<init> is called when coordinate node deserializes response that is responded by other nodes. DiscoveryNode.writeTo is called when coordinate node serializes request that will be sent to other nodes.Here is Flame Graph

Several superficial ideas try to solve this issue:

1. Coordinate node could construct NodesStatsRequest#indices based on query parameters to filter indices stats rather than calling NodesStatsRequest.indices(true) that contains all indices stats.For instance if users call _cat/nodes?h=m,coordinate node should not fetch indices stats from other nodes.This would avoid a lot of unnecessary deserialization of the response content from ShardStats.<init>.
2. Set NodeInfoRequest#concreteNodes to null after using NodeInfoRequest#concreteNodes to build iterator that used to send requests.This would avoid unnecessary serialization of the request content from DiscoveryNode.writeTo.

[elasticsearchmachine]

Pinging @elastic/es-distributed (Team:Distributed)

[elasticsearchmachine]

Pinging @elastic/es-data-management (Team:Data Management)

[DaveCTurner]

The _cat APIs are intended to be convenient wrappers around the more flexible JSON APIs, intended for occasional use by humans. I'm a little impressed that it only takes 3s to respond in such a huge cluster. If you need more control over the execution, use the JSON APIs directly.

For instance if users call _cat/nodes?h=m, coordinate node should not fetch indices stats from other nodes. This would avoid a lot of unnecessary deserialization of the response content from ShardStats..

This seems like a nice idea but it would be fragile, we'd need to keep track of the columns that included stats to know whether or not the stats request was needed, and make sure to keep that list up to date as the columns change over time. Also the _cat APIs generally don't have a way to get a list of the requested columns when processing the request. So this idea seems possible but a little tricky and requiring quite some effort.

Set NodeInfoRequest#concreteNodes to null

IMO this is a valid point, although I would not want to solve it as described. Today every node-level TransportNodesInfoAction.NodeInfoRequest carries the entire top-level NodesInfoRequest, but it only needs to carry the metrics. We should trim these things down for sure.

[DaveCTurner]

Just to add: another idea would be to indicate in the stats request that we don't care about stats for individual shards, we are only going to use a summary. That'd save a bunch of effort and network traffic with the nodes stats and indices stats APIs too.

[NEUpanning]

@DaveCTurner Thanks for the reply.

In the most scenarios we will use the JSON APIs instead of _cat/nodes.

If you need more control over the execution, use the JSON APIs directly.

This is a great idea that is more elegant than i think and avoids unnecessary serialization of the request content from DiscoveryNode.writeTo.

Today every node-level TransportNodesInfoAction.NodeInfoRequest carries the entire top-level NodesInfoRequest, but it only needs to carry the metrics. We should trim these things down for sure.

This is also a great idea that would trim useless shard stats from responses and avoid unnecessary deserialization of ShardStats.<init>.

another idea would be to indicate in the stats request that we don't care about stats for individual shards, we are only going to use a summary.

I think implementaion of these ideas could solve this issue.

[NEUpanning]

I don't quite understand why indices stats APIs can be optimized.I see coordinate node needs shard stats to build indices stats.

That'd save a bunch of effort and network traffic with the nodes stats and indices stats APIs too.

[NEUpanning]

I would like to do it, do you think I could give it a try?

I think implementaion of these ideas could solve this issue.

[DaveCTurner]

I see coordinate node needs shard stats to build indices stats.

I think we can reduce the work here if the user specifies ?level=cluster, but I'm not sure it's worth the effort.

I would like to do it, do you think I could give it a try?

Sure, go for it. I recommend you don't do it all in one PR tho, try and separate the independent changes out to make them easier to review.

[NEUpanning]

@DaveCTurner I've opened a pull request(#99938) for this idea . Could you please have a look when you have some time? Thanks

Today every node-level TransportNodesInfoAction.NodeInfoRequest carries the entire top-level NodesInfoRequest, but it only needs to carry the metrics.

[NEUpanning]

another idea would be to indicate in the stats request that we don't care about stats for individual shards, we are only going to use a summary.

I've opened a pull request for this idea in #100466.

[DaveCTurner]

Thanks @NEUpanning, I'll take a look next week. You might also be interested in #90631 which is kind of the same thing but for the GET _cluster/health API.

[NEUpanning]

I would like to resolve this issue. After that PR is merged, I will try it using the similar approach.

[NEUpanning]

After we have implemented these ideas mentioned above, the CPU usage and cost time of fetching nodes stats (without shards-level stats) reduce to 1/1000th of their original levels in the cluster with 200 data nodes and 140k shards. So this issue is closed as completed.

[DaveCTurner]

Nice work @NEUpanning, thanks for the report, the fixes, and for confirming that the problems are fixed.

[NEUpanning]

Thanks again David. Thanks for your help and patience in code review.