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

Migrate gzip compression from nginx to tower-http::compression #7330

Merged
merged 2 commits into from
Oct 21, 2023

Conversation

Turbo87
Copy link
Member

@Turbo87 Turbo87 commented Oct 20, 2023

see https://docs.rs/tower-http/latest/tower_http/compression/index.html

This is using slightly different settings than with nginx, but most of them seem reasonable:

  • nginx defaults to 20 bytes, we explicitly set it to 512 bytes for undocumented reasons, tower-http defaults to 32 bytes. using the defaults here appears to be fine on first glance. if we see problems with this we can still change this to use the old 512 bytes setting.
  • nginx uses a MIME type allow list list, while tower-http uses a deny list, which includes by default all images except for SVGs. since we don't use any exotic MIME types (crate files come from S3 directly) this should be fine.
  • nginx only supports gzip compression, tower-http enables brotli, deflate, gzip and zstd 🎉
  • tower-http uses the "default compression level set by the compression algorithm" by default, but since we're in a server setting we would like to respond as quick as possible. for static assets we can consider pre-compressing them on disk, but that is out of scope for this pull request (see Precompress static frontend assets with gzip and brotli #7332).

Note that I've successfully tested this out on our staging environment at https://staging-crates-io.herokuapp.com/. Interestingly, brotli compression worked for https://staging-crates-io.herokuapp.com/, but not when accessed via https://staging.crates.io/. My assumption is that brotli compression is currently disabled on CloudFront for some reason and it is re-encoding the responses with gzip. I will talk to the infra team about this next week, but I don't consider it a blocking concern.

This adds support for brotli, deflate, gzip and zstd compression of response bodies to the app.
We don't need this anymore as it is handled by the axum server now.
@Turbo87 Turbo87 added C-enhancement ✨ Category: Adding new behavior or a change to the way an existing feature works A-backend ⚙️ labels Oct 20, 2023
@Turbo87 Turbo87 merged commit 5c1ef69 into rust-lang:main Oct 21, 2023
7 checks passed
@Turbo87 Turbo87 deleted the tower-compression branch October 21, 2023 06:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-backend ⚙️ C-enhancement ✨ Category: Adding new behavior or a change to the way an existing feature works
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants