Page MenuHomePhabricator

CentralNotice: Review and update Varnish caching for Special:BannerLoader
Open, MediumPublic2 Estimated Story Points

Description

The Varnish caching of banners served via Special:BannerLoader is "hit-for-pass", which means cachability of an item isn't known until it's actually served up by the MW backend. Cache header values are set like so, in SpecialBannerLoader.

We should review this setup to see if it's currently optimal. Review should include any relevant vlc (varnish setup) code that comes into play, too.

As part of this update, we might well turn off caching for output for at caught errors.

Event Timeline

AndyRussG added a subscriber: aaron.

Copying some relevant comments by @aaron in T144952:

[...] To cover 11+ seconds of lag, MediaWiki lowers the CDN TTL to $wgCdnMaxageLagged when lag is 6+ seconds high. However, I see raw header() calls in the CN codebase...not sure how that interacts with this logic in MediaWiki::preOutputCommit.

The first approach might work using Varnish xkey support. I'm not how far along we are with using that in production though (AFAIK it's enabled).

Another idea is to add a cache-busting parameter to the URLs handed out, like the cache hash or a check key value.

The first approach might work using Varnish xkey support. I'm not how far along we are with using that in production though (AFAIK it's enabled).

Not available on the text cluster yet: T131503: Convert text cluster to Varnish 4 It's a goal for the Traffic team this quarter, afaik.

ggellerman triaged this task as Medium priority.Nov 8 2016, 10:57 PM
ggellerman moved this task from Triage to Q1 FY21-22 on the Fundraising-Backlog board.

The description and comments don't make a lot of sense from my outside (of CN/Banner code) POV. Are we talking about what the code does currently, or what it will do in some future release? Why would it be hit-for-pass if it's giving a CC header that allows caching? Why is cacheability different for logged-in and logged-out? Is it Vary-ing on Cookie if it really does need to be different for logged-in?

Another idea is to add a cache-busting parameter to the URLs handed out, like the cache hash or a check key value.

Great idea! We should definitely look into this!!!

The description and comments don't make a lot of sense from my outside (of CN/Banner code) POV.

Quite likely, I got a lot of stuff wrong. I guess I'm not even sure that "hit-for-pass" is what it really is...

Are we talking about what the code does currently, or what it will do in some future release?

I'd like to talk about how things work now, i.e., CN header-setting code as interacts w/ Varnish code, whether the effect all that has is what's desired, or needed, and how stuff might change.

Why would it be hit-for-pass if it's giving a CC header that allows caching? Why is cacheability different for logged-in and logged-out? Is it Vary-ing on Cookie if it really does need to be different for logged-in?

Yeah this is one thing we need to look into, especially check the use cases for. Maybe this is just legacy code that by default sent logged-in users directly to PHP. Hopefully we could start caching for them... :)

While it sure wouldn't hurt to look into this soon, a more pressing concern is the related but more-limited-in-scope T151418.

Thanks much, everyone!!

Are we still working on something here, or is this best closed and any remaining concerns opened in a fresh ticket? No real commentary in 4 years.