Skip to content

Conversation

@DaveCTurner
Copy link
Contributor

Following #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.

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.
@DaveCTurner DaveCTurner added :Core/Infra/Core Core issues without another label >refactoring v8.8.0 labels Mar 7, 2023
@DaveCTurner DaveCTurner requested a review from thecoop March 7, 2023 13:29
@elasticsearchmachine elasticsearchmachine added the Team:Core/Infra Meta label for core/infra team label Mar 7, 2023
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra (Team:Core/Infra)

if (done) {
return false;
}
if (threadContext != null) {
Copy link
Member

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

Copy link
Contributor Author

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.

Copy link
Member

@thecoop thecoop left a 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

@DaveCTurner
Copy link
Contributor Author

@elasticmachine update branch

@DaveCTurner DaveCTurner added the auto-merge-without-approval Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) label Mar 7, 2023
@DaveCTurner DaveCTurner merged commit 3b9a21f into elastic:main Mar 7, 2023
@DaveCTurner DaveCTurner deleted the 2023-03-07-ListenableFuture-using-ThreadedActionListener branch March 7, 2023 17:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

auto-merge-without-approval Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) :Core/Infra/Core Core issues without another label >refactoring Team:Core/Infra Meta label for core/infra team v8.8.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants