Add `thread_pool` information to the cluster info endpoint #96407

HiDAl · 2023-05-29T10:13:59Z

Add a new target (thread_pool) to the /_info API. It consolidates all the thread pools information from the cluster nodes and returns a summary at the cluster level (compared with _nodes/stats/thread_pool it lacks the <node> dimension)

GET _info/thread_pool { "cluster_name": "runTask", "thread_pool": { "analyze": { "threads": 0, "queue": 0, "active": 0, "rejected": 0, "largest": 0, "completed": 0 }, "auto_complete": { "threads": 0, "queue": 0, "active": 0, "rejected": 0, "largest": 0, "completed": 0 }, "azure_event_loop": { "threads": 0, "queue": 0, "active": 0, "rejected": 0, "largest": 0, "completed": 0 }, "ccr": { "threads": 0, "queue": 0, "active": 0, "rejected": 0, "largest": 0, "completed": 0 }, "cluster_coordination": { "threads": 1, "queue": 0, "active": 0, "rejected": 0, "largest": 1, "completed": 104 }, "fetch_shard_started": { "threads": 0, "queue": 0, "active": 0, "rejected": 0, "largest": 0, "completed": 0 }, "fetch_shard_store": { "threads": 0, "queue": 0, "active": 0, "rejected": 0, "largest": 0, "completed": 0 }, "flush": { "threads": 0, "queue": 0, "active": 0, "rejected": 0, "largest": 0, "completed": 0 }, "force_merge": { "threads": 0, "queue": 0, "active": 0, "rejected": 0, "largest": 0, "completed": 0 }, "generic": { "threads": 16, "queue": 0, "active": 0, "rejected": 0, "largest": 16, "completed": 226 }, "get": { "threads": 0, "queue": 0, "active": 0, "rejected": 0, "largest": 0, "completed": 0 }, "management": { "threads": 2, "queue": 0, "active": 1, "rejected": 0, "largest": 2, "completed": 18 }, "profiling": { "threads": 0, "queue": 0, "active": 0, "rejected": 0, "largest": 0, "completed": 0 }, "refresh": { "threads": 0, "queue": 0, "active": 0, "rejected": 0, "largest": 0, "completed": 0 }, "repository_azure": { "threads": 0, "queue": 0, "active": 0, "rejected": 0, "largest": 0, "completed": 0 }, "rollup_indexing": { "threads": 0, "queue": 0, "active": 0, "rejected": 0, "largest": 0, "completed": 0 }, "search": { "threads": 0, "queue": 0, "active": 0, "rejected": 0, "largest": 0, "completed": 0 }, "search_coordination": { "threads": 0, "queue": 0, "active": 0, "rejected": 0, "largest": 0, "completed": 0 }, "search_throttled": { "threads": 0, "queue": 0, "active": 0, "rejected": 0, "largest": 0, "completed": 0 }, "searchable_snapshots_cache_fetch_async": { "threads": 0, "queue": 0, "active": 0, "rejected": 0, "largest": 0, "completed": 0 }, "searchable_snapshots_cache_prewarming": { "threads": 0, "queue": 0, "active": 0, "rejected": 0, "largest": 0, "completed": 0 }, "snapshot": { "threads": 0, "queue": 0, "active": 0, "rejected": 0, "largest": 0, "completed": 0 }, "snapshot_meta": { "threads": 0, "queue": 0, "active": 0, "rejected": 0, "largest": 0, "completed": 0 }, "system_critical_read": { "threads": 0, "queue": 0, "active": 0, "rejected": 0, "largest": 0, "completed": 0 }, "system_critical_write": { "threads": 0, "queue": 0, "active": 0, "rejected": 0, "largest": 0, "completed": 0 }, "system_read": { "threads": 0, "queue": 0, "active": 0, "rejected": 0, "largest": 0, "completed": 0 }, "system_write": { "threads": 0, "queue": 0, "active": 0, "rejected": 0, "largest": 0, "completed": 0 }, "vector_tile_generation": { "threads": 0, "queue": 0, "active": 0, "rejected": 0, "largest": 0, "completed": 0 }, "warmer": { "threads": 0, "queue": 0, "active": 0, "rejected": 0, "largest": 0, "completed": 0 }, "watcher": { "threads": 0, "queue": 0, "active": 0, "rejected": 0, "largest": 0, "completed": 0 }, "write": { "threads": 0, "queue": 0, "active": 0, "rejected": 0, "largest": 0, "completed": 0 } } } $ curl localhost:9200/_info/_all | jq 'keys' [ "cluster_name", "http", "ingest", "thread_pool" ]

Closes #95393

ensure the constructor is tested makes clearer when checking list order

github-actions · 2023-05-29T10:14:10Z

Documentation preview:

✨ Changed pages

elasticsearchmachine · 2023-05-29T10:14:23Z

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

elasticsearchmachine · 2023-05-29T10:14:23Z

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

HiDAl · 2023-05-29T14:38:50Z

@elasticmachine run elasticsearch-ci/part-1

HiDAl · 2023-05-30T10:48:27Z

@elasticmachine run elasticsearch-ci/packaging-tests-windows-sample

gmarouli

Thank you @HiDAl this is looking great already! I started a discussion on some small topics :).

gmarouli · 2023-05-30T12:51:04Z

server/src/test/java/org/elasticsearch/threadpool/ThreadPoolStatsTests.java

+ ThreadPoolStats.Stats.merge(firstStats.get(1), secondStats.get(2)), // name-2
+ ThreadPoolStats.Stats.merge(firstStats.get(2), secondStats.get(3)), // name-3


I would prefer here to not use the methods we are testing, I would be ok with writing a helper function for this but in the test. The idea behind this is, if a bug is introduced accidentally in ThreadPoolStats.Stats.merge we will catch it easier.

What do you think?

yeah, I thought the same while writing this. That's the reason why I wrote a separate test for this method at testStatsMerge() (a couple of lines below). IMO, that would give us enough confidence in case a bug is introduced to the other method it'll be caught. WDYT?

True, I do have a preference to have it not calling the merge, in case the other test is removed or changed somehow, but I understand if you want to leave it as is.

gmarouli · 2023-05-31T07:10:16Z

docs/reference/cluster/cluster-info.asciidoc

 Ingest information.
+
+`thread_pool`::
+Statistics about each thread pool, including current size, queue and rejected tasks.


I think we should say queue size instead of queue, what do you think?

gmarouli · 2023-05-31T07:11:06Z

docs/reference/cluster/cluster-info.asciidoc

+[[cluster-info-api-response-body-threadpool]]
+`thread_pool`::
+(object)
+Contains thread pool information of the cluster.


Maybe Contains information about the thread pools of the cluster reads a bit better? What do you think?

gmarouli · 2023-05-31T07:12:48Z

docs/reference/cluster/cluster-info.asciidoc

+======
+`<thread_pool_name>`::
+(object)
+Contains information about the thread pool of the cluster.


What about something like this:

Contains information about the thread pool of the cluster with name <thread_pool_name>.

gmarouli · 2023-05-31T07:19:42Z

server/src/main/java/org/elasticsearch/rest/action/info/RestClusterInfoAction.java

+ .reduce(IngestStats.IDENTITY, IngestStats::merge),
+ //
+ THREAD_POOL.metricName(),
+ response -> response.getNodes().stream().map(NodeStats::getThreadPool).reduce(ThreadPoolStats.IDENTITY, ThreadPoolStats::merge)


Nit, if I am not mistaken this response is the same as the nodesStatsResponse in the other metrics. I would prefer to have it consistent over all the metrics to show that they have the same input.

gmarouli · 2023-05-31T07:44:51Z

server/src/test/java/org/elasticsearch/threadpool/ThreadPoolStatsTests.java

+ assertThat(copy.stream().map(ThreadPoolStats.Stats::name).toList(), contains("a", "d", "m", "m", "m", "t", "z"));
+ assertThat(copy.stream().map(ThreadPoolStats.Stats::threads).toList(), contains(-1, 2, -3, 4, 5, 6, 7));


What exactly are we testing with these two lines? It's not very clear to me. I thought the next two lines would be sufficient.

These are the old tests: basically, checking that the method compareTo is correct. anyway, you're right, the next 2 lines should suffice.

Ah, I didn't think of that :), thanks for the explanation.

gmarouli · 2023-05-31T07:48:57Z

server/src/main/java/org/elasticsearch/threadpool/ThreadPoolStats.java

 public ThreadPoolStats {
- Collections.sort(stats);
- stats = Collections.unmodifiableList(stats);
+ var statsCopy = new ArrayList<>(stats);
+ Collections.sort(statsCopy);
+ stats = Collections.unmodifiableList(statsCopy);
 }


Nit: Should we add another constructor that accepts a Collection to avoid having to do new ThreadPoolStats(new ArrayList<>(mergedThreadPools.values())) in the merge method?

sadly no, it clashes because we're using a List<Stat> (List is a Collection)...

What about changing the constructor to ThreadPoolStats(Collection<Stat> stats)?

ArrayList constructor is also accepting Collection:

public ArrayList(Collection<? extends E> c) {...}

good idea, will change it. Thanks Mary

…sttas

gmarouli

LGTM, I added two comments that continue our previous discussion.

Pablo Alcantar Morales added 5 commits May 29, 2023 11:19

wire up the new ThreadPoolStats endpoint

85adbab

add tests for merge

b20018c

ensure the constructor is tested makes clearer when checking list order

add YAML tests

4d4144d

add documentation

4fecbb3

mark _info/<target> endpoint as public

74ff6de

HiDAl added >enhancement :Data Management/Stats Statistics tracking and retrieval APIs Team:Data Management Meta label for data/management team v8.9.0 labels May 29, 2023

HiDAl requested a review from gmarouli May 29, 2023 10:13

HiDAl and others added 3 commits May 29, 2023 12:14

Update docs/changelog/96407.yaml

71bc302

spotless as usual

988de0d

forgot to change this value

88322d3

Pablo Alcantar Morales added 3 commits May 30, 2023 11:49

fix how the stats are merged

94d5446

tests stats merges

c3f5def

beauty the code a bit

0bd8661

gmarouli reviewed May 31, 2023

View reviewed changes

Pablo Alcantar Morales added 3 commits May 31, 2023 15:41

Merge remote-tracking branch 'elastic/main' into cluster-thread-pool-…

6259743

…sttas

reword

1e45b8a

tackle PR comments

45f9af1

HiDAl requested a review from gmarouli May 31, 2023 13:54

gmarouli approved these changes May 31, 2023

View reviewed changes

convert ThreadPoolStats.stats to a Collection

e85cd52

HiDAl merged commit f5d47cc into elastic:main Jun 1, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Add `thread_pool` information to the cluster info endpoint #96407

Add `thread_pool` information to the cluster info endpoint #96407

Uh oh!

HiDAl commented May 29, 2023

github-actions bot commented May 29, 2023

elasticsearchmachine commented May 29, 2023

elasticsearchmachine commented May 29, 2023

HiDAl commented May 29, 2023

HiDAl commented May 30, 2023

gmarouli left a comment

gmarouli May 30, 2023

HiDAl May 31, 2023 •

edited

Loading

gmarouli May 31, 2023

gmarouli May 31, 2023

gmarouli May 31, 2023

gmarouli May 31, 2023

gmarouli May 31, 2023

gmarouli May 31, 2023

HiDAl May 31, 2023

gmarouli May 31, 2023

gmarouli May 31, 2023

HiDAl May 31, 2023

gmarouli May 31, 2023 •

edited

Loading

HiDAl Jun 1, 2023

gmarouli left a comment

Labels

3 participants

		ThreadPoolStats.Stats.merge(firstStats.get(1), secondStats.get(2)), // name-2
		ThreadPoolStats.Stats.merge(firstStats.get(2), secondStats.get(3)), // name-3

		assertThat(copy.stream().map(ThreadPoolStats.Stats::name).toList(), contains("a", "d", "m", "m", "m", "t", "z"));
		assertThat(copy.stream().map(ThreadPoolStats.Stats::threads).toList(), contains(-1, 2, -3, 4, 5, 6, 7));

Add thread_pool information to the cluster info endpoint #96407

Add thread_pool information to the cluster info endpoint #96407

Uh oh!

Conversation

HiDAl commented May 29, 2023

github-actions bot commented May 29, 2023

elasticsearchmachine commented May 29, 2023

elasticsearchmachine commented May 29, 2023

HiDAl commented May 29, 2023

HiDAl commented May 30, 2023

gmarouli left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

HiDAl May 31, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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

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

gmarouli May 31, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gmarouli left a comment

Choose a reason for hiding this comment

Labels

3 participants

Add `thread_pool` information to the cluster info endpoint #96407

Add `thread_pool` information to the cluster info endpoint #96407

HiDAl May 31, 2023 •

edited

Loading

gmarouli May 31, 2023 •

edited

Loading