RFC 008: Server compression preference - #405
Conversation
|
@anuraaga is attempting to deploy a commit to the connectrpc Team on Vercel. A member of the Team first needs to authorize it. |
Signed-off-by: Anuraag Agrawal <anuraaga@gmail.com>
a51e222 to
62de975
Compare
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Signed-off-by: Anuraag Agrawal <anuraaga@gmail.com>
Signed-off-by: Anuraag Agrawal <anuraaga@gmail.com>
Co-authored-by: Nick Snyder <nickdsnyder@gmail.com> Signed-off-by: Anuraag (Rag) Agrawal <anuraaga@gmail.com>
Signed-off-by: Anuraag Agrawal <anuraaga@gmail.com>
timostamm
left a comment
timostamm
left a comment
There was a problem hiding this comment.
Reading through protocol.md, I noticed that Unary-Get-Request still asserts the previous behavior, and I believe that it needs to change, following that same rationale (see the exact location in #322 (review)).
Two more suggestions for this RFC:
| connect-web users to use the more modern and efficient `br` and `zstd` algorithms. | ||
| For example, given `Accept-Encoding: gzip, br, zstd` | ||
| - If the server is configured with `gzip, br, zstd`, then `gzip` is selected. | ||
| - If the server is configured with `br, zstd, gzip`, then `br` is selected. |
There was a problem hiding this comment.
Accept-Encoding: gzip, br, zstd is a precise example for what happens on the wire. I think it's confusing to use the same presentation for the server configuration. I suggest to use pseudo-code for the examples, e.g.
- If the server is configured with
compression = [gzip, br, zstd], thengzipis selected.- If the server is configured with
compression = [br, zstd, gzip], thenbris selected.
There was a problem hiding this comment.
I don't think the compression pseudo variable helps that much, I proposed an alternate wording in another comment.
| `gzip` now, it can't be confirmed all workloads will see no regression from a change from | ||
| `gzip` to e.g., `br`. In an abundance of caution, if this RFC is approved, then this new behavior will only be introduced in Connect implementations as part of a major version. It is planned to be introduced to Connect-Go in v2, Connect-Py before v1, and Connect-ES | ||
| in v3. | ||
|
|
There was a problem hiding this comment.
I suggest that we clarify that this change applies not only to unary RPCs, but also to streaming RPCs and unary GET, and that it applies to all three protocols if an implementation supports them, not only to the Connect protocol.
There was a problem hiding this comment.
Good point - I have added a note about it in 31e708f
| connect-web users to use the more modern and efficient `br` and `zstd` algorithms. | ||
| For example, given `Accept-Encoding: gzip, br, zstd` | ||
| - If the server is configured with `gzip, br, zstd`, then `gzip` is selected. | ||
| - If the server is configured with `br, zstd, gzip`, then `br` is selected. |
There was a problem hiding this comment.
I don't think the compression pseudo variable helps that much, I proposed an alternate wording in another comment.
| - The list of supported compression methods configured on the server | ||
| - The list of supported compression methods in the client's `Accept-Encoding` header, e.g., `gzip, br, zstd` | ||
|
|
||
| Currently, it is the order in the `Accept-Encoding` header that determines which | ||
| algorithm to prefer. The server chooses the first compression method that it supports. |
There was a problem hiding this comment.
In connect-go the response compression is chosen to match the request: https://github.com/connectrpc/connect-go/blob/51112608939254772c8e67577eb5b353741aaa7e/protocol.go#L308-L311
This seems to not be defined in the Spec.
There was a problem hiding this comment.
Thanks for pointing this out! It seems most intuitive to match the request when the request is compressed. Connect-Py currently does not, since I think I was reading the spec closely when implementing that.
This seems like the right time to consolidate this behavior. I have added a note to use the request compression when provided.
…eference.md Co-authored-by: Nick Snyder <nickdsnyder@gmail.com> Signed-off-by: Anuraag (Rag) Agrawal <anuraaga@gmail.com>
…eference.md Co-authored-by: Edward McFarlane <3036610+emcfarlane@users.noreply.github.com> Signed-off-by: Anuraag (Rag) Agrawal <anuraaga@gmail.com>
…nto rfc-server-compression
…connectrpc.com into rfc-server-compression
Signed-off-by: Anuraag Agrawal <anuraaga@gmail.com>
Signed-off-by: Anuraag Agrawal <anuraaga@gmail.com>
| unlocking them for connect-web. Servers like Envoy and NGINX behave in the same way for the | ||
| same reason. | ||
|
|
||
| While `br` and `zstd` have come a long way and commonly have similar CPU and RAM usage to |
There was a problem hiding this comment.
There's something to mention here in regard to Brotli. The official reference library defaults to a compression level of 11 (out of 11). That's level uses A LOT of CPU. And so little-by-little languages are starting to switch the default, but it seems like a lot have not gotten the memo:
- node.js:
node:zlib: 11 - Python:
brotli: 11 - Kotlin:
brotli4j: 11 - dart: 11
- go
andybalholm/brotli: 6 - go
google/brotli: 11 - swift
Compression: 2 (this is an outlier in the other direction. I think it's because of mobile device battery life. https://developer.apple.com/documentation/compression/algorithm/brotli)
And with load balancers, almost none of them use the high default:
- nginx
ngx_protli: 6 - Cloudflare: 4 (https://blog.cloudflare.com/this-is-brotli-from-origin/)
- Envoy: 3
- Caddy: 4
I don't think there's anything to do here immediately, but I wanted this to be known about. Defaults are super important for things like this. I suspect documentation of the server compression priority for each language is where this is useful.
From my experiments, brotli compression at level 11 uses so much CPU that the expected RPS for a typical app service will plummet.
While
brandzstdhave come a long way and commonly have similar CPU and RAM usage togzipnow
when configured appropriately.
There was a problem hiding this comment.
Yup we aligned with Envoy in connect-py
I'm not going to complicate the prose for that here though. If we want to add compression levels to the spec, probably another PR that doesn't necessarily need an RFC (maybe)
5 participants
Introduce RFC 008, which proposes to move compression preference from client to server.
This allows using modern compression methods with connect-web and follows the general ecosystem.
The concrete changes proposed are in #322