-
Notifications
You must be signed in to change notification settings - Fork 1.2k
tonic: add compression threshold for message encoding #2404
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
base: v0.14.x
Are you sure you want to change the base?
tonic: add compression threshold for message encoding #2404
Conversation
76a268b to
449c7cc
Compare
Messages smaller than 1024 bytes are no longer compressed even when compression is enabled. This optimization avoids the overhead of compression for small messages where the compression ratio would be minimal or potentially increase the message size. The threshold is set to 1024 bytes (matching UNCOMPRESSED_MIN_BODY_SIZE used in tests). Messages below this threshold are sent uncompressed with the compression flag unset in the gRPC header.
449c7cc to
7d51ce6
Compare
When compressing large payloads we use spawn_blocking to move the job to a tread to not block the async worker thread
5aa700e to
14a3a64
Compare
14a3a64 to
472812a
Compare
|
@nossrannug hi, sorry for the delay on this. Do you have any background on if this is something other implementations of gRPC do? I'd ideally not like to diverge to much from them. |
|
This is not a feature any other gRPC library offers. It has been discussed, but never implemented. We do provide the ability to selectively compress messages on the stream -- except for Go, which is missing it. We also provide the ability to control compression on a per-stream basis in all languages. I wouldn't be opposed to Tonic having this feature, but the new grpc-rust channel will not provide it unless it's a feature that goes through the gRFC process (see https://github.com/grpc/proposal). |
|
@LucioFranco , thanks for the comment. I didn't check how others do this before making this change. I just thought of the way Starlette does the gzip middleware and went with that assuming that was the way 😄 The main driver behind this PR is that I have a server stream rpc that first sends a large state to clients and then after will send individual messages (these are very small) with incremental updates as the state changes and also occasional sends empty messages. I did just now check and the C++ core library will always compress a message (when client accepts compression) and discard the output if it's larger than the input to send the input uncompressed. And the server implementation controls the per message compression by using WriteOptions. class MyServiceImpl final : public ExampleService::Service {
public:
Status StreamThings(ServerContext* context,
const StreamRequest* request,
ServerWriter<StreamReply>* writer) override {
StreamReply reply;
// Send first message compressed (default)
reply.set_message("large_payload_data");
writer->Write(reply); // normal compression applies
// Send second message uncompressed
reply.set_message("small_ping");
WriteOptions opts;
opts.set_no_compression(); // disable compression for this message only
writer->Write(reply, opts);
return Status::OK;
}
};If we want to follow the cpp implementation I can update the PR to:
Does that sounds as a reasonable plan? |
|
@dfawley , just now noticed your comment. Thanks for joining the conversation. I'd be more than happy to take this through all the right channels. I'll take a look at https://github.com/grpc/proposal and see if I can't get the ball rolling. |
|
@nossrannug if you are going to broach this issue, then be aware that there has been prior discussion on the topic. There is probably some appetite for a feature like this, but there might be folks that want it to be more complicated -- or be extensible so it could become more complicated later -- than simply using the size of the message to be sent. There are probably several other things to debate, like whether it's a client/server setting or whether it should be part of the service config (which tonic doesn't support), whether it should be on by default, etc. In the meantime, I think if you can find a way to add per-message configuration of compression in tonic, that would be ideal as it would match the other grpc implementations. |
Add configurable compression thresholds for Tonic
This PR adds runtime-configurable compression thresholds to improve performance for large message compression scenarios.
Changes:
TONIC_COMPRESSION_THRESHOLD(default 1024 bytes) are sent uncompressedTONIC_SPAWN_BLOCKING_THRESHOLDcan be compressed on a blocking thread pool to avoid blocking the async runtimeConfiguration:
Set via environment variables:
Benefits:
Implementation notes: