Page MenuHomePhabricator

Improve handling of mobile variants in Varnish
Closed, ResolvedPublic


Our current strategy for varying the cache on cookie values is to check the cookie header for specific cookies which we know are significant to the final output. When the cookie header contains a cache-relevant cookie, we treat the request as un-cacheable.

On desktop, only token or session cookies are considered relevant. We explicitly don't want to cache logged-in page views, so this is OK.

On mobile, the situation is different. We have two additional cache-relevant cookies — disableImages and optin. Both of these have a small set of possible values (disableImages may be "1" or blank; optin may be "alpha", "beta" or "stable"). Both of these cookies represent preferences that can be set by anonymous users. Anon requests that have one or both of these cookies set can and should enjoy a high cache hit-rate, but at the moment they bypass the cache entirely. This is especially regrettable in the case of disableImages, which is presumably set by bandwidth- and latency-conscious users.

The way we handle these cookies (and soon also the NetSpeed cookie, proposed in T119797) should be different from the way we handle session and token cookies. Requests which set one or more of these cookies should be looked up in the cache.

Event Timeline

ori raised the priority of this task from to Needs Triage.
ori updated the task description. (Show Details)
ori added subscribers: ori, BBlack, faidon.
ori added a subscriber: Tnegrin.
fgiunchedi triaged this task as Medium priority.Dec 7 2015, 2:44 PM
fgiunchedi edited projects, added SRE, Traffic; removed Varnish.

Hi Adam -- we should review this "soon" as it potentially impacts users who are trying to increase page performance by not downloading images but have the additional undesirable effect of disabling the cache.



Even more amazing: when the 'disableImages' cookie is unset, it is not actually cleared; instead it is simply set to a blank value. This means that changing the preference either way condemns the user to 180 days of cache misses.

Change 257375 had a related patch set uploaded (by Ori.livneh):
Improve disableImages cookie code

Gents, just acknowledging I saw this task. I'll tag this with Reading-Admin. @JKatzWMF, @Jdlrobson is this eligible for the current sprint or does it need to wait until the next sprint?

@Yurik, we'll need to analyze the ZeroOpts cookie. Gut feeling is it isn't necessary anymore, but we should verify and also discuss with @DFoy in case any changes are needed in the actual per-operator configs themselves.

I think the disableImages feature is a misfeature, at the end of the day.

(a) It never worked properly.
(b) Every modern graphical browser can disable images.

@dr0ptp4kt I think this needs to wait at least until next sprint, as we have a full load and it sounds like there are still some unknowns/blockers

Ugh, ignore ^. In a rush and thought this was something else. Do we know how many users this impacts?

If people turn it on, then off, will we be able to count them?

Change 257375 merged by jenkins-bot:
Improve disableImages cookie code

Change 257486 had a related patch set uploaded (by Ori.livneh):
Improve disableImages cookie code

Change 257486 merged by jenkins-bot:
Improve disableImages cookie code

Change 257491 had a related patch set uploaded (by Ori.livneh):
Improve handling of disableImages cookie

ori claimed this task.


Change 257491 abandoned by BBlack:
Improve handling of disableImages cookie

Already done in I686de38fee