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

Drop action future that forks on listener executor #53261

Merged

Conversation

jasontedor
Copy link
Member

This commit drops the dispatching listenable action future that forks to the listener thread pool. This was previously used in the transport client but is no longer used.

Relates #53049

This commit drops the dispatching listenable action future that forks to
the listener thread pool. This was previously used in the transport
client but is no longer used.
@jasontedor jasontedor added >non-issue :Core/Infra/Core Core issues without another label v8.0.0 labels Mar 7, 2020
@elasticmachine
Copy link
Collaborator

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

Copy link
Member

@original-brownbear original-brownbear left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@jasontedor jasontedor merged commit d5ae7db into elastic:master Mar 8, 2020
jasontedor added a commit that referenced this pull request Mar 8, 2020
This commit drops the dispatching listenable action future that forks to
the listener thread pool. This was previously used in the transport
client but is no longer used.
@jasontedor jasontedor deleted the dispatchable-listener-action-future branch March 8, 2020 16:38
DaveCTurner added a commit to DaveCTurner/elasticsearch that referenced this pull request Feb 9, 2021
As of elastic#53261 today `PlainListenableActionFuture` is the sole
implementation of `ListenableActionFuture`, the reason for the
`Plain...` prefix is now lost. It also heavily uses variable shadowing
in its implementation which makes it quite hard to read, uses a mix of
`volatile` fields and mutexes, and keeps hold of the deferred listeners
even after completion. Finally it does not have many tests.

This commit drops the unnecessary interface and renames the class to
simply `ListenableActionFuture`. It reworks the implementation to avoid
shadowing variables, drops the deferred listeners on completion, and
moves to an entirely mutex-based implementation. Finally, it adds
another test that it works as advertised.
DaveCTurner added a commit that referenced this pull request Feb 9, 2021
As of #53261 today `PlainListenableActionFuture` is the sole
implementation of `ListenableActionFuture`, the reason for the
`Plain...` prefix is now lost. It also heavily uses variable shadowing
in its implementation which makes it quite hard to read, uses a mix of
`volatile` fields and mutexes, and keeps hold of the deferred listeners
even after completion. Finally it does not have many tests.

This commit drops the unnecessary interface and renames the class to
simply `ListenableActionFuture`. It reworks the implementation to avoid
shadowing variables, drops the deferred listeners on completion, and
moves to an entirely mutex-based implementation. Finally, it adds
another test that it works as advertised.
DaveCTurner added a commit that referenced this pull request Feb 9, 2021
As of #53261 today `PlainListenableActionFuture` is the sole
implementation of `ListenableActionFuture`, the reason for the
`Plain...` prefix is now lost. It also heavily uses variable shadowing
in its implementation which makes it quite hard to read, uses a mix of
`volatile` fields and mutexes, and keeps hold of the deferred listeners
even after completion. Finally it does not have many tests.

This commit drops the unnecessary interface and renames the class to
simply `ListenableActionFuture`. It reworks the implementation to avoid
shadowing variables, drops the deferred listeners on completion, and
moves to an entirely mutex-based implementation. Finally, it adds
another test that it works as advertised.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants