-
Notifications
You must be signed in to change notification settings - Fork 25.3k
Use ThreadedActionListener in ListenableFuture #94367
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
Use ThreadedActionListener in ListenableFuture #94367
Conversation
Following elastic#94363 we can use `ThreadedActionListener` to represent the forking that happens within `ListenableFuture`, rather than tracking the executor and listener as separate components of a Tuple.
Pinging @elastic/es-core-infra (Team:Core/Infra) |
if (done) { | ||
return false; | ||
} | ||
if (threadContext != null) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor: these operations could be done outside the lock. Not sure if there's enough contention here to make any difference though
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think there isn't all that much contention here in practice, and it's been like this since it was introduced 4 years ago in #37327.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, one minor ordering comment
@elasticmachine update branch |
Following #94363 we can use
ThreadedActionListener
to represent the forking that happens withinListenableFuture
, rather than tracking the executor and listener as separate components of a Tuple.