Skip to content

Do not disable gzip compression for nginx completely#386

Merged
MorrisJobke merged 1 commit into
nextcloud:masterfrom
benediktg:enable-compression-nginx
May 14, 2017
Merged

Do not disable gzip compression for nginx completely#386
MorrisJobke merged 1 commit into
nextcloud:masterfrom
benediktg:enable-compression-nginx

Conversation

@benediktg

Copy link
Copy Markdown
Member

see also issue #325

@benediktg

Copy link
Copy Markdown
Member Author

Maybe somebody could remove irrelevant MIME types from the gzip_types directive?

@benediktg benediktg requested review from JSoko and removed request for JSoko March 29, 2017 11:21
@MorrisJobke

Copy link
Copy Markdown
Member

I just tried this with the latest master and I could not find a request where this is valid, because we send an etag with every request somehow. So the gzip was never enabled. On the other side we enabled gzip for the static assets in the server itself.

I think this can be closed then.

@MorrisJobke MorrisJobke closed this May 9, 2017
@MorrisJobke

Copy link
Copy Markdown
Member

On the other side we enabled gzip for the static assets in the server itself.

See nextcloud/server#4070

@benediktg

benediktg commented May 9, 2017

Copy link
Copy Markdown
Member Author

Enabling gzip saves about 70 % data transfer and reduces the loading time by about 20 %:

screenshot_20170509_212601
screenshot_20170509_212619

(source)

@MorrisJobke

Copy link
Copy Markdown
Member

With current master?

@MorrisJobke

Copy link
Copy Markdown
Member

With current master?

Because we do the GZIP compression in master already. ;)

@benediktg

Copy link
Copy Markdown
Member Author

With current master?

With 11.0.3

@MorrisJobke

Copy link
Copy Markdown
Member

With 11.0.3

There you have it ;)

@MorrisJobke

Copy link
Copy Markdown
Member

With 11.0.3

Just upgrade to the 12 beta 2 😉

@benediktg

Copy link
Copy Markdown
Member Author

Here is the comparison with NC 12 beta 2:
screenshot_20170510_151323
screenshot_20170510_151340

(source)

@MorrisJobke MorrisJobke reopened this May 10, 2017
@ghost

ghost commented May 12, 2017

Copy link
Copy Markdown

I just performed some tests on Nextcloud 11.0.3 and as well on Nextcloud 12b2 ... Nextclouds performance significantly increased with NGINX 1.13 and ngx_cache_purge enabled. Thanks!!!

@MorrisJobke

Copy link
Copy Markdown
Member

Nextclouds performance significantly increased with NGINX 1.13 and ngx_cache_purge enabled. Thanks!!!

Could you show what you enabled so we can document it?

@ghost

ghost commented May 12, 2017

Copy link
Copy Markdown

don't get me wrong but all i configured is shown at https://www.c-rieger.de/nextcloud-installation-guide/
regarding gzip i first tried your recommendations (gzip off) and had iterations from

gzip on;
gzip_vary on;
gzip_types application/javascript application/x-javascript text/javascript text/xml text/css;

to

gzip on;
gzip_min_length 1024;
gzip_proxied expired no-cache no-store private auth;
gzip_types text/plain text/css text/xml text/javascript application/x-javascript application/json application/xml;
gzip_disable msie6;

to finally yours:

gzip on;
gzip_vary on;
gzip_comp_level 4;
gzip_min_length 256;
gzip_proxied expired no-cache no-store private no_last_modified no_etag auth;
gzip_types application/atom+xml application/javascript application/json application/ld+json application/manifest+json application/rss+xml application/vnd.geo+json application/vnd.ms-fontobject application/x-font-ttf application/x-web-app-manifest+json application/xhtml+xml application/xml font/opentype image/bmp image/svg+xml image/x-icon text/cache-manifest text/css text/plain text/vcard text/vnd.rim.location.xloc text/vtt text/x-component text/x-cross-domain-policy;

