Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adjust socket options (buffer sizes) from the websocket handler #1378

Open
larshesel opened this issue May 28, 2019 · 8 comments
Open

Adjust socket options (buffer sizes) from the websocket handler #1378

larshesel opened this issue May 28, 2019 · 8 comments

Comments

@larshesel
Copy link

This is a feature request.

For some of our uses cases it is important to be able to set various socket options from within a websocket handler. For example the desired sizes for some options like buffer, sndbuf, recbuf are not known when the handler is started but first later after this information has been retrieved externally after authentication.

For now the workaround exists to write a wrapper around cowboy_websocket which extracts the socket and adds it to the handler state.

@essen
Copy link
Member

essen commented May 28, 2019

To be implemented via the new set_options command.

@essen essen added this to the 2.7 milestone May 28, 2019
@essen
Copy link
Member

essen commented Oct 2, 2019

I need to hear more about this feature.

This makes sense for HTTP/1.1 Websocket because Websocket takes over the protocol and thus is in direct control of the socket.

This makes no sense for HTTP/2 Websocket because there Websocket is just a normal stream.

My question is how do you determine the right size for those options and can there be a more generic approach that would work both for HTTP/1.1 and HTTP/2 Websocket?

Thanks.

@larshesel
Copy link
Author

Sure - I'll try.

This is all in the context of VerneMQ and MQTT over websockets.

Often we can set some pre-defined (small) values for the sndbuf and recbuf sizes in order to not use all available memory when having thousands of MQTT devices over websockets - this is fine for most as they only send and receive small bits of data every so often. Then there are some specific MQTT clients which have much more traffic (usually due to a fan-in scenario) and hence benefits from larger values for sndbuf and recbuf. Usually we let this configuration for the specific clients come from an external source which is only queried after the MQTT websocket client has connected to VerneMQ and then VerneMQ will use check the credentials against and external database from which it may also retrieve the sndbuf and recbuf value overrides.

So I guess we don't really decide the values, i.e., we've outsourced the decision to those tuning the system.

Does this make sense?

@essen
Copy link
Member

essen commented Oct 2, 2019

Yeah. Thanks! It just doesn't fit with set_options though so I'll have to think of something else.

@essen essen removed this from the 2.7 milestone Oct 2, 2019
@larshesel
Copy link
Author

We worked around this by implementing a stream_handler and cowboy_sub_protocol (which you kindly suggested when we first discussed it on IRC) which inserts the socket into our websocket state - it would of course be nice to just be able to use set_options, but the current approach works perfectly. In other words, feel free to close this if this is a non-feature for you.

@essen
Copy link
Member

essen commented Oct 2, 2019

Let's keep it open, it's useful to have but it will have to be implemented a different way. It just doesn't fit set_options which is about setting protocol options. Maybe Cowboy can have a callback when switching the entire connection to a new protocol that could be used to set transport options, in which case you could just send the socket to yourself and receive it in your Websocket handler directly if you need to do operations later.

@dergraf
Copy link
Contributor

dergraf commented Nov 25, 2019

The proposed solution with a custom stream handler and implementing the cowboy_sub_protocol behaviour breaks a type contract when upgrading to Cowboy 2.7.
Apparently Dialyzer doesn't like the recent addition of the _ => _ to the cowboy_req:req() type.
See: https://travis-ci.org/vernemq/vernemq/jobs/616746997#L1061

@essen
Copy link
Member

essen commented Nov 25, 2019

There's probably a type error in cowboy_websocket that leads to Dialyzer only seeing the HTTP/2 case for websocket_handshake. Could you extract a module reproducing this behavior? For example when you put that module in the Cowboy source and run make dialyze.

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

No branches or pull requests

3 participants