Add busy threads stat #3517
Add busy threads stat #3517
Conversation
be96f05 to
1eda2aa
Compare
1eda2aa to
2f37c84
Compare
There was a problem hiding this comment.
I like the idea. I didn't actually realize running wasn't this metric until you mentioned it in slack chat.
I've made a perf change request. I also ask that you update the doc/stats.md to document this new feature.
I made a comment about a locking call to the thread pool. I would like to think more on it or hear more thoughts on it before figuring out how I think we should move. Basically: What do we think our guarantees are or should be for these metrics regarding threadsafety and race conditions?
|
okay i took a stab at at documentation, take a look. please note that I have now written most of this document, starting in #2525 when i created it. this is, frankly, ridiculous 😅 - someone with a better understanding of this stuff should spruce is up and fill in the |
# running
attr_reader :spawned
# backlog
with_mutex { @todo.size }
# pool_capacity
waiting + (@max - spawned)
# busy_threads
with_mutex { @spawned - @waiting + @todo.size }
# max thread: doesn't need mutex
# request count: theoretically can also be incorrect number in the stats snapshot
# but i'm guessing we are okay with that, because it doesn't have an interesting relationship
# to the other stats |
|
i propose that improvements to stat locking should go into another PR, since status quo is already inconsistent locking for stats |
Not sure about that, don't we want stats we can trust? |
I’m saying, this PR doesn’t introduce new phenomenon or behavior - the stats in master already don’t use the same mutex-controlled timeslice. putting all stats in the same mutex could be done in a PR before or after this one (I could put it together if interested). |
|
@dentarg WDYT? |
|
I'm assuming that #3527 addresses @schneems issue with multiple locking calls. My question to you is if #3527 and #3528 are merged/accepted, are you ok with rebasing and adjusting this to work with the updates? Or, we could flip the order and merge this first. Not sure which is easier 'collectively'... |
|
definitely discuss and merge the other PRs first |
|
@schneems You ok with this as is? |
Expose a prometheus metric new thread stats from Puma - puma/puma#3517
Expose a prometheus metric new thread stats from Puma - puma/puma#3517
Expose a prometheus metric new thread stats from Puma - puma/puma#3517
New Puma version has a [nice addition](puma/puma#3517) of metric on busy threads. > busy_threads: running - how many threads are waiting to receive work + how many requests are waiting for a thread to pick them up. this is a "wholistic" stat reflecting the overall current state of work to be done and the capacity to do it. [source](https://github.com/puma/puma/blob/master/docs/stats.md#single-mode-and-individual-workers-in-cluster-mode)
This new stat was [added about 6 months ago](puma/puma#3517) to track "the overall current state of work to be done and the capacity to do it". It seems like it could be useful to track, potentially for autoscaling or other applications.
This new stat was [added about 6 months ago](puma/puma#3517) to track "the overall current state of work to be done and the capacity to do it". It seems like it could be useful to track, potentially for autoscaling or other applications.
This new stat was [added about 6 months ago](puma/puma#3517) to track "the overall current state of work to be done and the capacity to do it". It seems like it could be useful to track, potentially for autoscaling or other applications.
This new stat was [added about 6 months ago](puma/puma#3517) to track "the overall current state of work to be done and the capacity to do it". It seems like it could be useful to track, potentially for autoscaling or other applications.
* Add `busy_threads` metric to Puma integration This new stat was [added about 6 months ago](puma/puma#3517) to track "the overall current state of work to be done and the capacity to do it". It seems like it could be useful to track, potentially for autoscaling or other applications. * Update puma/metadata.csv Co-authored-by: May Lee <may.lee@datadoghq.com> * Update puma/metadata.csv Co-authored-by: Steven Yuen <steven.yuen@datadoghq.com> --------- Co-authored-by: May Lee <may.lee@datadoghq.com> Co-authored-by: Steven Yuen <steven.yuen@datadoghq.com>
Description
adds busy_threads to stats and also DRYs related code
the DRYing code (worker_handle.rb and worker.rb) can go into another PR or removed if preferred
Your checklist for this pull request
[ci skip]to the title of the PR.#issue" to the PR description or my commit messages.