conclusion:
modifications to nginx, php, mysql and redis will result in a significant performance boost for Nextcloud, as well for uploads as for downloads.

@MorrisJobke

Copy link
Copy Markdown
Member

don't get me wrong but all i configured is shown at https://www.c-rieger.de/nextcloud-installation-guide/

Sorry :/ Didn't reminded that.

gzip_comp_level 4;
gzip_min_length 256;
gzip_proxied expired no-cache no-store private no_last_modified no_etag auth;
gzip_types application/atom+xml application/javascript application/json application/ld+json application/manifest+json application/rss+xml application/vnd.geo+json application/vnd.ms-fontobject application/x-font-ttf application/x-web-app-manifest+json application/xhtml+xml application/xml font/opentype image/bmp image/svg+xml image/x-icon text/cache-manifest text/css text/plain text/vcard text/vnd.rim.location.xloc text/vtt text/x-component text/x-cross-domain-policy;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@josh4trunks Any opinions on this? Looks good from my side.

@MorrisJobke

Copy link
Copy Markdown
Member

@benediktg Could you rebase this?

@LukasReschke Are you up to set this also up for apache? I guess it makes sense to gzip the static assets.

@josh4trunks

josh4trunks commented May 12, 2017

Copy link
Copy Markdown
Contributor

I personally have my Nextcloud instance behind a Varnish cache server that does all the gzipping, and gzip is disabled for Nginx. There I gzip all "(text|application)/"

Is it posible to have gzip_types be something like application/* text/*? Otherwise this is probably fine, but I'll leave it to others to test.

###EDIT###
https://serverfault.com/questions/731018/nginx-gzip-text-seems-not-to-compress-text-javascript
I guess nginx isn't as regexy about this than varnish. so the huge block of gzip_types is the best option here I guess.

@benediktg

benediktg commented May 13, 2017 via email

Copy link
Copy Markdown
Member Author

@benediktg benediktg force-pushed the enable-compression-nginx branch from 22d9b39 to c3b9b13 Compare May 13, 2017 21:44
@benediktg

Copy link
Copy Markdown
Member Author

Do you think it would make sense to add

gzip_vary on;

? (i.e. add the Vary: Accept-Encoding header)

@MorrisJobke

Copy link
Copy Markdown
Member

Do you think it would make sense to add

Yep - seems that it allows to make this compatible with clients that don't support gzip.

@MorrisJobke

Copy link
Copy Markdown
Member

While working on this: Could you also add the same change to the subdir configuration below in the same file?

@benediktg benediktg force-pushed the enable-compression-nginx branch from c3b9b13 to 33c1100 Compare May 14, 2017 19:06
see also issue nextcloud#325

Signed-off-by: Benedikt Geissler <benedikt@g5r.eu>
@benediktg benediktg force-pushed the enable-compression-nginx branch from 33c1100 to 4077741 Compare May 14, 2017 20:14
@MorrisJobke MorrisJobke merged commit 71269c0 into nextcloud:master May 14, 2017
gzip off;
# Enable gzip but do not remove ETag headers
gzip on;
gzip_vary on;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@josh4trunks Forgot to ask you: Any opinion on this? Works here fine and seems to make sense from my point of view.

MorrisJobke added a commit that referenced this pull request May 14, 2017
* thanks to @benediktg
* see #386

Signed-off-by: Morris Jobke <hey@morrisjobke.de>
@MorrisJobke

MorrisJobke commented May 14, 2017

Copy link
Copy Markdown
Member

I added it to the release notes: f6497cf

@benediktg benediktg deleted the enable-compression-nginx branch May 15, 2017 00:26
@josh4trunks

josh4trunks commented May 15, 2017

Copy link
Copy Markdown
Contributor

gzip_vary looks good to me. As I said before, I do not personally use gzip with Nginx, I let an upstream Varnish Cache proxy do that for me so I have not dived deep into the various gzip options Nginx has.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants