Page MenuHomePhabricator

Enable gzip compression for interface icon SVGs served by MediaWiki
Closed, ResolvedPublic

Description

This is becoming more visible with @Krinkle 's efforts to reduce CSS embed usage: T121730: Audit use of @embed and remove where not needed (2019)

SVGs that used to be embedded in gzipped CSS files are now becoming separate requests, where they aren't compressed anymore, despite being textual content:

https://en.wikipedia.org/w/skins/Vector/images/arrow-down.svg?2f021

content-length: 221
content-type: image/svg+xml

(and no content-encoding header to be found)

XML's repetitive nature means that we should always gain from compressing SVG files.

Details

Related Gerrit Patches:
operations/puppet : productionATS: lower compress plugin minimum-content-length
operations/puppet : productionDocument Apache gzip sidestepping
operations/puppet : productionGzip SVGs served by MediaWiki
operations/puppet : productionLower gzip threshold for SVGs served by MediaWiki

Event Timeline

Gilles created this task.Sep 11 2019, 1:59 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptSep 11 2019, 1:59 PM

Change 535860 had a related patch set uploaded (by Gilles; owner: Gilles):
[operations/puppet@production] Gzip SVGs served by MediaWiki

https://gerrit.wikimedia.org/r/535860

Nice catch. I could've sworn this was done already, but maybe it got lost somehow. Perhaps in the transition from bits to canonical domain, or when we introduced the static.php router.

For JS files, it does still apply gzip compression, e.g. https://en.wikipedia.org/w/skins/Vector/vector.js.

See also:

# Multiversion static files (T99096)
RewriteRule ^/w/skins/.*$ /w/static.php [PT]
RewriteRule ^/w/resources/.*$ /w/static.php [PT]
RewriteRule ^/w/extensions/.*$ /w/static.php [PT]
Gilles claimed this task.Sep 11 2019, 3:31 PM
Gilles triaged this task as High priority.

Reporting a discussion happened on IRC: the change looks good, but it seems that Varnish/ATS explicitly unsed the Accept-Encoding request header before calling mediawiki:

https://gerrit.wikimedia.org/r/#/q/I2e7576fa9b5e2fd6f154f1be0e51cec96aa966ca (ATS)
https://gerrit.wikimedia.org/r/#/c/operations/puppet/+/421267 (Varnish)

As far as I can understand if you want a change in external behavior you'll need to do a ATS/VCL config change (so ATS/Varnish will gzip specific contents before sending to the client). There is a do_gzip VCL setting that might be useful to check:

https://github.com/wikimedia/puppet/blob/da1b4c7429b861121392a63c20f96d434daea20d/modules/varnish/templates/vcl/wikimedia-common.inc.vcl.erb#L451-L454

Change 537974 had a related patch set uploaded (by Gilles; owner: Gilles):
[operations/puppet@production] Lower gzip threshold for SVGs served by MediaWiki

https://gerrit.wikimedia.org/r/537974

TODO: add comment in the Apache config code pointing to the VCL and explaining the context

Restricted Application added a project: Operations. · View Herald TranscriptSep 23 2019, 8:54 PM
Krinkle renamed this task from Apache configuration: SVGs served by MediaWiki aren't gzipped to Enable gzip compression for interface icon SVGs served by MediaWiki.Sep 23 2019, 8:54 PM

Change 537974 merged by Vgutierrez:
[operations/puppet@production] Lower gzip threshold for SVGs served by MediaWiki

https://gerrit.wikimedia.org/r/537974

Change 535860 abandoned by Gilles:
Gzip SVGs served by MediaWiki

https://gerrit.wikimedia.org/r/535860

Change 539842 had a related patch set uploaded (by Gilles; owner: Gilles):
[operations/puppet@production] Document Apache gzip sidestepping

https://gerrit.wikimedia.org/r/539842

alaa_wmde added a subscriber: alaa_wmde.EditedSep 30 2019, 10:43 AM

hey @Gilles

We encountered an incident related to this change.
Described in comment T234183#5533440. I suspect either there's something to fix here still, or that there's just a wrong cached version of the svgs in varnish.

Indeed, it seems like Varnish claims that the content is gzipped but it actually isn't.

I think this is a Varnish bug or limitation, because if I manually cachebust the URL, the response is correctly gzipped: https://test.m.wikidata.org/w/extensions/Wikibase/view/lib/wikibase-termbox/assets/pen.svg?thisisnew

It seems like this object was cached uncompressed before the change, and that Varnish serves it as-is now, with the compressed headers, because in the new logic these objects are compressed.

Varnish should check that when do_gzip is enabled, the object already stored is indeed gzipped, but it doesn't. Or, if it decides to serve the uncompressed copy it already has, it shouldn't apply the gzip headers.

@Vgutierrez is there a simple way to purge all responses with content-type image/svg+xml from the Varnish text cluster? Is it worth reporting an upstream bug to Varnish?

@Gilles, yeah, with varnishadm ban like it's documented on https://wikitech.wikimedia.org/wiki/Varnish#One-off_purges_(bans) but I think that we should purge the responses with content-type image/svg+xml and content-length < 860, cause for bigger images do_gzip was already enabled.

Yep, exactly. In fact you can go as far as only banning content-length between 150 and 860 (inclusive).

@Vgutierrez purged SVGs whose content-length is > 100 and <= 899 in both cache layers for text and upload.

Actually I'm missing the backend layer on the upload cluster.. it's powered by ATS and the procedure is different

Change 542996 had a related patch set uploaded (by Ema; owner: Ema):
[operations/puppet@production] ATS: lower compress plugin minimum-content-length

https://gerrit.wikimedia.org/r/542996

ema moved this task from Triage to Caching on the Traffic board.Oct 14 2019, 6:20 PM

Change 542996 abandoned by Ema:
ATS: lower compress plugin minimum-content-length

Reason:
Plugin disabled altogether for now. It's unusable as it is, see https://gerrit.wikimedia.org/r/#/c/operations/puppet/ /559053/

https://gerrit.wikimedia.org/r/542996

Krinkle closed this task as Resolved.Dec 20 2019, 4:19 PM
Krinkle edited projects, added Performance-Team-publish; removed Patch-For-Review.