Varnish does not cache Action API responses when logged in
Open, NormalPublic

Description

Steps to reproduce: send a cacheable Action API request (e.g. https://en.wikipedia.org/w/api.php?action=query&meta=siteinfo&uselang=content&smaxage=10) from a browser window where you are logged in as a user.
Expected result: x-cache-status header is hit or miss
Actual result: x-cache-status header is pass

$ curl -is 'https://en.wikipedia.org/w/api.php?action=query&meta=siteinfo&uselang=content&smaxage=10' -H 'Cookie: enwikiSession=<redacted>; enwikiUserID=20093189; enwikiUserName=Tgr+%28WMF%29; forceHTTPS=true; centralauth_User=Tgr+%28WMF%29; centralauth_Token=<redacted>; centralauth_Session=<redacted>;' | grep Cache
Cache-control: s-maxage=10, max-age=0, public
X-Cache: cp1067 pass, cp4018 pass, cp4009 pass
X-Cache-Status: pass

This is caused by the API always emitting a Vary: Cookies header, which makes Varnish never cache any request with (certain) cookies in it. Which essentially disables Varnish caching for all API requests if the user is logged in, even if the given API endpoint returns public information and does not care who the user is.

Tgr created this task.Jan 14 2017, 5:36 PM
Restricted Application added a project: Operations. · View Herald TranscriptJan 14 2017, 5:36 PM
Restricted Application added a subscriber: Aklapper. · View Herald Transcript
Anomie added a subscriber: Anomie.Jan 14 2017, 6:35 PM

Duplicate of T97096 (I'm on my phone or I'd close it as such).

Tgr reopened this task as Open.Jan 14 2017, 9:03 PM

That task is about the MediaWiki API not emitting caching headers when uselang=content is not used. This is about Varnish marking such requests as uncacheable in the first place (maybe by presence of the session cookie) - as far as I understand pass means that the decision is made at the Varnish layer, before contacting the API.

In any case:

$ curl -is 'https://en.wikipedia.org/w/api.php?action=query&meta=siteinfo&uselang=content&smaxage=10' -H 'Cookie: enwikiSession=<redacted>; enwikiUserID=20093189; enwikiUserName=Tgr+%28WMF%29; forceHTTPS=true; centralauth_User=Tgr+%28WMF%29; centralauth_Token=<redacted>; centralauth_Session=<redacted>;' | grep Cache
Cache-control: s-maxage=10, max-age=0, public
X-Cache: cp1067 pass, cp4018 pass, cp4009 pass
X-Cache-Status: pass
Tgr renamed this task from Action API caching does not work when logged in to Varnish does not cache Action API responses when logged in.Jan 14 2017, 9:06 PM
Tgr updated the task description. (Show Details)
Tgr updated the task description. (Show Details)

Note it doesn't really depend on "logged in", just on the presence of anything matching ([sS]ession|Token)= in the Cookie header:

$ curl -is 'https://en.wikipedia.org/w/api.php?action=query&meta=siteinfo&uselang=content&smaxage=10' -H 'Cookie: xToken=1;' | grep -i cache
cache-control: s-maxage=10, max-age=0, public
x-cache: cp1067 pass, cp1053 pass
x-cache-status: pass

Chances are that to avoid this we'd have to figure out how to reliably differentiate requests that actually depend on the session (including the session user's identity or preferences) from those that don't and then somehow avoid setting Vary: Cookie for the latter.

ema moved this task from Triage to Caching on the Traffic board.Jan 16 2017, 6:50 PM
Tgr added a comment.Jan 17 2017, 7:09 AM

Yeah, it seems like Varnish just looks at the presence of the Vary header.

Can't the API just ask the modules via some overridable method API (like isReadMode etc)?

No, because the module might not know in the first place. Look at the crazy things that cause trouble for T127233: Endpoints which do not need to authenticate users should set MW_NO_SESSION, it's basically the same thing but without the luxury of being able to actually define MW_NO_SESSION and blacklist all session access.

A rough idea might be to add code (probably in SessionBackend and User::loadFromSession()?) to set some flag to indicate that the session was accessed, without triggering from whatever Setup.php does. Then make sure nothing can change that flag between when you check it to set the Vary header and when things are actually output.

Ottomata triaged this task as Normal priority.Mar 6 2017, 7:02 PM
Ottomata added subscribers: ema, Ottomata.

Ping @ema.

I'm not sure how to triage this. Does something need to change on the varnish end? Or just the change @Anomie suggests in MW?

Tgr added a comment.Mar 6 2017, 10:22 PM

A rough idea might be to add code (probably in SessionBackend and User::loadFromSession()?) to set some flag to indicate that the session was accessed, without triggering from whatever Setup.php does. Then make sure nothing can change that flag between when you check it to set the Vary header and when things are actually output.

That wouldn't guarantee that the same URL reliably always or never emits Vary: Cookie. E.g. asking data from normal vs. revdel'd revision - from the API's point of view there is not much of a difference.

Maybe there could be a separate non-authenticated API endpoint?

I'm not sure how to triage this. Does something need to change on the varnish end? Or just the change @Anomie suggests in MW?

Setting (or not) Vary seems to be the right way to tell Varnish whether to cache or not, and the Varnish docs claim it's honored, so there shouldn't be any need for change.

Change 341502 had a related patch set uploaded (by ema):
[operations/puppet] cache_text varnishtest: vary cookie

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

Change 341502 merged by Ema:
[operations/puppet] cache_text varnishtest: vary cookie

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

ema added a comment.Mar 7 2017, 8:39 AM

Setting (or not) Vary seems to be the right way to tell Varnish whether to cache or not, and the Varnish docs claim it's honored, so there shouldn't be any need for change.

That's right. I've added a varnish test case to test/document Vary: Cookie handling.

This seems to WAI, only responses for anons are cached in varnish, anything left to do?

Tgr added a comment.Jul 19 2017, 2:05 PM

This seems to WAI, only responses for anons are cached in varnish, anything left to do?

Fix api.php to not vary on cookies when the response did not depend on session data. Or decide that that is unrealistic to detect and decline.

Tgr updated the task description. (Show Details)Jul 19 2017, 2:07 PM
Tgr added a comment.Jul 19 2017, 2:11 PM

This is not a huge deal right now as clients have to explicitly ask for API responses to be cached in Varnish (although some tools do that, e.g. MediaViewer, and become slower for logged-in users as the result of this bug); depending on the outcome of T122867: Evaluate the feasibility of cache invalidation for the action API it might become a bigger obstacle in the future.