Direct data delivery for HTTP/2 Websocket #1685
Open
+331
−50
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
essen
force-pushed
the
direct-data_delivery-for-h2-websocket
branch
from
31162a1 to
cb5efcb
Compare
The current implementation moves the Websocket data through
stream handlers before the Websocket process receives it and
parses it. The new mechanism relays the data directly as
an Erlang message without any intermediary. It also manages
HTTP/2 flow control in a simpler way (it maintains it at a
configured value).
A new switch_protocol option (coming from the options returned
from init/2) called `data_delivery` decides which mechanism
to use: `stream_handlers` is the old behavior; `relay` the new.
WIP: Need to test the relay mechanism in other test suites
WIP: the benchmark test suite is greatly modified because I
wanted to make sure Gun's poor performance was not negatively
impacting results (this bit me earlier). Some tests use it,
some don't, we should keep both for good measure.
WIP: Other minor todos.
essen
force-pushed
the
direct-data_delivery-for-h2-websocket
branch
from
cb5efcb to
316a9d7
Compare
essen
commented
Aug 27, 2025
| %% - The relaying state is used by extended CONNECT protocols to | ||
| %% use a 'relay' data_delivery method. | ||
| %% - The stopping state indicates the stream used the 'stop' command. | ||
| %% @todo Do we need to buffer data before the response is sent? Probably. |
There was a problem hiding this comment.
I don't know if we must do this now but at least the location of this todo is wrong.
essen
commented
Aug 27, 2025
| data_delivery_flow => maps:get(data_delivery_flow, Opts, 131072) | ||
| }, | ||
| Pid ! {{Pid, StreamID}, {switch_protocol, Headers, ?MODULE, ModState}}, | ||
| %% @todo We can't call the normal takeover because it tries to parse. |
There was a problem hiding this comment.
I mean, we can, the buffer is empty anyway. Remove the comment or clarify it to indicate that attempting to parse is not useful.
essen
commented
Aug 27, 2025
| %% Unfortunately we cannot currently cancel read_body. | ||
| %% But that's OK, we will just stop reading the body | ||
| %% after the next message. | ||
| %% @todo We can't stop relay data_delivery currently. |
There was a problem hiding this comment.
Stop of reads comes from:
- Implement a Websocket backpressure mechanism #1054
- Handle credits/conserve resources rabbitmq/rabbitmq-web-stomp#40
But to make it work with this feature we need a way to signal that we don't want to continue reading data from the stream.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The current implementation moves the Websocket data through stream handlers before the Websocket process receives it and parses it. The new mechanism relays the data directly as an Erlang message without any intermediary. It also manages HTTP/2 flow control in a simpler way (it maintains it at a configured value).
A new switch_protocol option (coming from the options returned from init/2) called
data_deliverydecides which mechanism to use:stream_handlersis the old behavior;relaythe new.and don't increase too much. The default flow control value
will depend on that. The value must be configurable.
wanted to make sure Gun's poor performance was not negatively
impacting results (this bit me earlier). Some tests use it,
some don't, we should keep both for good measure.