Page MenuHomePhabricator

No in-process cache for CentralAuthUser
Closed, ResolvedPublic

Description

If the same CentralAuthUser object is loaded multiple times in the same process, each load triggers a WAN cache read. Fixing that should be trivial and low-risk - just make sure the in-process cache is memory-limited (e.g. HashBagOStuff with an entry count limit) and is invalidated any time the WAN cache is.

Event Timeline

Tgr created this task.Feb 17 2016, 9:04 PM
Tgr raised the priority of this task from to Needs Triage.
Tgr updated the task description. (Show Details)
Tgr added a subscriber: Tgr.
Restricted Application added subscribers: StudiesWorld, Aklapper. · View Herald TranscriptFeb 17 2016, 9:04 PM
Tgr added a subscriber: Anomie.EditedFeb 17 2016, 9:13 PM

Filed mainly because SessionManager used to initialize the session multiple times for a single request, due to backwards-compatibility with PHP's internal session handling and limitations on what can be cached in-process without messing up HHVM garbage collection; I cannot reproduce that behavior anymore though. @Anomie, did something change there?

Anomie added a comment.EditedFeb 17 2016, 10:05 PM

While SessionManager does create a Session object multiple times, it will generally only be determining the session from the request cookies and headers once. The rest of the times it should just be loading the session from the SessionID saved in the WebRequest after that first determination.

The PHP backwards-compatibility stuff (MediaWiki\Session\PHPSessionHandler) never actually determines the session from the request cookies and headers, since it always deals with a passed-in session ID.

However, CentralAuthSessionProvider::refreshSessionInfo() probably will be called for every Session object created, even if it is created from an ID, and that does create a new CentralAuthUser for each call.

Unfortunately we can't use a CachedBagOStuff here, since CentralAuthUser stores its stuff in a WANObjectCache rather than a BagOStuff of any sort.

Tgr added a comment.Feb 19 2016, 6:52 PM

Instead of putting something transparent before the WANObjectCache I would rather cache the central user objects themselves and add another layer of cache lookup.

Change 271803 had a related patch set uploaded (by Anomie):
Cache CentralAuthUsers more aggressively

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

ori awarded a token.Feb 22 2016, 11:09 AM

Change 271803 merged by jenkins-bot:
Cache CentralAuthUsers more aggressively

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

Change 272642 had a related patch set uploaded (by Paladox):
Cache CentralAuthUsers more aggressively

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

Change 272642 merged by jenkins-bot:
Cache CentralAuthUsers more aggressively

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

Anomie closed this task as Resolved.Feb 24 2016, 12:57 AM
Anomie claimed this task.

Change 273273 had a related patch set uploaded (by Paladox):
Cache CentralAuthUsers more aggressively

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

Change 273273 abandoned by Krinkle:
Cache CentralAuthUsers more aggressively

Reason:
Was intentionally not backported to wmf.13 yesterday.

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