Add utilization to the THREAD_POOL node stats#132156
Add utilization to the THREAD_POOL node stats#132156nicktindall wants to merge 5 commits intoelastic:mainfrom
Conversation
|
|
||
| public float getUtilization() { | ||
| return (float) apmUtilizationTracker.getLastValue(); | ||
| } |
There was a problem hiding this comment.
Just a stop-gap until we have the singular utilisation value
| sumStat(firstStats.largest, secondStats.largest), | ||
| sumStat(firstStats.completed, secondStats.completed) | ||
| sumStat(firstStats.completed, secondStats.completed), | ||
| NaN // Don't sum utilization, it makes no sense |
There was a problem hiding this comment.
This is used by the get cluster info REST api to merge the stats for each thread pool received from each node. I don't think it makes sense to merge utilization here, and in any case we don't render it in toXContent. Perhaps if/when we start rendering it we can implement a sensible merge?
There was a problem hiding this comment.
I think this is the indication that we don't really want to do this here. Although this might seem simpler than #131480 in terms of lines-of-code added, IMO that's because this is a bit of a short-cut. Fields in the transport messages which aren't represented in the REST API are tech debt.
I'd rather we moved towards having the allocator use dedicated messages for all these things rather than relying on stats APIs - the stats APIs we use today return a bunch of stuff about which we don't care.
There was a problem hiding this comment.
We have a few like this now for features that aren't settled. Not because it was too hard, but rather to allow the freedom to change them before we commit to something that would be considered a breaking change.
Point taken about moving to dedicated messages though, this approach was an attempt to remain consistent with existing code, but it sounds like we don't want to do that.
nicktindall
added
>non-issue
:Distributed/Allocation
nicktindall
changed the title
|
Pinging @elastic/es-distributed-coordination (Team:Distributed Coordination) |
elasticsearchmachine
added
the
Team:Distributed Coordination (obsolete)
# Conflicts: # server/src/main/java/org/elasticsearch/TransportVersions.java
| sumStat(firstStats.largest, secondStats.largest), | ||
| sumStat(firstStats.completed, secondStats.completed) | ||
| sumStat(firstStats.completed, secondStats.completed), | ||
| NaN // Don't sum utilization, it makes no sense |
There was a problem hiding this comment.
I think this is the indication that we don't really want to do this here. Although this might seem simpler than #131480 in terms of lines-of-code added, IMO that's because this is a bit of a short-cut. Fields in the transport messages which aren't represented in the REST API are tech debt.
I'd rather we moved towards having the allocator use dedicated messages for all these things rather than relying on stats APIs - the stats APIs we use today return a bunch of stuff about which we don't care.
4 participants
Amend the thread pool node stats to include a float value for utilization.
Currently I've configured it to return the last value calculated for APM, because there is work underway to provide a singular utilization value for thread pools (see #131898). If that work merges before this I'll use that instead.
Relates ES-12316