Page MenuHomePhabricator

ResourceLoader: Module cache should invalidate (new timestamp) when module definition changes
Closed, ResolvedPublic

Description

When documenting the cache system[1] I was wondering something we need to check, nothing here for now:

So we recursively go through a modules components to determine the timestamp of the module as a whole (mtimes of the js/css files, mtimes of images referenced inside css, wiki pages (either as js/css page or as interface message) etc.).

Do we properly invalidate the cache if a new component is added that has a last-modified timestamp that is lower than the current max().

e.g. adding a file to 'scripts', or adding a message that hasn't been changed in a while but was forgotten.


Version: unspecified
Severity: normal
Whiteboard: deploysprint-13

Details

Reference
bz37812

Event Timeline

bzimport raised the priority of this task from to High.Nov 22 2014, 12:28 AM
bzimport set Reference to bz37812.

I believe message additions are handled correctly, but file additions probably aren't.

Note this also affects removal of items in the registry.

The only way to do this I've come up with so far is by adding a hash of the module registry to the module registry itself. And then version that in the configured cache handler.

Basically the same way we cache/invalidate cache of global variables. Just like the registry, global variables aren't files so they have no timestamp. Instead we hash the contents, and add it + timestamp to cache, and next time hash again and compare hashes. If different, update timestamp to current timestamp.

See also the way I implemented it in ResourceLoaderLanguageDataModule.

This bug is causing a major headache for the mobile team. The MobileFrontend codebase is (currently) in a state of massive flux. Many of our recent deployments have been bitten by this bug - and particularly if someone who's not familiar with MobileFrontend/MobileFrontend Beta performs a deployment, it's easy to overlook issues that arise from this. Considering how longstanding this issue is, can this be prioritized higher?

  • Bug 45877 has been marked as a duplicate of this bug. ***

Bug 45877 is like this but may be slightly different. Our deploy added a brand new RL module for a new .js file. The request for it and a bunch of other RL modules (e.g. https://bits.wikimedia.org/en.wikipedia.org/load.php?debug=false&lang=en&modules=...|ext.gettingstarted.openTask|...&skin=vector&version=20130308T000751Z&* cached a state of it of "missing" for over 50 minutes. More details in that bug.

Where does a fix for this issue need to occur? In ResourceLoader itself or somewhere in a deploy script?

Renamed from:

Module cache should invalidate (new timestamp) when a less old items (scripts, style and/or message) are added or removed.

to:

Module cache should invalidate (new timestamp) when module registration changes

As it should also cover when things are re-arranged.

e.g.
'scripts' => array( 'foo', 'bar', 'baz' );
to:
'scripts' => array( 'bar', 'foo', 'baz' );

Since this affects concatenation order, which can be significant for large modules that are made up of smaller files that don't work stand-alone but rely on context etc.

Change 90541 had a related patch set uploaded by Krinkle:
resourceloader: Add definition hashing to improve cache invalidation

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

Change 90541 merged by jenkins-bot:
resourceloader: Add definition hashing to improve cache invalidation

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