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

EsAbortPolicy should show thread pool name #9732

Closed
pickypg opened this issue Feb 17, 2015 · 3 comments
Closed

EsAbortPolicy should show thread pool name #9732

pickypg opened this issue Feb 17, 2015 · 3 comments
Assignees
Labels
:Core/Infra/Core Core issues without another label >enhancement good first issue low hanging fruit help wanted adoptme v2.0.0-beta1

Comments

@pickypg
Copy link
Member

pickypg commented Feb 17, 2015

Whenever EsAbortPolicy aborts an action, it should report the thread pool's name to make it easier to diagnose where it's coming from (usually the stacktrace is enough, but it would be a lot faster to have its name).

An instanceof check needs to be done to run it against the EsThreadPoolExecutor, and each EsThreadPoolExecutor needs to be given its name to allow reporting of it.

@jpountz
Copy link
Contributor

jpountz commented Feb 18, 2015

An instanceof check needs to be done to run it against the EsThreadPoolExecutor, and each EsThreadPoolExecutor needs to be given its name to allow reporting of it.

We could even skip the instanceof call and make sure that the thread pool name is included in the toString() representation of the thread pool?

@clintongormley clintongormley added >enhancement good first issue low hanging fruit help wanted adoptme :Core/Infra/Core Core issues without another label labels Feb 27, 2015
@nik9000 nik9000 self-assigned this Jul 29, 2015
@nik9000
Copy link
Member

nik9000 commented Jul 29, 2015

This turned out a to be a little more extensive then I'd thought it would be at first. Adding the name involves lots of fun ductwork and making EsThreadPoolExecutor's toString nice gives you lots of fun diagnostic information.

@nik9000
Copy link
Member

nik9000 commented Jul 29, 2015

Old message:
EsRejectedExecutionException[rejected execution (shutdown) on dummy runnable]
New message:
EsRejectedExecutionException[rejected execution of dummy runnable on EsThreadPoolExecutor[some-name, queue capacity = 78, state = Terminated, pool size = 2, active threads = 0, queued tasks = 0, completed tasks = 1231241234]]

Old message:
EsRejectedExecutionException[rejected execution (queue capacity 78) on dummy runnable]
New message:
EsRejectedExecutionException[rejected execution of dummy runnable on EsThreadPoolExecutor[some-name, queue capacity = 78, state = Running, pool size = 2, active threads = 2, queued tasks = 78, completed tasks = 0]]

nik9000 added a commit to nik9000/elasticsearch that referenced this issue Aug 5, 2015
Improving the toString allows for nicer error reporting. Also cleaned up
the way that EsRejectedExecutionException notices that it was rejected
from a shutdown thread pool. I left javadocs about how its not 100% correct
but good enough for most uses.

The improved toString on EsThreadPoolExecutor mean every one of them needs
a name. In most cases the name to use is obvious. In tests I use the name
of the test method and in real thread pools I use the name of the thread
pool. In non-ThreadPool executors I use the thread's name.

Closes elastic#9732
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Core/Infra/Core Core issues without another label >enhancement good first issue low hanging fruit help wanted adoptme v2.0.0-beta1
Projects
None yet
Development

No branches or pull requests

4 participants