Remove `max-open-requests` constraint on Flow-Based APIs

[patrick-premont]

I think it's natural to expect Flow-based APIs to backpressure, yet the max-open-requestsetting violates that expectation, introducing run-time errors instead of back-pressure.

This answer appears to agree with that expectation: https://discuss.lightbend.com/t/difference-between-http-singlerequest-and-http-superpool/713

Despite some efforts of the documentation, it is not clear how to set max-open-requests to guarantee those errors will never occur. Apparently, more materializations that share the same pool necessitates a higher value for the setting? If so that suggests more requests would be "open" than what the shared pooled is supported to allow.

It appears that this setting is there to facilitate the implementation, as a buffer of that size is allocated.

Perhaps removing this constraint would require using a MergeHub?

There are issues that relate to max-open-requests in Flow-Based APIs that attest to the difficulties it introduces: #843 #125

[jrudolph]

We never answered here (but maybe discussed it somewhere else?). In any case, it seems like a good idea.

[jrudolph]

I'll try to describe why it is the way it is currently and what we could do about it in 10.2.0.

The first thing to understand is the difference in how Http.singleRequest(), Http().cachedHostConnectionPoolHttps and Http().superPool are implemented. Requests coming in through any of those entry point APIs may end up in the same shared pool. singleRequest is a push-based Future API while the others are streaming APIs.

Currently, a shared pool to a host is implemented with the functionality of Http().singleRequest in mind. Since a while, this is abstracted as the PoolInterface which offers a push-based request method to run a single request. PoolInterfaces are then registered in the PoolMasterActor which dispatches a request based on the target host and spawns pools to a host as needed.

The pool itself (NewHostConnectionPool) and the interface in front of it are implemented as a streaming API. To support the push-based PoolInterface API, a buffer is needed to store requests if they come in a burst. The size of that buffer is goverened by the max-open-requests setting (which is and has been a misnomer since the beginning).

The streaming based APIs cachedHostConnectionPoolHttps and superPool are just implemented on top of that push-based API, using mapAsyncUnordered to offer requests to the pool using the pool interface. That means, however, that there's no API that would offer end-to-end backpressure. Instead, parallelism is limited just by the value given to mapAsyncUnordered which is hardcoded to be settings.pipeliningLimit * settings.maxConnections. This makes somewhat sense because it tries to queue up no more requests in front of a pool as the pool may run concurrently. As @patrick-premont noticed, it breaks down as soon as you attach multiple such streams to a shared pool that all share a fixed number of max-open-requests that might be smaller than settings.pipeliningLimit * settings.maxConnections * number of streams.

Incidentally, the superPool API doesn't seem to be useful because the superPool flow has to group by target hosts, so there can be head-of-line blocking when the stream mixes requests to fast and slow hosts (there's nothing we can do about that, it's inherent in the streaming superPool API).

That still leaves cachedHostConnectionPoolHttps and newHostConnectionPool for both of which an end-to-end backpressured API would make sense. Otherwise, with the current API, what should you set the value of max-open-requests to? The safe choice would be to set it to settings.pipeliningLimit * settings.maxConnections * number of streams but that means that 1) you need a static limit of streams and 2) you will usually have an oversized buffer in front of the pool adding to the end-to-end latency you will measure when using that pool.

So much for the current situation.

[jrudolph]

Now to what we could do about it in the future.

The pool itself is already implemented in a streaming way, so it should be easy enough to offer that as an API. As suggested we could put use a MergeHub / BroadcastHub to offer that streaming API to multiple consumers. However, with this sharing come a few new complications:

• There would be no inherent buffer for the streaming APIs any more. A (request) buffer would only be needed for the push-based API.
• How would the idle-timeout work? If we would keep it for the streaming API it would mean that some requests might be lost when idle-timeout and new incoming requests race against each other and the request is lost somewhere in transit where cancellation meets an incoming request. So, I'd say we should probably disable idle-timeouts when streams are involved because streams are long-lived and already managed. An open question is then still how to deal with both a streaming consumer and a singleRequest consumer for a pool. singleRequest consumers would want to rely on a timeout to clean up resources currently not needed any more. But as soon as there would be streaming consumers the time out should be disabled (e.g. a streaming consumer would have to automatically send a tick once in a while to prevent an idle timeout from kicking in).
• End-to-end backpressure with multiple users of the same pool means, that there has to be some dispatch behind the pool which dispatches incoming responses to the right stream. This means we now have a head-of-line blocking issue at this end of the stream. I.e. per consumer stream we want to enforce that we only pull in a request of that stream into the pool if the consumer stream has already pulled for a response. There are two problems with that: 1) the stream can only pull at most one response which means that there will be no parallelism or we still need a buffer of size settings.maxConnections per consumer-stream to be able to support parallelism per consumer stream. 2) It's unclear how to achieve that kind of backpressure model across the MergeHub / BroadcastHub combo.

So, in summary, we'll need:

• to expose the streaming interface to a pool
• a shared request buffer and an idle timeout for the singleRequest API
• a way to disable the idle timeout for a streaming consumer
• a response buffer per streaming consumer to allow parallelism (same as with mapAsync which also requests parallelism input elements even if the downstream has only requested a single element).

[jrudolph]

Pushed back for now.