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

Improve ReservedThreadExecutor dump #6652

Closed
sbordet opened this issue Aug 23, 2021 · 0 comments · Fixed by #6653 or #6662
Closed

Improve ReservedThreadExecutor dump #6652

sbordet opened this issue Aug 23, 2021 · 0 comments · Fixed by #6653 or #6662

Comments

@sbordet
Copy link
Contributor

sbordet commented Aug 23, 2021

Target Jetty version(s)
9.4.x

Enhancement Description
ReservedThreadExecutor.dump() can yield un-intuitive output; for example with capacity 1, this may happen:

+= ReservedThreadExecutor@26c2e1dc{s=1/1,p=1} - STARTED
  |  +> reserved size=3
  |     +> ReservedThread@a6fccfc{RESERVED,thread=Thread[server-2188,5,main]}
  |     +> ReservedThread@645f60a4{PENDING,thread=null}
  |     +> ReservedThread@4647ec4a{RUNNING,thread=Thread[server-2191,5,main]}

which is un-intuitive because there are 3 threads when the capacity is 1.

The state of ReservedThreadExecutor is correct because there is only 1 thread that is RESERVED.
One is RUNNING and the one PENDING will be checked against the capacity and returned to the pool -- it was "caught" by the dump in a transient moment.

sbordet added a commit that referenced this issue Aug 23, 2021
Now adding/removing ReservedThread instances to _threads only in reservedWait().
In this way the window of time in which they can be caught for a dump() is shorter, producing more accurate dumps.

Signed-off-by: Simone Bordet <simone.bordet@gmail.com>
@sbordet sbordet added this to To do in Jetty 9.4.44 FROZEN via automation Aug 23, 2021
@sbordet sbordet added this to To do in Jetty 10.0.7/11.0.7 FROZEN via automation Aug 23, 2021
@sbordet sbordet linked a pull request Aug 23, 2021 that will close this issue
sbordet added a commit that referenced this issue Aug 23, 2021
Fixed test failing due to the changes in toString().

Signed-off-by: Simone Bordet <simone.bordet@gmail.com>
sbordet added a commit that referenced this issue Aug 24, 2021
Incorporate suggestions from review.

Signed-off-by: Simone Bordet <simone.bordet@gmail.com>
sbordet added a commit that referenced this issue Aug 24, 2021
Updated doStop() to interrupt non-null threads.

Signed-off-by: Simone Bordet <simone.bordet@gmail.com>
sbordet added a commit that referenced this issue Aug 24, 2021
Fixes #6652 - Improve ReservedThreadExecutor dump.

Filtering out non-reserved threads in dump() and doStop().

Signed-off-by: Simone Bordet <simone.bordet@gmail.com>
Co-authored-by: Greg Wilkins <gregw@webtide.com>
sbordet added a commit that referenced this issue Aug 24, 2021
Fixes #6652 - Improve ReservedThreadExecutor dump.

Filtering out non-reserved threads in dump() and doStop().

Signed-off-by: Simone Bordet <simone.bordet@gmail.com>
Co-authored-by: Greg Wilkins <gregw@webtide.com>
(cherry picked from commit b2a0236)
Jetty 9.4.44 FROZEN automation moved this from To do to Done Aug 25, 2021
Jetty 10.0.7/11.0.7 FROZEN automation moved this from To do to Done Aug 25, 2021
sbordet added a commit that referenced this issue Aug 25, 2021
Fixes #6652 - Improve ReservedThreadExecutor dump.

Filtering out non-reserved threads in dump() and doStop().

Signed-off-by: Simone Bordet <simone.bordet@gmail.com>
Co-authored-by: Greg Wilkins <gregw@webtide.com>
(cherry picked from commit b2a0236)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
1 participant