Page MenuHomePhabricator

Introduce ObjectCacheFactory to MediaWiki core
Closed, ResolvedPublic

Description

ObjectCache is responsible for creating and fetching various cache instances (backends) for MediaWiki making use of BagOStuff which is the mechanism to do caching in the software's ecosystem.

Overtime, ObjectCache has been built to create and configure various types of caching backend such as: RESTBagOStuff, SqlBagOStuff and many more and the class itself was beginning to handle returning different types of BagOStuff with respect to the parameters supplied. This was an indication that we would someday need a factory to extract such logic into it and leave ObjectCache with the bare minimum. So logic for creating the different types of BOS instances will be handled by a separate service on its own - a name of such service an be ObjectCacheFactory.

As of today, there are 2 ways for ObjectCache to get a cache instance:

(1) Directly via ObjectCache::getInstance() plus some config setting like $wgObjectCaches['...'] = [ ... ] and this bypasses service wiring and creates a cache instance directly.

One of such use cases for this is creating a DB connection (on installing MW or otherwise), we don't necessarily need a services container to have been ready by this time so we need a way to still make use of local caches.

Another use case is when we're doing extension registration, the ExtensionRegistry would want to make use of local caches and at this time, the MW services container may not have been initialized meaning getting one will have to do so without service wiring.

(2) Fetching or creating cache through service wiring. This is something we can achieve when the services container is available and we're using it already in a lot of places already. A use case is an extension like ConfirmedEdit which makes use of the MicroStash to save captcha tokens. By this time, the extension has already been loaded and the MW services container is already initialized so using a service to access the cache for get/set is completely fine.

Problem

The problem now arrises when both approaches above gets used interchangeable. It works but it creates a lot of inconsistencies in our codebase and makes code unstable. We want to unify and make things consistent so that only when we need to bypass service wiring that we do otherwise we will have to go through service wiring as a lot of our code is already doing today.

The factory would need to support backward compatibility as there are cases whereby we try to mutate ObjectCache::$instances directly since this member is public. Worse of all, even some tests try to do this too.

Event Timeline

Change 955771 had a related patch set uploaded (by D3r1ck01; author: Derick Alangi):

[mediawiki/core@master] objectcache: Introduce `ObjectCacheFactory` MW service

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

Change 1005972 had a related patch set uploaded (by Krinkle; author: Derick Alangi):

[mediawiki/extensions/AbuseFilter@master] Handlers: Drop `AutoPromoteGroupsHandler::factory()`

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

Change 1005972 merged by jenkins-bot:

[mediawiki/extensions/AbuseFilter@master] Handlers: Drop `AutoPromoteGroupsHandler::factory()`

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

Change 1009498 had a related patch set uploaded (by D3r1ck01; author: Derick Alangi):

[mediawiki/extensions/ConfirmEdit@master] Fix ConfirmEdit to avoid global state

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

Change 1009514 had a related patch set uploaded (by D3r1ck01; author: Derick Alangi):

[mediawiki/extensions/DonationInterface@master] IPVelocityTest: Avoid use of global state `ObjectCache::$instances`

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

Change 1009514 merged by jenkins-bot:

[mediawiki/extensions/DonationInterface@master] IPVelocityTest: Avoid use of global state `ObjectCache::$instances`

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

Change 1009498 merged by jenkins-bot:

[mediawiki/extensions/ConfirmEdit@master] Fix ConfirmEdit to avoid global state

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

DAlangi_WMF changed the task status from Open to In Progress.Mar 12 2024, 10:44 AM

Change 1005964 had a related patch set uploaded (by Jforrester; author: Derick Alangi):

[mediawiki/extensions/WikiLambda@master] ServiceWiring: Use `newFromParams()` via MW services

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

Change 955771 merged by jenkins-bot:

[mediawiki/core@master] objectcache: Introduce `ObjectCacheFactory` MW service

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

Change 1005964 merged by jenkins-bot:

[mediawiki/extensions/WikiLambda@master] ServiceWiring: Use `newFromParams()` via MW services

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

Change #1013392 had a related patch set uploaded (by D3r1ck01; author: Derick Alangi):

[mediawiki/core@master] objectcache: Dependency inject the wiki ID & some improvements

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

Change #1015097 had a related patch set uploaded (by Krinkle; author: Krinkle):

[mediawiki/core@master] objectcache: Fix typo in getLocalServerInstance deprecation notice

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

Change #1015097 merged by jenkins-bot:

[mediawiki/core@master] objectcache: Fix typo in getLocalServerInstance deprecation notice

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

Change #1015204 had a related patch set uploaded (by Gergő Tisza; author: Derick Alangi):

[mediawiki/core@wmf/1.42.0-wmf.24] objectcache: Restore default keyspace for LocalServerCache service

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

Change #1015204 merged by jenkins-bot:

[mediawiki/core@wmf/1.42.0-wmf.24] objectcache: Restore default keyspace for LocalServerCache service

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

Mentioned in SAL (#wikimedia-operations) [2024-03-28T18:54:24Z] <jhuneidi@deploy1002> Started scap: Backport for [[gerrit:1015204|objectcache: Restore default keyspace for LocalServerCache service (T358346 T361177)]]

Mentioned in SAL (#wikimedia-operations) [2024-03-28T18:56:52Z] <jhuneidi@deploy1002> tgr and jhuneidi: Backport for [[gerrit:1015204|objectcache: Restore default keyspace for LocalServerCache service (T358346 T361177)]] synced to the testservers (https://wikitech.wikimedia.org/wiki/Mwdebug)

Mentioned in SAL (#wikimedia-operations) [2024-03-28T19:28:24Z] <jhuneidi@deploy1002> Finished scap: Backport for [[gerrit:1015204|objectcache: Restore default keyspace for LocalServerCache service (T358346 T361177)]] (duration: 34m 00s)

Change #1013392 merged by jenkins-bot:

[mediawiki/core@master] objectcache: Restore default keyspace for LocalServerCache service

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

DAlangi_WMF added a subscriber: Tgr.

Thank you @daniel and @Krinkle for your reviews on this tasks and guidance to make it land.

More thanks @Tgr, @pmiazga, and others for dealing with the regressions while I was out. I appreciate <3