Hi all,
I recently ran into an issue withHttpClientImplwhere I wanted to handle the
reply right after callingsendAsync. What surprised me is that the
returnedCompletableFuturealready runs on thecommonPool, instead of using the
executor I provided to theHttpClient.
Looking into the implementation, I noticed this piece of code:
// makes sure that any dependent actions happen in the CF default
// executor. This is only needed for sendAsync(...), when
// exchangeExecutor is non-null.
if (exchangeExecutor != null) {
res = res.whenCompleteAsync((r, t) -> { /* do nothing */}, ASYNC_POOL);
}
I understand that thisexchangeExecutoris meant to cover the request/response
exchange itself, not necessarily theCompletableFuturechaining. But the fact
that we always force the returned future back onto thecommonPool, without any
way to change this, feels quite limiting.
In environments where thecommonPoolis already heavily loaded, this can easily
introduce performance issues or unpredictable behavior. And since
private static final Executor ASYNC_POOL = new
CompletableFuture<Void>().defaultExecutor();
is fixed and cannot be replaced, users don’t have any way around it. For
comparison, the default executor for CompletableFuture.supplyAsync or for
parallelStream() is also the commonPool, but in those cases it’s easy to
override it with a custom executor. It would be nice if HttpClientImpl offered
the same flexibility.
This is why I’d like to propose a change: when creating anHttpClientImpl, it
should be possible to specify not only the exchange executor, but also the
executor used for the returnedCompletableFuture
This would be backwards compatible (just an additional optional builder
parameter), and it could bring several benefits:
-
reduced context switching for clients that care about which thread executes
response handling,
-
more predictable performance in environments wherecommonPoolis shared with
other workloads,
-
easier integration with frameworks that already manage their own executors,
- clearer control and observability over where callbacks are executed.
Would such a change make sense, or is there a strong reason why wemust always
fallback to thecommonPool?
