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

Max message size #73

Closed
dbrgn opened this issue Apr 9, 2018 · 7 comments
Closed

Max message size #73

dbrgn opened this issue Apr 9, 2018 · 7 comments
Assignees

Comments

@dbrgn
Copy link
Member

dbrgn commented Apr 9, 2018

Currently when sending 4 MiB messages over the websocket, the connection is closed with status code 1009.

If you take a look at the WS server docs, you can see that the limit is 1 MiB by default.

The max_size parameter enforces the maximum size for incoming messages in bytes. The default value is 1MB. None disables the limit. If a message larger than the maximum size is received, recv() will raise ConnectionClosed and the connection will be closed with status code 1009.

The max_queue parameter sets the maximum length of the queue that holds incoming messages. The default value is 32. 0 disables the limit. Messages are added to an in-memory queue when they’re received; then recv() pops from that queue. In order to prevent excessive memory consumption when messages are received faster than they can be processed, the queue must be bounded. If the queue fills up, the protocol stops processing incoming data until recv() is called. In this situation, various receive buffers (at least in asyncio and in the OS) will fill up, then the TCP receive window will shrink, slowing down transmission to avoid packet loss.

Since Python can use up to 4 bytes of memory to represent a single character, each websocket connection may use up to 4 * max_size * max_queue bytes of memory to store incoming messages. By default, this is 128MB. You may want to lower the limits, depending on your application’s requirements.

I would suggest to document this limit, and to make the two values configurable by the user, since the limits should depend on the use case. For the WebRTC Task, 1 MiB is more than enough. For Threema Web + Relayed Data, we'd like to support messages up to maybe 50 MiB.

The problem is that according to the docs above, 50 MiB limits @ queue size 32 might result in memory use up to 6.4 GiB per process (worst case).

Chunking might be a possibility, but it's a bit stupid to do chunking on top of a chunked protocol.

@lgrahl RFC :)

@lgrahl
Copy link
Member

lgrahl commented Apr 9, 2018

I would suggest to document this limit, and to make the two values configurable by the user, since the limits should depend on the use case.

👍

For the WebRTC Task, 1 MiB is more than enough. For Threema Web + Relayed Data, we'd like to support messages up to maybe 50 MiB.

Don't, for several reasons:

  1. Streamed forwarding is not implemented, yet, see Streamed forwarding #72.
  2. Neither RTCDataChannel nor WebSocket have a streaming API.
  3. Encrypting large blocks will block the browser's UI thread for a longer period.

If you made a flow graph for this, there would be large gaps in multiple places significantly decreasing throughput.

Chunking might be a possibility, but it's a bit stupid to do chunking on top of a chunked protocol.

Instead, I would propose the following:

  1. Choose a sane chunk size (64 KiB isn't bad) for both WebSocket and RTCDataChannel and apply that to both.
  2. Chunk, then encrypt.
  3. For RTCDataChannel, register onbufferedamountlowthreshold, set bufferedAmountLowThreshold to 32 KiB and pause sending when reaching bufferedAmount 128 KiB. Continue sending when the event fires. Be aware that browsers currently lie when it comes to bufferedAmount, so under no circumstances should you do that in a loop. Always release the task (or microtask or whatever they call it) after a send call.
  4. Do the same for WebSocket but ughhhh... you have to periodically poll bufferedAmount when reaching bufferedAmount 128 KiB. Don't poll too slowly or the send window will decrease and that will result in bad throughput.

If you do that, you can go well beyond 50 MiB.

(This will be a lot less ugly once RTCDataChannel has a streaming API. The WHATWG people are interested in porting this to WebSocket as well.)

@dbrgn
Copy link
Member Author

dbrgn commented May 15, 2018

Do the same for WebSocket but ughhhh... you have to periodically poll bufferedAmount when reaching bufferedAmount 128 KiB. Don't poll too slowly or the send window will decrease and that will result in bad throughput.

Can you elaborate? Can't we just pass all the chunks to the websocket, and the socket should handle buffering?

@lgrahl
Copy link
Member

lgrahl commented May 15, 2018

No. If you do that the WebSocket will simply close when it exceeds some internal buffer threshold. It's a terrible API design:

if the data cannot be sent, e.g. because it would need to be buffered but the buffer is full, the user agent must flag the WebSocket as full and then close the WebSocket connection

https://html.spec.whatwg.org/multipage/web-sockets.html#dom-websocket-send

@dbrgn
Copy link
Member Author

dbrgn commented May 15, 2018

Ok. That's on the web side though. (I guess you're right about that one.)

In our case, the sending of the data from Rust happens inside the relayed data task. The outside code does not (and should not) have access to the internals of the WebSocket. Thus, no access to the buffered amount, unless chunking is part of the task. And I do not thing chunking belongs in the task (some applications might not need chunking at all if only small messages are exchanged).

I guess I should simply try it and see what happens.

@dbrgn
Copy link
Member Author

dbrgn commented May 15, 2018

Hm, actually I guess the task could handle buffering without caring about chunking...

@lgrahl
Copy link
Member

lgrahl commented May 15, 2018

You will have to have some API that exposes a way to control the flow of the stream. You could mimic the RTCDataChannel API design regarding buffering but that is clunky (not as clunky as WebSocket but still clunky). This might be the easiest way.

The ideal way would be to use the Streams API create a WritableStream instance and retrieving a ReadableStream instance per message. We need this to be per message since WebSocket and the relayed data task transfers messages and not streams. Modelling each message as a stream is an elegant way to map this.

@lgrahl
Copy link
Member

lgrahl commented Aug 30, 2018

Closed in favour of #72

@lgrahl lgrahl closed this as completed Aug 30, 2018
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

2 participants