Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

QueuedThreadPool "free" threads #5994

Closed
sbordet opened this issue Feb 22, 2021 · 4 comments · Fixed by #5995
Closed

QueuedThreadPool "free" threads #5994

sbordet opened this issue Feb 22, 2021 · 4 comments · Fixed by #5995

Comments

@sbordet
Copy link
Contributor

sbordet commented Feb 22, 2021

Jetty version
9.4.x

Description
On large machines, the heuristics we have for number of acceptors, selectors, and reserved threads may "steal" a lot of threads from the thread pool, so that a metric such as (threads - idleThreads)/maxThreads may show large percentages even for a completely idle server.

For example, for a 112 cores machine we have

  • 4 acceptors
  • 56 selectors
  • 112 reserved

for a total of 172 "stolen" threads.
With maxThreads=1024 or similar, that is a 17% "utilization" even if the server is idle.

Using busyThreads instead of (threads - idleThreads) takes into account reserved threads, so that figure would be only 60 "stolen" threads for a 6% "utilization".

I wonder if we should at exclude acceptors and selectors from the count (i.e. rather than taking them from the pool, allocate the threads outside the pool -- however this would make the thread budget component irrelevant now).

Excluding them would help people to answer the question: "should I increase maxThreads or I'm good?".
Users that monitor busyThreads/maxThreads will know on a 0-100% scale rather than always having a 6-17% "noise" even when the server is idle.

@gregw @joakime @lorban thoughts?

@sbordet
Copy link
Contributor Author

sbordet commented Feb 22, 2021

OTOH, it's good to know how many threads are actually used, internally, by Jetty.

@gregw
Copy link
Contributor

gregw commented Feb 22, 2021

I think the (threads - idleThreads) is not the right number. It should be (threads - idleThreads + reservedThreads). While a reserved thread is not available directly to handle a request etc. they are available to replace a selector thread so that the selector thread can handle a request it has just accepted.

Other metrics to consider making available:

 readyThreads = idle + reserved;
 unusedThreads = max - threads
 availableThreads = readyThreads + unusedThreads
 unavailableThreads = threads - idle - reserved

@sbordet
Copy link
Contributor Author

sbordet commented Feb 22, 2021

After initial discussion, we may expose the thread budget component, so that we will know how many threads are used internally by Jetty, but also we could use it to know how many are "stolen" and should not be accounted for.

@gregw
Copy link
Contributor

gregw commented Feb 23, 2021

@sbordet
We need to work on vocab, because "stolen" doesn't really capture the meaning hear. I can see the following levels of usage for threads in a QTP, each needs a short name: something like:

  • inactive: not started but there is capacity to start another thread
  • idle: started and waiting in the pool for a task
  • reserved: taken from the pool but reserved to do a task
  • budgeted: taken from the pool to do a budgeted task like selecting (could call it "server", but QTP used on client.. maybe allocated? )
  • busy: handling a task (eg request). This is a little different to current busy definition

So we have some invariants and derived stats:

 max = inactive + idle + reserved + budgeted + busy
 threads = idle + reserved + budgeted + busy
 busy = threads - idle - reserved - budgeted 
 usable = max - budgeted
 utilised = busy / usable # 0.0 means idle, 1.0 means at capacity 

sbordet added a commit that referenced this issue Feb 23, 2021
Updates after review.

Signed-off-by: Simone Bordet <simone.bordet@gmail.com>
sbordet added a commit that referenced this issue Feb 24, 2021
@sbordet sbordet linked a pull request Feb 24, 2021 that will close this issue
This was referenced Mar 10, 2021
This was referenced Mar 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants