# What is the proper design pattern for plugins that must submit multiple requests to Elasticsearch? **URL:** \ **Category:** Elasticsearch\ **Tags:** language-clients\ **Created:** [January 26, 2021, 3:42am UTC](https://discuss.elastic.co/t/what-is-the-proper-design-pattern-for-plugins-that-must-submit-multiple-requests-to-elasticsearch/262190 "2021-01-26T03:42:18Z")\ **Posts on this page:** 15\ **Page:** 1
**Author:** ![davemoore](https://sea2.discourse-cdn.com/elastic/user_avatar/discuss.elastic.co/davemoore/32/134233_2.png) [@davemoore](https://discuss.elastic.co/u/davemoore)\ **Post date:** [January 26, 2021, 3:42am UTC](https://discuss.elastic.co/t/what-is-the-proper-design-pattern-for-plugins-that-must-submit-multiple-requests-to-elasticsearch/262190/1 "2021-01-26T03:42:18Z")
**Question for plugin authors** What is the proper way to implement a REST handler in a custom [`ActionPlugin`](https://github.com/elastic/elasticsearch/blob/v7.10.2/server/src/main/java/org/elasticsearch/plugins/ActionPlugin.java) (a.k.a. [API extension plugin](https://www.elastic.co/guide/en/elasticsearch/plugins/current/api.html)) that must wait for the response of multiple Elasticsearch requests before returning a response to the user? **Example** Consider the example handler below, which sends two requests to Elasticsearch on behalf of a single request from the user. The handler uses its given [`NodeClient`](https://github.com/elastic/elasticsearch/blob/v7.10.2/server/src/main/java/org/elasticsearch/rest/BaseRestHandler.java#L92) to: 1. Create an index 2. Perform a search 3. Respond to the user after both actions have completed ```java @Override protected RestChannelConsumer prepareRequest(final RestRequest request, final NodeClient client) { return channel -> { try { // Create an arbitrary index client.admin().indices().prepareCreate("sample-index") .setSettings(Settings.builder() .put("index.number_of_shards", 1) .put("index.number_of_replicas", 0) ) .addMapping("doc", "{\"properties\":{\"foo\":{\"type\":\"keyword\"}}}", XContentType.JSON) .get(); // Submit an arbitrary search request client.prepareSearch("*").get(); // Return an arbitrary response XContentBuilder content = XContentFactory.jsonBuilder(); content.startObject().field("acknowledged", true).endObject(); channel.sendResponse(new BytesRestResponse(RestStatus.OK, content)); } catch (final Exception e) { channel.sendResponse(new BytesRestResponse(channel, e)); } }; } ``` **Problem** The problem with the example above is that the two `.get()` actions submitted by the `NodeClient` are blocking calls (see [explanation](https://github.com/elastic/elasticsearch/issues/67960#issuecomment-767222673)). This will [cause an Elasticsearch cluster to hang](https://github.com/elastic/elasticsearch/issues/67960). What is the proper way to implement the desired behavior of the example above? ---
**Author:** ![DavidTurner](https://sea2.discourse-cdn.com/elastic/user_avatar/discuss.elastic.co/davidturner/32/22453_2.png) [@DavidTurner](https://discuss.elastic.co/u/DavidTurner)\ **Post date:** [January 26, 2021, 7:13am UTC](https://discuss.elastic.co/t/what-is-the-proper-design-pattern-for-plugins-that-must-submit-multiple-requests-to-elasticsearch/262190/2 "2021-01-26T07:13:01Z")
You generally write your own `ActionListener<...>` and pass it to the `.execute(listener)` method rather than calling `.get()`. Your listener's `onResponse()` method can then run the next step of the process. There's a bunch of utilities to make this a bit simpler and/or to encapsulate common patterns, e.g. `ActionListener#delegateFailure`, `ActionListener#map`, `RestResponseListener<>`, `StepListener<>`, `ActionRunnable<>` etc. It always ends up with things in a slightly funny order, but there's not really a way around that. Native syntax for async code would be nice, but that's not something Java has today. ---
**Author:** ![austince](https://sea2.discourse-cdn.com/elastic/user_avatar/discuss.elastic.co/austince/32/80987_2.png) [@austince](https://discuss.elastic.co/u/austince)\ **Post date:** [January 26, 2021, 4:03pm UTC](https://discuss.elastic.co/t/what-is-the-proper-design-pattern-for-plugins-that-must-submit-multiple-requests-to-elasticsearch/262190/3 "2021-01-26T16:03:32Z")
Would you advise against using something like the [CompletableFuture API](https://docs.oracle.com/javase/8/docs/api/java/util/concurrent/CompletableFuture.html) to wrap Actions? That seems to be the most "native" way to order async work that Java has today. ---
**Author:** ![DavidTurner](https://sea2.discourse-cdn.com/elastic/user_avatar/discuss.elastic.co/davidturner/32/22453_2.png) [@DavidTurner](https://discuss.elastic.co/u/DavidTurner)\ **Post date:** [January 26, 2021, 4:26pm UTC](https://discuss.elastic.co/t/what-is-the-proper-design-pattern-for-plugins-that-must-submit-multiple-requests-to-elasticsearch/262190/4 "2021-01-26T16:26:59Z")
Yes, IIRC the problem with `CompletableFuture` is that it swallows `Throwable` so it means you risk missing something vitally important like an `OutOfMemoryException` or an `AssertionError`. There's things like `PlainActionFuture` and `PlainListenableActionFuture` in Elasticsearch that do the same sort of thing but which only catch `Exception` which is much safer. ---
**Author:** ![austince](https://sea2.discourse-cdn.com/elastic/user_avatar/discuss.elastic.co/austince/32/80987_2.png) [@austince](https://discuss.elastic.co/u/austince)\ **Post date:** [January 26, 2021, 4:30pm UTC](https://discuss.elastic.co/t/what-is-the-proper-design-pattern-for-plugins-that-must-submit-multiple-requests-to-elasticsearch/262190/5 "2021-01-26T16:30:17Z")
Can you expand on what you mean by "swallow"? Shouldn't those errors still be reported in [`CompletableFuture#exceptionally`](https://docs.oracle.com/javase/8/docs/api/java/util/concurrent/CompletableFuture.html#exceptionally-java.util.function.Function-), as long as the actions are wrapped correctly? I think the main benefit here is that it makes controlling the async flow much, much simpler with easy control over threads, composing async work, etc. ---
**Author:** ![DavidTurner](https://sea2.discourse-cdn.com/elastic/user_avatar/discuss.elastic.co/davidturner/32/22453_2.png) [@DavidTurner](https://discuss.elastic.co/u/DavidTurner)\ **Post date:** [January 26, 2021, 4:44pm UTC](https://discuss.elastic.co/t/what-is-the-proper-design-pattern-for-plugins-that-must-submit-multiple-requests-to-elasticsearch/262190/6 "2021-01-26T16:44:50Z")
Ehh I'm probably not the best person to go into the details here so I might be working off of incorrect or dated information. I'm haven't used `CompletableFuture` very much at all. There are places where we permit completing a listener twice, and we don't observe the result of every listener either, both of which I think might swallow a vital `Error` if listeners caught them. `ActionListener` and `CompletableFuture` are _almost_ interchangeable, but this one subtle point means that they're not. AFAIK these are design decisions that Elasticsearch made before the whole `Future` framework landed in the JDK and it's a little unfortunate that they don't align. ---
**Author:** ![austince](https://sea2.discourse-cdn.com/elastic/user_avatar/discuss.elastic.co/austince/32/80987_2.png) [@austince](https://discuss.elastic.co/u/austince)\ **Post date:** [January 26, 2021, 4:49pm UTC](https://discuss.elastic.co/t/what-is-the-proper-design-pattern-for-plugins-that-must-submit-multiple-requests-to-elasticsearch/262190/7 "2021-01-26T16:49:39Z")
Ah, that makes a lot of sense, thanks David! Yeah, a bit unfortunate about the diverged async frameworks, but good to know about. Last question, promise! Do you know if there are any general situations/ guidelines on where listeners are completed twice, or is it on a case-by-case basis? ---
**Author:** ![DavidTurner](https://sea2.discourse-cdn.com/elastic/user_avatar/discuss.elastic.co/davidturner/32/22453_2.png) [@DavidTurner](https://discuss.elastic.co/u/DavidTurner)\ **Post date:** [January 26, 2021, 4:54pm UTC](https://discuss.elastic.co/t/what-is-the-proper-design-pattern-for-plugins-that-must-submit-multiple-requests-to-elasticsearch/262190/8 "2021-01-26T16:54:50Z")
No, I don't know of a general pattern. I tried to make a change once that enforced once-and-only-once completion (by throwing an `AssertionError` on a double-call) just to see how bad it was. It broke all the things. There's stuff like [`GroupedActionListener<>`](https://github.com/elastic/elasticsearch/blob/4bf960310b6c33cd0dd06617d8171b0ebc22ab3a/server/src/main/java/org/elasticsearch/action/support/GroupedActionListener.java#L38) that _deliberately_ gets called _N_ times, and things like timeouts are implemented by racing to complete a listener too. * * * (edit) Also it's pretty common that if `onResponse` throws an exception then it's passed to `onException` of the same listener. There were loads of other things too, it would be a major piece of work to migrate. ---
**Author:** ![austince](https://sea2.discourse-cdn.com/elastic/user_avatar/discuss.elastic.co/austince/32/80987_2.png) [@austince](https://discuss.elastic.co/u/austince)\ **Post date:** [January 26, 2021, 11:57pm UTC](https://discuss.elastic.co/t/what-is-the-proper-design-pattern-for-plugins-that-must-submit-multiple-requests-to-elasticsearch/262190/9 "2021-01-26T23:57:22Z")
Just adding some more notes from digging around, it looks like there is a bit of CompletableFuture usage in ES, mostly in the transport layer, wrapped by a [`CompletableContext`](https://github.com/elastic/elasticsearch/blob/v7.10.2/libs/core/src/main/java/org/elasticsearch/common/concurrent/CompletableContext.java), which interops with `ActionListener`s mostly via [`CloseableConnection`](https://github.com/elastic/elasticsearch/blob/v7.10.2/server/src/main/java/org/elasticsearch/transport/CloseableConnection.java) and [`ActionListener#toBiConsumer`](https://github.com/elastic/elasticsearch/blob/747e1cc71def077253878a59143c1f785afa92b9/server/src/main/java/org/elasticsearch/action/ActionListener.java#L185-L201). Not to go against your suggestion, just taking a look at how that API is making its way into ES. ---
**Author:** ![DavidTurner](https://sea2.discourse-cdn.com/elastic/user_avatar/discuss.elastic.co/davidturner/32/22453_2.png) [@DavidTurner](https://discuss.elastic.co/u/DavidTurner)\ **Post date:** [January 27, 2021, 7:25am UTC](https://discuss.elastic.co/t/what-is-the-proper-design-pattern-for-plugins-that-must-submit-multiple-requests-to-elasticsearch/262190/10 "2021-01-27T07:25:39Z")
That's an interesting question. `CompletableContext` was added to isolate the usage of `CompletableFuture` in the transport layer to avoid catching `Throwable`: > I'm not sure why the implementation is still based on `CompletableFuture`, I think there are other viable options too, I'll have to ask around. ---
**Author:** ![austince](https://sea2.discourse-cdn.com/elastic/user_avatar/discuss.elastic.co/austince/32/80987_2.png) [@austince](https://discuss.elastic.co/u/austince)\ **Post date:** [January 27, 2021, 6:23pm UTC](https://discuss.elastic.co/t/what-is-the-proper-design-pattern-for-plugins-that-must-submit-multiple-requests-to-elasticsearch/262190/11 "2021-01-27T18:23:11Z")
Oh, that's a very interesting history. Do you know the reason behind the pretty strict differentiation between `Exception` and `Error` in Elasticsearch? What's the issue with using `Throwable`? ---
**Author:** ![DavidTurner](https://sea2.discourse-cdn.com/elastic/user_avatar/discuss.elastic.co/davidturner/32/22453_2.png) [@DavidTurner](https://discuss.elastic.co/u/DavidTurner)\ **Post date:** [January 27, 2021, 6:48pm UTC](https://discuss.elastic.co/t/what-is-the-proper-design-pattern-for-plugins-that-must-submit-multiple-requests-to-elasticsearch/262190/12 "2021-01-27T18:48:45Z")
Just following [the docs](https://docs.oracle.com/en/java/javase/15/docs/api/java.base/java/lang/Error.html): > An `Error` is a subclass of `Throwable` that indicates serious problems that a reasonable application should not try to catch. Elasticsearch is a reasonable application and it therefore tries hard not to catch any `Error`. If an `Error` is thrown then there's no sensible way to recover or handle it, all you can do is exit. ---
**Author:** ![davemoore](https://sea2.discourse-cdn.com/elastic/user_avatar/discuss.elastic.co/davemoore/32/134233_2.png) [@davemoore](https://discuss.elastic.co/u/davemoore)\ **Post date:** [January 29, 2021, 6:25pm UTC](https://discuss.elastic.co/t/what-is-the-proper-design-pattern-for-plugins-that-must-submit-multiple-requests-to-elasticsearch/262190/13 "2021-01-29T18:25:22Z")
Future readers: Check out this blog post which proposes some elegant solutions with code examples for asynchronous usage of the Elasticsearch 7.x Java APIs: > **[Wrap Elasticsearch Response Into CompletableFuture](https://mincong.io/2020/07/26/es-client-completablefuture/)** > > Wrap Elasticsearch client response into CompletableFuture in Java for Elasticsearch transport client or Java high level REST client. See also: This [PR](https://github.com/elastic/elasticsearch/pull/32512) for a `CompletableFuture` implementation that is slated for Elasticsearch 8.x. ---
**Author:** ![DavidTurner](https://sea2.discourse-cdn.com/elastic/user_avatar/discuss.elastic.co/davidturner/32/22453_2.png) [@DavidTurner](https://discuss.elastic.co/u/DavidTurner)\ **Post date:** [January 29, 2021, 8:18pm UTC](https://discuss.elastic.co/t/what-is-the-proper-design-pattern-for-plugins-that-must-submit-multiple-requests-to-elasticsearch/262190/14 "2021-01-29T20:18:47Z")
I can't recommend the examples in that blog post for the same reasons we were discussing above: they catch (and sometimes silently swallow) an `Error` which no reasonable application should do. The PR you link may get merged eventually but we're definitely not committing to merging it into any particular version. The `8.0.0` label is only because a version is required on PRs and this is the largest number available today. The whole point of that PR is to prevent folks from using a bare `CompletableFuture` since it might accidentally swallow an `Error`. FWIW we just merged a PR that removes some other usages of `CompletableFuture` that we had inadvertently introduced: > ---
**Author:** ![system](https://us1.discourse-cdn.com/elastic/original/3X/1/a/1ac57faf039f6b580b3f104ef42a2a89e41014de.png) [@system](https://discuss.elastic.co/u/system)\ **Post date:** [February 26, 2021, 8:19pm UTC](https://discuss.elastic.co/t/what-is-the-proper-design-pattern-for-plugins-that-must-submit-multiple-requests-to-elasticsearch/262190/15 "2021-02-26T20:19:27Z")
This topic was automatically closed 28 days after the last reply. New replies are no longer allowed.