-
Notifications
You must be signed in to change notification settings - Fork 24.4k
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
Move conceptual docs about ActionListener
#107875
Move conceptual docs about ActionListener
#107875
Conversation
This information is more discoverable as the class-level javadocs for `ActionListener` itself rather than hidden away in a separate Markdown file. Also this way the links all stay up to date.
Pinging @elastic/es-distributed (Team:Distributed) |
channel with which to pass the response back to the network caller. Netty has a many-to-one association of network callers to channels, so a | ||
call taking a long time generally won't hog resources: it's cheap. A transport action can take hours to respond and that's alright, barring | ||
caller timeouts. | ||
See the [Javadocs for `ActionListener`](https://github.com/elastic/elasticsearch/blob/main/server/src/main/java/org/elasticsearch/action/ActionListener.java) |
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 expect this link isn't going to break for a very long time, and if it ever does it should be easy enough to fix it up, but in principle once these docs land in a released version we can pin this to that version instead.
* <li>Most commonly, they can be passed on to other methods that themselves require a callback.</li> | ||
* <li>They can be wrapped in another callback which modifies the behaviour of the original callback, perhaps adding some extra code to run | ||
* before or after completion, before passing them on.</li> | ||
* </ul> |
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.
Should we mention that action listener is our preferred alternative to Future/CompletableFutre as that is a well known way to pass a result of async computation in java?
* expensive to waste with unnecessary blocking) and at worst outright broken (the blocking can lead to deadlock). Unfortunately this means | ||
* that most of our code ends up having to be written with callbacks, simply because it's ultimately calling into some other code that takes | ||
* a callback. The entry points for all Elasticsearch APIs are callback-based (e.g. REST APIs all start at {@link | ||
* org.elasticsearch.rest.BaseRestHandler}{@code #prepareRequest} and transport APIs all start at {@link |
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.
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.
Yeah unfortunately BaseRestHandler#prepareRequest
is protected
so we can't link to it directly here. I still think it's better to link to it like this, although this obstacle suggests that we should consider moving some of these docs onto BaseRestHandler
(or maybe org.elasticsearch.rest
).
This information is more discoverable as the class-level javadocs for `ActionListener` itself rather than hidden away in a separate Markdown file. Also this way the links all stay up to date.
This information is more discoverable as the class-level javadocs for
ActionListener
itself rather than hidden away in a separate Markdownfile. Also this way the links all stay up to date.