-
-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Websocket compression for send_bytes on by default #2856
Comments
What is exact reason for dropping connections? Timeouts on sending/receiving data? If both client and server negotiates that they want and able to use compression -- why force disabling it? I don't see why aiohttp should make assumption about data content. Binary data could be random but very often they are structured and could be compressed. If you don't need a compression -- just disable it. The parameter's default value is kind of frozen. To don't make a mess we should support a consistency for all aiohttp 3.x releases. We can return to the question at aiohttp 4.0 development time but I still not convinced that we should. I see very many cases when compression is useful (maybe with lower throughput) |
Thanks for the quick reply.
Timeout on
Emphasis mine. If that is indeed the case, then just go ahead and close this issue. In my experience, with the data that I'm used to seeing, binary data is very often near-random (and therefore incompressible). Only rarely do I send structured binary data. I haven't done any studies of the usual structure (or non-structure) of binary data in HTTP-based applications. It may very well be that structured binary case is the common case (and that my case is rare).
That makes a lot of sense. Let's see if others are having the same issue that I had. If not, just keep things as they are. Closing this for now. |
@asvetlov I've noticed that aiohttp uses the default level for zlib.compressobj, which is 6. I think that is too large for a transport compression. We should set it to 1 (Z_BEST_SPEED). Setting the compress level to Z_BEST_SPEED improves nearly 100% performance for common text data like JSON, with only 2% larger result. There are a lot of binary data that are already compressed (e.g. JPEG images, MP4 videos, MP3 audio, ZIP archives, etc). Processing such data causes a lot of CPU time, and user should disable compression explicitly (even Z_BEST_SPEED is too slow) |
What https://tools.ietf.org/html/rfc7692 recommends? |
Compressing already compressed data is pointless, you are right. |
@asvetlov Yes you are right. Users are responsible for these situations. RFC said nothing about compress level, I think it is just an implementation detail, because it is transparent to the decompression side. |
Nginx gzip module uses 1 by default. |
Level 1 makes sense. Does anybody want to make a pull request? |
This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a [new issue] for related bugs. |
Long story short
I recently upgraded from Aiohttp 2.2.5 to 3.0.6. After that, I got a lot of dropped websocket connections. Through investigation, I found that 2.3.0 introduced the
compression=True
parameter for websockets. It turns out, that this change caused the dropped connections. Simply settingcompression=False
(and thereby returning to the pre-2.3.0 behaviour) solved the issue.I can understand that having compression on by default makes a lot of sense for text messages (e.g.,
send_str
orsend_json
messages). We can naturally assume statistical redundancy for text messages.However, I find it strange to find compression on by default for binary messages (e.g.,
send_bytes
messages). I don't think we can assume anything about binary messages. Said messages may very well be near-random (and therefore compress very poorly).In my application, I send a lot of binary data over a websocket (roughly 500 KBps) that doesn't compress very well at all. The (needless) compression caused the python process take up all CPU resources and, ultimately, drop the connection due to timeout.
Expected behaviour
I expected
send_bytes
to have compression off by default.Actual behaviour
Since 2.3.0,
WebSocketResponse
has compression on by default. Since 3.0.0, it is possible to disable compression on a per-method basis (send_bytes(..., compression=False)
). However, compression is on by default, which I think is very questionable.The text was updated successfully, but these errors were encountered: