Skip to content

Conversation

@arturobernalg
Copy link
Member

@arturobernalg arturobernalg commented Dec 27, 2025

This change adds an optional hard cap on the number of pending request execution commands queued per HTTP/2 connection. When the per-connection limit is reached, new submissions fail fast with RejectedExecutionException and the exchange handler releases its resources immediately. The cap is configured via H2MultiplexingRequesterBootstrap and passed into H2MultiplexingRequester at construction time to keep requester configuration immutable and avoid API incompatibilities.

@arturobernalg arturobernalg requested a review from ok2c December 27, 2025 18:14
@ok2c
Copy link
Member

ok2c commented Dec 28, 2025

@arturobernalg I do not like this approach. There is already a command queue built into the IOSession. We should leverage that queue instead of creating another layer of complexity. What we need is to extend IOSession interface to expose information about already pending commands and optionally reject commands if the queue is over a particular limit.

…ommands per connection and fail fast with RejectedExecutionException when the limit is reached.
@arturobernalg arturobernalg force-pushed the max-requests-per-connection branch from de6f206 to b777f5d Compare December 29, 2025 17:58
@arturobernalg
Copy link
Member Author

arturobernalg commented Dec 29, 2025

@arturobernalg I do not like this approach. There is already a command queue built into the IOSession. We should leverage that queue instead of creating another layer of complexity. What we need is to extend IOSession interface to expose information about already pending commands and optionally reject commands if the queue is over a particular limit.

@ok2c . Sorry about the churn — I’m still getting up to speed on this part of the reactor stack and my previous commit took a wrong turn. I dropped the extra wrapper and enforce the cap via the existing IOSession command queue. I extended IOSession with a bounded enqueue + pending count and delegated through InternalDataChannel and SSLIOSession so the limit is not bypassed by decorators.

* @since 5.5
*/
default int getPendingCommandCount() {
return 0;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@arturobernalg I think the method by default return -1 as not known

* @return {@code true} if the command was enqueued, {@code false} otherwise.
* @since 5.5
*/
default boolean enqueue(final Command command, final Command.Priority priority, final int maxPendingCommands) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@arturobernalg I would not do that on the i/o session level. Different protocol handlers may have different restrictions. 10 concurrent requests may be perfectly OK for HTTP/2 and too much for HTTP/1.1. Please move this logic to individual protocol handlers.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants