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

Enable per message deflate on server #8314

Merged

Conversation

pradeepvairamani
Copy link
Contributor

@pradeepvairamani pradeepvairamani commented Nov 17, 2021

permessage-deflate extension enables the client and server to negotiate a compression algorithm and its parameters, and then selectively applies it to the data payloads of each WebSocket message.

permessage-deflate header is used in the handshake to indicate whether a connection should use compression.

  1. When a client sends a websocket request, it send's permessage-deflate in the websocket extensions header if the client browser supports it. The server knows if a client supports compression based on this header.
  2. If the server decides to use compression, it responds with the same header similar to an ACK message. The client after receiving the response decides whether or not to compress the data based on the server's response.

Verified that the appropriate headers are being sent back:
image

By default, the threshold for compression is set to 1024 (in bytes). Messages with size less than this will not be compressed.

@github-actions github-actions bot added the area: server Server related issues (routerlicious) label Nov 17, 2021
@@ -119,7 +119,8 @@ export function create(
const io = new Server(server, {
// enable compatibility with socket.io v2 clients
allowEIO3: true,
perMessageDeflate: false,
// Indicates whether a connection should use compression
perMessageDeflate: true,
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it possible that this setting can cause a spike in memory as we suspected last time we turned it off? If yes, do we want to make it configurable through config.json, so it's easier to disable that if needed during an emergency?

Copy link
Contributor

Choose a reason for hiding this comment

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

I have the same concern. There can be a CPU and memory spike (gorilla/websocket#203).

@GaryWilber Does push enable it?

Copy link
Contributor Author

@pradeepvairamani pradeepvairamani Nov 18, 2021

Choose a reason for hiding this comment

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

Yes, I saw Henrique's comment after merging the code. Will add the config as he suggested. Push is not enabling it because AFD does not support this extension. With respect to the memory and cpu consumption, pasting the offline conversation I had with Henrique:

It is a valid point but I do not think we will get affected by it for a few reasons:

The theory that this flag was causing the memory issues is probably not true. The default for this flag was always false..I just added this line explicitly to make doubly sure.

This comes into play only for socket payloads that are more than 1mb..that that does not happen in most cases for us

I stress tested it pretty extensively and did not see any difference in the memory or cpu usage

@pradeepvairamani pradeepvairamani merged commit 33b4533 into microsoft:main Nov 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: server Server related issues (routerlicious)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants