Add repo throttle metrics to node stats api response #96678

volodk85 · 2023-06-07T22:40:45Z

What: Expose snapshot/restore throttling time in stats APIs
Change: Added repository section to _nodes/stats API response. It shows throttling stats for registered repositories per node.

Sample:
Setup: 3-nodes cluster, FS repository, index 1000 documents 8Kbytes each, take a snapshot, observe throttling stats
Request: http://localhost:9200/_nodes/stats/repository?pretty
Response (truncated to one node):

{ "_nodes" : { "total" : 3, "successful" : 3, "failed" : 0 }, "cluster_name" : "stateless", "nodes" : { "nGCCGizzRoSaUNeB5VTdtQ" : { "timestamp" : 1686171059819, "name" : "es01", "transport_address" : "172.18.0.2:9300", "host" : "172.18.0.2", "ip" : "172.18.0.2:9300", "roles" : [ "index", "master" ], "attributes" : { "stateless.memory" : "8232894464", "xpack.installed" : "true" }, "repository" : { "my_repository" : { "total_read_throttled_nanos" : 0, "total_write_throttled_nanos" : 178347835 } } },

Closes #89385

github-actions · 2023-06-07T22:41:00Z

Documentation preview:

✨ Changed pages

elasticsearchmachine · 2023-06-07T22:41:09Z

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

elasticsearchmachine · 2023-06-07T22:41:32Z

Hi @volodk85, I've created a changelog YAML for you.

fcofdez

This looks mostly good. We need to handle the mixed-version serialization logic and I would prefer to have an integration test where we a repository is configured with a low rate-limit and we see that the added stats reflect the throttling.

fcofdez · 2023-06-08T08:06:09Z

server/src/main/java/org/elasticsearch/repositories/RepositoriesThrottlingStats.java

+
+import static java.util.stream.Collectors.toMap;
+
+public class RepositoriesThrottlingStats implements Writeable, ToXContentFragment {


Maybe we can be a bit more general here and name this class RepositoriesStats?

fcofdez · 2023-06-08T08:09:03Z

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

 ingestStats = in.readOptionalWriteable(IngestStats::read);
 adaptiveSelectionStats = in.readOptionalWriteable(AdaptiveSelectionStats::new);
 indexingPressureStats = in.readOptionalWriteable(IndexingPressureStats::new);
+ repositoriesThrottlingStats = in.readOptionalWriteable(RepositoriesThrottlingStats::new);


we should handle the case where the node is communicating with a node in a different version. See the javadocs in org.elasticsearch.TransportVersion for more information.

fcofdez · 2023-06-08T08:10:34Z

server/src/main/java/org/elasticsearch/repositories/RepositoriesThrottlingStats.java

+ return Collections.unmodifiableMap(repositoryToThrottlingStats);
+ }
+
+ public record RepositoryThrottling(String repositoryName, long totalReadThrottledNanos, long totalWriteThrottledNanos) {}


can't we use this class instead of ThrottlingStats? and as I stated above maybe we should name this RepositoryStats?

fcofdez

Thanks for tackling the review comments! This looks mostly good but we still need to handle the mixed-cluster serialization in the NodeStats class and add a new transport version for this change.

fcofdez · 2023-06-09T09:44:22Z

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

 ingestStats = in.readOptionalWriteable(IngestStats::read);
 adaptiveSelectionStats = in.readOptionalWriteable(AdaptiveSelectionStats::new);
 indexingPressureStats = in.readOptionalWriteable(IndexingPressureStats::new);
+ repositoriesStats = in.readOptionalWriteable(RepositoriesStats::new);


We should handle the mixed-node serialization here too. That's the reason why the bwc tests are failing.

fcofdez · 2023-06-09T09:46:38Z

server/src/main/java/org/elasticsearch/repositories/RepositoriesStats.java

+ private final Map<String, ThrottlingStats> repositoryThrottlingStats;
+
+ public RepositoriesStats(StreamInput in) throws IOException {
+ if (in.getTransportVersion().onOrAfter(TransportVersion.V_8_9_0)) {


We should add a new transport version, this changed recently to handle a new scenario where versions are deployed more often in serverless. I can share some documentation about this.

DaveCTurner

Looks great. I left a few comments & questions, all superficial.

docs/reference/cluster/nodes-stats.asciidoc

DaveCTurner · 2023-06-10T07:15:59Z

server/src/main/java/org/elasticsearch/repositories/RepositoriesStats.java

+ builder.field("total_read_throttled_nanos", totalReadThrottledNanos);
+ builder.field("total_write_throttled_nanos", totalWriteThrottledNanos);


Could we also have these fields represented in a human-readable fashion if the ?human flag is specified? IIRC that's a little awkward with nanoseconds, because if you use org.elasticsearch.xcontent.XContentBuilder#humanReadableField with a TimeValue then you get the raw output in milliseconds (because of this) but still we can just check the human flag directly (like we do e.g. here).

Those values are just long counters (not wrapped in TimeValue) and I think those transformers won't affect them, right?

That's correct.

I think this is the only remaining point now.

Following ...

http://localhost:9200/_nodes/rVt20tYyS3yN5LNYHjPoSw/stats/repository?pretty

{ "_nodes" : { "total" : 1, "successful" : 1, "failed" : 0 }, "cluster_name" : "stateless", "nodes" : { "rVt20tYyS3yN5LNYHjPoSw" : { "timestamp" : 1686604771491, "name" : "es01", "transport_address" : "172.18.0.4:9300", "host" : "172.18.0.4", "ip" : "172.18.0.4:9300", "roles" : [ "index", "master" ], "attributes" : { "stateless.memory" : "8232894464", "xpack.installed" : "true" }, "repositories" : { "my_repository" : { "total_read_throttled_time_nanos" : 0, "total_write_throttled_time_nanos" : 551110710 } } } } }

http://localhost:9200/_nodes/rVt20tYyS3yN5LNYHjPoSw/stats/repository?pretty&human

{ "_nodes" : { "total" : 1, "successful" : 1, "failed" : 0 }, "cluster_name" : "stateless", "nodes" : { "rVt20tYyS3yN5LNYHjPoSw" : { "timestamp" : 1686604850299, "name" : "es01", "transport_address" : "172.18.0.4:9300", "host" : "172.18.0.4", "ip" : "172.18.0.4:9300", "roles" : [ "index", "master" ], "attributes" : { "stateless.memory" : "8232894464", "xpack.installed" : "true" }, "repositories" : { "my_repository" : { "total_read_throttled_time" : "0s", "total_write_throttled_time" : "551.1ms" } } } } }

Note that field names are different for human and non-human mode. asciidoc was updated.

I think this needs to follow the same pattern/convention as in AdaptiveSelectionStats: we always output the machine-readable field, plus the human-readable field if requested.

server/src/main/java/org/elasticsearch/repositories/RepositoriesStats.java

server/src/main/java/org/elasticsearch/repositories/RepositoriesService.java

fcofdez

LGTM. Thanks for all the iterations 👍. I agree with David's comments, but once these are tackled this is good to get merged.

DaveCTurner

I'd like to give this another review before a final LGTM so marking it as request changes for now.

DaveCTurner

LGTM

volodk85 · 2023-06-13T13:36:43Z

Thank you @DaveCTurner @fcofdez for reviewing this

Add repo throttle metrics to node stats api response

1b90906

volodk85 added >feature :Distributed Coordination/Snapshot/Restore Anything directly related to the `_snapshot/*` APIs labels Jun 7, 2023

volodk85 requested review from DaveCTurner and fcofdez June 7, 2023 22:40

elasticsearchmachine added v8.9.0 Team:Distributed (Obsolete) Meta label for distributed team (obsolete). Replaced by Distributed Indexing/Coordination. labels Jun 7, 2023

volodk85 and others added 3 commits June 7, 2023 15:41

Update docs/changelog/96678.yaml

8af1dc9

Change x-content output structure

3a9406b

Merge branch 'main' into issues/89385-throttle-stats

26021f9

fcofdez requested changes Jun 8, 2023

View reviewed changes

volodk85 added 5 commits June 8, 2023 09:45

Merge branch 'main' into issues/89385-throttle-stats

79435f0

Fix test after merge from main

9d47417

Follow PR comments

8a688ee

minor fixes

38d1529

minor fixes 2

7e4b194

volodk85 requested a review from fcofdez June 8, 2023 23:33

fcofdez requested changes Jun 9, 2023

View reviewed changes

volodk85 added 3 commits June 9, 2023 10:50

Merge branch 'main' into issues/89385-throttle-stats

2ae516f

Introduce new TransportVersion (V_8_500_010)

a1e97a4

Fix yaml test

a146bb3

volodk85 requested a review from fcofdez June 9, 2023 22:13

DaveCTurner reviewed Jun 10, 2023

View reviewed changes

fcofdez approved these changes Jun 12, 2023

View reviewed changes

This comment was marked as off-topic.

Sign in to view

DaveCTurner requested changes Jun 12, 2023

View reviewed changes

Merge branch 'main' into issues/89385-throttle-stats

b112445

Follow PR comments

76e6cd5

volodk85 requested review from DaveCTurner and fcofdez June 12, 2023 19:29

volodk85 added 3 commits June 12, 2023 14:09

Make stats datapoints human readable

a039d09

Merge branch 'main' into issues/89385-throttle-stats

bcc564e

Follow common pattern for human readable output

06ae3c8

DaveCTurner approved these changes Jun 13, 2023

View reviewed changes

volodk85 added 2 commits June 13, 2023 06:42

Merge branch 'main' into issues/89385-throttle-stats

e772bce

Bump up TransportVersion

6e48de8

volodk85 merged commit 7abe8cb into elastic:main Jun 13, 2023

volodk85 deleted the issues/89385-throttle-stats branch June 13, 2023 16:04

volodk85 mentioned this pull request Jun 13, 2023

Fix Node stats API metric name #96815

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Add repo throttle metrics to node stats api response #96678

Add repo throttle metrics to node stats api response #96678

Uh oh!

volodk85 commented Jun 7, 2023 •

edited

Loading

github-actions bot commented Jun 7, 2023

elasticsearchmachine commented Jun 7, 2023

elasticsearchmachine commented Jun 7, 2023

fcofdez left a comment

fcofdez Jun 8, 2023

fcofdez Jun 8, 2023

volodk85 Jun 8, 2023

fcofdez Jun 8, 2023

volodk85 Jun 8, 2023

fcofdez left a comment

fcofdez Jun 9, 2023

fcofdez Jun 9, 2023

DaveCTurner left a comment

Uh oh!

Uh oh!

Uh oh!

DaveCTurner Jun 10, 2023

volodk85 Jun 12, 2023

DaveCTurner Jun 12, 2023

DaveCTurner Jun 12, 2023

volodk85 Jun 12, 2023

DaveCTurner Jun 13, 2023

Uh oh!

Uh oh!

Uh oh!

fcofdez left a comment

This comment was marked as off-topic.

DaveCTurner left a comment

DaveCTurner left a comment

volodk85 commented Jun 13, 2023

Labels

5 participants


		import static java.util.stream.Collectors.toMap;

		public class RepositoriesThrottlingStats implements Writeable, ToXContentFragment {

		builder.field("total_read_throttled_nanos", totalReadThrottledNanos);
		builder.field("total_write_throttled_nanos", totalWriteThrottledNanos);

Add repo throttle metrics to node stats api response #96678

Add repo throttle metrics to node stats api response #96678

Uh oh!

Conversation

volodk85 commented Jun 7, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

github-actions bot commented Jun 7, 2023

elasticsearchmachine commented Jun 7, 2023

elasticsearchmachine commented Jun 7, 2023

fcofdez left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fcofdez left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DaveCTurner left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

fcofdez left a comment

Choose a reason for hiding this comment

This comment was marked as off-topic.

DaveCTurner left a comment

Choose a reason for hiding this comment

DaveCTurner left a comment

Choose a reason for hiding this comment

volodk85 commented Jun 13, 2023

Labels

5 participants

volodk85 commented Jun 7, 2023 •

edited

Loading