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:
| - 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.
There was a problem hiding this comment.
Good catch. This needs to be specified in protocol.md. It currently says that servers should not do that. I'll suggest a change.
There was a problem hiding this comment.
Oops I had updated the RFC here but forgot to update the protocol.md PR, updated it
There was a problem hiding this comment.
Just to clarify, I'm not sure if they should do this behavior. But the current connect-go does, and I wanted clarification so implementations are consistent.
…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)
This PR moves codec and compressor configuration from additive options to authoritative slices (`WithCodecs` and `WithCompressors`). This resolves a few historical API limitations and lets the caller state the preference order directly. Compression negotiation is unchanged. The server still responds with the client's first supported entry, matching v1. The recent spec update connectrpc/connectrpc.com#405 would let the server choose the response encoding instead, which matters because browsers hardcode `Accept-Encoding: gzip, deflate, br, zstd` and can never negotiate past `gzip` under the current rule. That spec change isn't ready for v2, so it's left out here. The ordered slices are the piece it needs, so it can land on top later without another API break. Ordering was the underlying problem. Additive options made the preference order implicit in registration order, and the most recently registered compressor was the most preferred, which reads backwards. `WithCompressors` and `WithCodecs` now replace the defaults, and the order you provide in the slice is exactly what gets advertised. It applies to both codecs and compressors for consistency and matches the pattern in [connect-py](https://github.com/connectrpc/connect-py). This simplifies the option API surface. Removes `WithAcceptCompression`, registered compressors are now always advertised, and removes `WithNoCompression`, an empty `WithCompressors()` disables compressors. Migration tool has been updated to warn on `connect.WithCodec`, `WithCompression`, and `WithAcceptCompression` options. These aren't mechanical translations and require review. For example, the conformance tests set a custom JSON codec for a stricter encoding and has been updated. ### Examples Compression preference in order: ```go // Prefer brotli, fall back to gzip. Sends Accept-Encoding: br,gzip connecthttp.WithCompressors(brotli, connectgzip.New()) ``` Removing the default JSON codec was previously impossible: ```go // Serve binary Protobuf only. connecthttp.Mount(mux, srv, connecthttp.WithCodecs(connectproto.NewBinaryCodec())) ``` Client prefer the first registered codec, so looses the need to always set these two options: ```go // before connecthttp.WithCodec(jsonCodec), connecthttp.WithSendCodec(jsonCodec.Name()) // after connecthttp.WithCodecs(jsonCodec) ``` --------- Signed-off-by: Edward McFarlane <3036610+emcfarlane@users.noreply.github.com>
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