Increase gzip level and types#1329
Conversation
| gzip_http_version 1.1; | ||
| gzip_types text/plain text/css application/json application/x-javascript application/javascript text/xml text/csv; | ||
| gzip_types text/plain text/css application/json application/geo+json application/x-javascript application/javascript text/xml text/csv image/svg+xml; | ||
| gzip_proxied any; |
There was a problem hiding this comment.
What does this change? Requests from central seem to already be gzipped.
There was a problem hiding this comment.
Wasn't sure if data coming from service and enketo were gzipped. This seemed like a no-cost way to guarantee that.
There was a problem hiding this comment.
Is there something in front of test.getodk.cloud which could be zipping things?
There was a problem hiding this comment.
Oh, I thought this was downstream. Upstream, if you are running a proxy or load-balanced in front of Central that might zip things? I'm happy to drop gzip_proxied. I agree it's not necessary.
There was a problem hiding this comment.
I think it's potentially a good addition, I'm just trying to understand why it doesn't appear to be necessary. Potentially all zipping could be deferred to whatever is zipping downstream...
There was a problem hiding this comment.
Any reason why it shouldn't be added? Seems like the only risk seems to be double-compression (unlikely because most modern proxies respect Content-Encoding: gzip) and maybe minor increase in nginx CPU.
|
I'm going to add this PR to the project, since I'll probably mention it in the detailed release notes. |
|
Just wanted to check about the status of this PR. @yanokwa, is this PR ready to be merged, or are you still waiting for feedback? |
|
Please merge! |
gzip_comp_level of 4-6 seems to be the sweet spot for compression and CPU usage. We don't often see huge CPU usage on servers so 5 seems pretty safe.
gzip_proxied makes sure to gzip anything coming from upstream (reverse proxy mode)
gzip_min_length 1024 is a common default. 1280 suggests we've profiled the responses and we are making a sensible choice, but we haven't. Might as well use the common default.
I've added geojson and svg because we'll be serving those in the future.