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

Don't compress empty or small messages #6832

Closed
wants to merge 1 commit into from

Conversation

jroper
Copy link
Contributor

@jroper jroper commented Dec 3, 2023

Fixes #6831.

gzip compressing messages less than 21 bytes is always unfruitful, it always results in "compressed" messages that are longer than the original message length, since the minimum length of a gzipped message is 21 bytes, due to headers and CRC trailer.

Fixes grpc#6831.

gzip compressing messages less than 21 bytes is always unfruitful, it always
results in "compressed" messages that are longer than the original message
length, since the minimum length of a gzipped message is 21 bytes, due to
headers and CRC trailer.
@jroper
Copy link
Contributor Author

jroper commented Dec 3, 2023

I can see I've broken the tests, because the tests expect empty messages to be compressed. Before I fix the tests (to assert that empty messages aren't encrypted, and to assert that messages that are greater than 21 bytes are encrypted), I'd appreciate feedback on whether this PR is likely to be accepted.

@dfawley
Copy link
Member

dfawley commented Dec 8, 2023

In light of the comments in #6831, I'll close this. Feel free to reopen or send a new PR when the changes are ready for the alternate approach.

@dfawley dfawley closed this Dec 8, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 6, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Compression is applied to empty messages, which makes them bigger, not smaller
2 participants