HTTPCLIENT-2398 - Cap async execution queue to break recursive re-entry

[arturobernalg]

Implements Option 1 from the discussion: add a configurable cap on the async execution pipeline to prevent the recursive callback loop that led to StackOverflowError.

[ok2c]

@arturobernalg Could you please rebase this change-set?

[ok2c]

@arturobernalg This is a good start. Please also do not forget about InternalH2AsyncExecRuntime.

[ok2c]

@arturobernalg Looks good. Can we add the same functionality to H2AsyncClientBuilder as well?

[arturobernalg]

ks good. Can we add the same functionality to H2AsyncClientBuilder as well?

@ok2c please do another pass

[ok2c]

@arturobernalg There is again plenty of white space / formatting noise. Please try to avoid it.

[ok2c]

@arturobernalg I am not super crazy about code re-use at all cost but in this case there is just too much duplicate code. Please consider extracting into a separate proxy class similar to RequestEntityProxy / ResponseEntityProxy

[ok2c]

@arturobernalg This is bad. We need to find a way to maintain the counter without using a static hash map. The limit should ideally be per connection. If this is too difficult, then, per client instance.

[arturobernalg]

@ok2c please do another pass

[ok2c]

@arturobernalg Do not see how this is any better. The static map is still there and what you have is essentially a major resource leak right there.

[ok2c]

@arturobernalg Do not see how this is any better. The static map is still there and what you have is essentially a major resource leak right there.

[arturobernalg]

@ok2c got it. No more static var.

[ok2c]

@arturobernalg This is better. Please mark new setters in the builder classes as experimental.

Okay, this is what we have now

Users could now limit the number of concurrent active requests on a per client basis. How useful is that, though? Say, one sets max concurrent request number to 1000, then opens 100 active connections and equally distributes requests over those connections. 10 concurrent requests per connection is very little. However, having just one connection and trying to execute 1000 requests concurrently over it would likely be wrong.

What would be more useful is to cap the total number of requests on a per connection basis.

What could also be useful is enqueuing requests instead of failing them and executing them as soon as the number of concurrent requests drops below the maximum. Moreover, the same queuing mechanism could be used to re-queue and retry requests that did not get executed due to the opposite endpoint closing the connection.

This change-set is a start in the right direction. Let's keep it though clearly marked as experimental for now.

[arturobernalg]

What could also be useful is enqueuing requests instead of failing them and executing them as soon as the number of concurrent requests drops below the maximum. Moreover, the same queuing mechanism could be used to re-queue and retry requests that did not get executed due to the opposite endpoint closing the connection.

@ok2c Marked the new builder setters as @experimental. Agreed on the limitations of a per-client cap; I’ll keep this change-set scoped to fail-fast limiting for now. I’ll follow up with separate PRs for (1) per-connection caps and (2) optional bounded queuing / re-queue of not-yet-started requests.

[ok2c]

I’ll follow up with separate PRs for (1) per-connection caps

@arturobernalg Please note this will likely have to be done in core.

[arturobernalg]

I’ll follow up with separate PRs for (1) per-connection caps

@arturobernalg Please note this will likely have to be done in core.

@ok2c Ill take a look the option to move it to the core