Thijs/wiki.techinc.nl
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

objectcache: Complete refactor of ObjectCache.php

Browse source 
This patch completes the rest of the ObjectCache refactor and
migrates methods to the appropriate class while deprecating them
in `ObjectCache.php`.

It also moves the `_LocalClusterCache` internal service logic
into ObjectCacheFactory and calls that instead making sure that
wiring code stays wiring code and let the class do the heavy lifting.

`::makeLocalServerCache()` is retained as a static method in the
ObjectCacheFactory class because it's called early in Setup.php
before the services container is available (so it needs to be stand-
alone).

To add, we also converts all global variables that were used in the
`ObjectCache.php` class into the config schema approach and retrieves
them using ServiceOptions injected in service wiring.

NOTE: MediaWikiIntegrationTestCase::setMainCache() was slightly
rewritten to take care of service reset which throws away the cache
object preserved by setInstanceForTesting() after service reset.
Instead, we preserve the object via MainConfigNames::ObjectCaches
setting with a factory closure which returns the correct cache object.

As a nice side effect of the above, the setInstanceForTesting() method
was removed entirely.

As a follow-up to this patch, I would like to remove the internal
_LocalClusterCache service in a stand-alone patch.

Bug: T363770
Change-Id: Ia2b689243980dbac37ee3bcfcbdf0683f9e1779b
This commit is contained in:
Derick Alangi 2024-05-13 12:16:27 +01:00 committed by Krinkle
parent 37ae84be8b
commit 7475063bfd
9 changed files with 185 additions and 159 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

3
RELEASE-NOTES-1.42
View file

@ -681,8 +681,7 @@ because of Phabricator reports.
available in ObjectCacheFactory, use them instead:
- ::getInstance() -> ObjectCacheFactory::getInstance()
- ::newFromParams() -> ObjectCacheFactory::newFromParams()
- ::newAnything() -> ObjectCacheFactory::newInstance(
ObjectCache::getAnythingId() )
- ::newAnything() -> ObjectCacheFactory::newInstance( CACHE_ANYTHING )
- ::getLocalServerInstance() -> ObjectCacheFactory::getLocalServerInstance()
- ::clear() -> ObjectCacheFactory::clear()
- In addition, the ObjectCache::$instances member has been deprecated as well.

2
RELEASE-NOTES-1.43
View file

@ -247,6 +247,8 @@ because of Phabricator reports.
use wfMessage() or RequestContext::msg() instead.
* SearchEngineConfig::getConfig() has been deprecated, use DI with
ServiceOptions to inject the required options.
* ObjectCache::isDatabaseId() and ::getLocalClusterInstance() have been
deprecated. Use their equivalents in ObjectCacheFactory.
* Using the "post" source in parameter declarations returned from
Handler::getParamSettings() is deprecated, use "body" instead.
* IMaintainableDatabase::textFieldSize() is now deprecated.

12
includes/ServiceWiring.php
View file

@ -489,7 +489,9 @@ return [
'ChronologyProtector' => static function ( MediaWikiServices $services ): ChronologyProtector {
$mainConfig = $services->getMainConfig();
$cpStashType = $mainConfig->get( MainConfigNames::ChronologyProtectorStash );
$isMainCacheBad = ObjectCache::isDatabaseId( $mainConfig->get( MainConfigNames::MainCacheType ) );
$isMainCacheBad = $services->getObjectCacheFactory()->isDatabaseId(
$mainConfig->get( MainConfigNames::MainCacheType )
);
if ( is_string( $cpStashType ) ) {
$cpStash = $services->getObjectCacheFactory()->getInstance( $cpStashType );
@ -726,7 +728,9 @@ return [
'DBLoadBalancerFactoryConfigBuilder' => static function ( MediaWikiServices $services ): MWLBFactory {
$mainConfig = $services->getMainConfig();
if ( ObjectCache::isDatabaseId( $mainConfig->get( MainConfigNames::MainCacheType ) ) ) {
if ( $services->getObjectCacheFactory()->isDatabaseId(
$mainConfig->get( MainConfigNames::MainCacheType )
) ) {
$wanCache = WANObjectCache::newEmpty();
} else {
$wanCache = $services->getMainWANObjectCache();
@ -2597,9 +2601,7 @@ return [
},
'_LocalClusterCache' => static function ( MediaWikiServices $services ): BagOStuff {
$mainConfig = $services->getMainConfig();
$id = $mainConfig->get( MainConfigNames::MainCacheType );
return $services->getObjectCacheFactory()->getInstance( $id );
return $services->getObjectCacheFactory()->getLocalClusterInstance();
},
'_MediaWikiTitleCodec' => static function ( MediaWikiServices $services ): MediaWikiTitleCodec {

131
includes/objectcache/ObjectCache.php
View file

@ -34,12 +34,6 @@ class ObjectCache {
*/
public static $instances = [];
/**
* @internal for ObjectCacheTest
* @var string
*/
public static $localServerCacheClass;
/**
* Get a cached instance of the specified type of cache object.
*
@ -75,51 +69,13 @@ class ObjectCache {
* If no cache choice is configured (by default $wgMainCacheType is CACHE_NONE),
* then CACHE_ANYTHING will forward to CACHE_DB.
*
* @deprecated since 1.42,
* Use ObjectCacheFactory::getInstance( ObjectCache::getAnythingId() );
* @deprecated since 1.42, Use ObjectCacheFactory::getInstance( CACHE_ANYTHING );
*
* @return BagOStuff
*/
public static function newAnything() {
return MediaWikiServices::getInstance()->getObjectCacheFactory()
->getInstance( self::getAnythingId() );
}
/**
* @internal Used by ObjectCacheFactory and ObjectCache.
*
* Get the ID that will be used for CACHE_ANYTHING
* @return string|int
*/
public static function getAnythingId() {
global $wgMainCacheType, $wgMessageCacheType, $wgParserCacheType;
$candidates = [ $wgMainCacheType, $wgMessageCacheType, $wgParserCacheType ];
foreach ( $candidates as $candidate ) {
if ( $candidate === CACHE_ACCEL ) {
// CACHE_ACCEL might default to nothing if no APCu
// See includes/ServiceWiring.php
$class = self::getLocalServerCacheClass();
if ( $class !== EmptyBagOStuff::class ) {
return $candidate;
}
} elseif ( $candidate !== CACHE_NONE && $candidate !== CACHE_ANYTHING ) {
return $candidate;
}
}
$services = MediaWikiServices::getInstance();
if ( $services->isServiceDisabled( 'DBLoadBalancer' ) ) {
// The DBLoadBalancer service is disabled, so we can't use the database!
$candidate = CACHE_NONE;
} elseif ( $services->isStorageDisabled() ) {
// Storage services are disabled because MediaWikiServices::disableStorage()
// was called. This is typically the case during installation.
$candidate = CACHE_NONE;
} else {
$candidate = CACHE_DB;
}
return $candidate;
->getInstance( CACHE_ANYTHING );
}
/**
@ -137,40 +93,14 @@ class ObjectCache {
/**
* Get the main cluster-local cache object.
*
* @deprecated since 1.43, Use ObjectCacheFactory::getLocalClusterInstance()
*
* @since 1.27
* @return BagOStuff
*/
public static function getLocalClusterInstance() {
return MediaWikiServices::getInstance()->get( '_LocalClusterCache' );
}
/**
* Determine whether a config ID would access the database
*
* @param string|int $id A key in $wgObjectCaches
* @return bool
*/
public static function isDatabaseId( $id ) {
global $wgObjectCaches;
// NOTE: Sanity check if $id is set to CACHE_ANYTHING and
// everything is going through service wiring. CACHE_ANYTHING
// would default to CACHE_DB, let's handle that early for cases
// where all cache configs are set to CACHE_ANYTHING (T362686).
if ( $id === CACHE_ANYTHING ) {
$id = self::getAnythingId();
return self::isDatabaseId( $id );
}
if ( !isset( $wgObjectCaches[$id] ) ) {
return false;
}
$cache = $wgObjectCaches[$id];
if ( ( $cache['class'] ?? '' ) === SqlBagOStuff::class ) {
return true;
}
return false;
return MediaWikiServices::getInstance()->getObjectCacheFactory()
->getLocalClusterInstance();
}
/**
@ -181,53 +111,4 @@ class ObjectCache {
public static function clear() {
MediaWikiServices::getInstance()->getObjectCacheFactory()->clear();
}
/**
* Create a new BagOStuff instance for local-server caching.
*
* Only use this if you explicitly require the creation of
* a fresh instance. Whenever possible, use or inject the object
* from MediaWikiServices::getLocalServerObjectCache() instead.
*
* NOTE: This method is called very early via Setup.php by ExtensionRegistry,
* and thus must remain fairly standalone so as to not cause initialization
* of the MediaWikiServices singleton.
*
* @internal For use by ServiceWiring and ExtensionRegistry. There are use
* cases whereby we want to build up local server cache without service
* wiring available.
* @since 1.35
* @param string $keyspace
* @return BagOStuff
*/
public static function makeLocalServerCache( $keyspace ): BagOStuff {
$params = [
'reportDupes' => false,
// Even simple caches must use a keyspace (T247562)
'keyspace' => $keyspace,
];
$class = self::getLocalServerCacheClass();
return new $class( $params );
}
/**
* Get the class which will be used for the local server cache
* @return string
*/
private static function getLocalServerCacheClass() {
if ( self::$localServerCacheClass !== null ) {
return self::$localServerCacheClass;
}
if ( function_exists( 'apcu_fetch' ) ) {
// Make sure the APCu methods actually store anything
if ( PHP_SAPI !== 'cli' || ini_get( 'apc.enable_cli' ) ) {
return APCUBagOStuff::class;
}
} elseif ( function_exists( 'wincache_ucache_get' ) ) {
return WinCacheBagOStuff::class;
}
return EmptyBagOStuff::class;
}
}

156
includes/objectcache/ObjectCacheFactory.php
View file

@ -22,6 +22,7 @@ use MediaWiki\Config\ServiceOptions;
use MediaWiki\Http\Telemetry;
use MediaWiki\Logger\Spi;
use MediaWiki\MainConfigNames;
use MediaWiki\MediaWikiServices;
use Wikimedia\Stats\StatsFactory;
/**
@ -45,6 +46,12 @@ use Wikimedia\Stats\StatsFactory;
* and EmptyBagOStuff in CLI mode).
* Not replicated to the other servers.
*
* - ObjectCacheFactory::getLocalClusterInstance()
* Purpose: Memory storage for per-cluster coordination and tracking.
* A typical use case would be a rate limit counter or cache regeneration mutex.
* Stored centrally within the local data-center. Not replicated to other DCs.
* Configured by $wgMainCacheType.
*
* - ObjectCacheFactory::getInstance( $cacheType )
* Purpose: Special cases (like tiered memory/disk caches).
* Get a specific cache type by key in $wgObjectCaches.
@ -70,6 +77,8 @@ class ObjectCacheFactory {
MainConfigNames::CachePrefix,
MainConfigNames::ObjectCaches,
MainConfigNames::MainCacheType,
MainConfigNames::MessageCacheType,
MainConfigNames::ParserCacheType,
];
private ServiceOptions $options;
@ -80,6 +89,11 @@ class ObjectCacheFactory {
private string $domainId;
/** @var callable */
private $dbLoadBalancerFactory;
/**
* @internal ObjectCacheTest only, needed for b/c.
* @var string
*/
public static $localServerCacheClass;
public function __construct(
ServiceOptions $options,
@ -122,7 +136,7 @@ class ObjectCacheFactory {
*/
private function newFromId( $id ): BagOStuff {
if ( $id === CACHE_ANYTHING ) {
$id = ObjectCache::getAnythingId();
$id = $this->getAnythingId();
}
if ( !isset( $this->options->get( MainConfigNames::ObjectCaches )[$id] ) ) {
@ -132,7 +146,7 @@ class ObjectCacheFactory {
} elseif ( $id === CACHE_HASH ) {
return new HashBagOStuff();
} elseif ( $id === CACHE_ACCEL ) {
return ObjectCache::makeLocalServerCache( $this->getDefaultKeyspace() );
return self::makeLocalServerCache( $this->getDefaultKeyspace() );
}
throw new InvalidArgumentException( "Invalid object cache type \"$id\" requested. " .
@ -157,12 +171,12 @@ class ObjectCacheFactory {
}
/**
* Create a new cache object from parameters specification supplied.
*
* @internal Using this method directly outside of MediaWiki core
* is discouraged. Use getInstance() instead and supply the ID
* of the cache instance to be looked up.
*
* Create a new cache object from parameters specification supplied.
*
* @param array $params Must have 'factory' or 'class' property.
* - factory: Callback passed $params that returns BagOStuff.
* - class: BagOStuff subclass constructed with $params.
@ -309,16 +323,132 @@ class ObjectCacheFactory {
}
/**
* @internal For tests ONLY.
*
* @param string|int $cacheId
* @param BagOStuff $cache
* @return void
* Get the class which will be used for the local server cache
* @return string
*/
public function setInstanceForTesting( $cacheId, BagOStuff $cache ): void {
if ( !defined( 'MW_PHPUNIT_TEST' ) ) {
throw new LogicException( __METHOD__ . ' can not be called outside of tests' );
private static function getLocalServerCacheClass() {
if ( self::$localServerCacheClass !== null ) {
return self::$localServerCacheClass;
}
$this->instances[$cacheId] = $cache;
if ( function_exists( 'apcu_fetch' ) ) {
// Make sure the APCu methods actually store anything
if ( PHP_SAPI !== 'cli' || ini_get( 'apc.enable_cli' ) ) {
return APCUBagOStuff::class;
}
} elseif ( function_exists( 'wincache_ucache_get' ) ) {
return WinCacheBagOStuff::class;
}
return EmptyBagOStuff::class;
}
/**
* Get the ID that will be used for CACHE_ANYTHING
*
* @internal
* @return string|int
*/
public function getAnythingId() {
$candidates = [
$this->options->get( MainConfigNames::MainCacheType ),
$this->options->get( MainConfigNames::MessageCacheType ),
$this->options->get( MainConfigNames::ParserCacheType )
];
foreach ( $candidates as $candidate ) {
if ( $candidate === CACHE_ACCEL ) {
// CACHE_ACCEL might default to nothing if no APCu
// See includes/ServiceWiring.php
$class = self::getLocalServerCacheClass();
if ( $class !== EmptyBagOStuff::class ) {
return $candidate;
}
} elseif ( $candidate !== CACHE_NONE && $candidate !== CACHE_ANYTHING ) {
return $candidate;
}
}
$services = MediaWikiServices::getInstance();
if ( $services->isServiceDisabled( 'DBLoadBalancer' ) ) {
// The DBLoadBalancer service is disabled, so we can't use the database!
$candidate = CACHE_NONE;
} elseif ( $services->isStorageDisabled() ) {
// Storage services are disabled because MediaWikiServices::disableStorage()
// was called. This is typically the case during installation.
$candidate = CACHE_NONE;
} else {
$candidate = CACHE_DB;
}
return $candidate;
}
/**
* Create a new BagOStuff instance for local-server caching.
*
* Only use this if you explicitly require the creation of
* a fresh instance. Whenever possible, use or inject the object
* from MediaWikiServices::getLocalServerObjectCache() instead.
*
* NOTE: This method is called very early via Setup.php by ExtensionRegistry,
* and thus must remain fairly standalone so as to not cause initialization
* of the MediaWikiServices singleton.
*
* @internal For use by ServiceWiring and ExtensionRegistry. There are use
* cases whereby we want to build up local server cache without service
* wiring available.
* @since 1.43, previously on ObjectCache.php since 1.35
*
* @param string $keyspace
* @return BagOStuff
*/
public static function makeLocalServerCache( string $keyspace ) {
$params = [
'reportDupes' => false,
// Even simple caches must use a keyspace (T247562)
'keyspace' => $keyspace,
];
$class = self::getLocalServerCacheClass();
return new $class( $params );
}
/**
* Determine whether a config ID would access the database
*
* @internal For use by ServiceWiring.php
* @param string|int $id A key in $wgObjectCaches
* @return bool
*/
public function isDatabaseId( $id ) {
// NOTE: Sanity check if $id is set to CACHE_ANYTHING and
// everything is going through service wiring. CACHE_ANYTHING
// would default to CACHE_DB, let's handle that early for cases
// where all cache configs are set to CACHE_ANYTHING (T362686).
if ( $id === CACHE_ANYTHING ) {
$id = $this->getAnythingId();
return $this->isDatabaseId( $id );
}
if ( !isset( $this->options->get( MainConfigNames::ObjectCaches )[$id] ) ) {
return false;
}
$cache = $this->options->get( MainConfigNames::ObjectCaches )[$id];
if ( ( $cache['class'] ?? '' ) === SqlBagOStuff::class ) {
return true;
}
return false;
}
/**
* Get the main cluster-local cache object.
*
* @since 1.43, previously on ObjectCache.php since 1.27
* @return BagOStuff
*/
public function getLocalClusterInstance() {
return $this->getInstance(
$this->options->get( MainConfigNames::MainCacheType )
);
}
}

2
includes/registration/ExtensionRegistry.php
View file

@ -253,7 +253,7 @@ class ExtensionRegistry {
$keyspace = ( is_string( $wgCachePrefix ) && $wgCachePrefix !== '' )
? $wgCachePrefix
: WikiMap::getCurrentWikiDbDomain()->getId();
return ObjectCache::makeLocalServerCache( $keyspace );
return ObjectCacheFactory::makeLocalServerCache( $keyspace );
}
return $this->cache;

10
tests/phpunit/MediaWikiIntegrationTestCase.php
View file

@ -961,8 +961,14 @@ abstract class MediaWikiIntegrationTestCase extends PHPUnit\Framework\TestCase {
protected function setMainCache( $cache ) {
if ( $cache instanceof BagOStuff ) {
$cacheId = 'UTCache';
$this->getServiceContainer()->getObjectCacheFactory()
->setInstanceForTesting( $cacheId, $cache );
$objectCaches = $this->getServiceContainer()->getMainConfig()
->get( MainConfigNames::ObjectCaches );
$objectCaches[$cacheId] = [
'factory' => static function () use ( $cache ) {
return $cache;
}
];
$this->overrideConfigValue( MainConfigNames::ObjectCaches, $objectCaches );
} else {
$cacheId = $cache;
$cache = $this->getServiceContainer()->getObjectCacheFactory()

27
tests/phpunit/includes/objectcache/ObjectCacheTest.php
View file

@ -24,7 +24,7 @@ class ObjectCacheTest extends MediaWikiIntegrationTestCase {
}
protected function tearDown(): void {
ObjectCache::$localServerCacheClass = null;
ObjectCacheFactory::$localServerCacheClass = null;
}
private function setCacheConfig( $arr = [] ) {
@ -37,13 +37,14 @@ class ObjectCacheTest extends MediaWikiIntegrationTestCase {
$this->overrideConfigValue( MainConfigNames::ObjectCaches, $arr + $defaults );
// Mock ACCEL with 'hash' as being installed.
// This makes tests deterministic regardless of APC.
ObjectCache::$localServerCacheClass = 'HashBagOStuff';
ObjectCacheFactory::$localServerCacheClass = 'HashBagOStuff';
}
public function testNewAnythingNothing() {
$ocf = $this->getServiceContainer()->getObjectCacheFactory();
$this->assertInstanceOf(
SqlBagOStuff::class,
$this->getServiceContainer()->getObjectCacheFactory()->getInstance( ObjectCache::getAnythingId() ),
$ocf->getInstance( $ocf->getAnythingId() ),
'No available types. Fallback to DB'
);
}
@ -51,9 +52,10 @@ class ObjectCacheTest extends MediaWikiIntegrationTestCase {
public function testNewAnythingHash() {
$this->setMainCache( CACHE_HASH );
$ocf = $this->getServiceContainer()->getObjectCacheFactory();
$this->assertInstanceOf(
HashBagOStuff::class,
$this->getServiceContainer()->getObjectCacheFactory()->getInstance( ObjectCache::getAnythingId() ),
$ocf->getInstance( $ocf->getAnythingId() ),
'Use an available type (hash)'
);
}
@ -61,21 +63,23 @@ class ObjectCacheTest extends MediaWikiIntegrationTestCase {
public function testNewAnythingAccel() {
$this->setMainCache( CACHE_ACCEL );
$ocf = $this->getServiceContainer()->getObjectCacheFactory();
$this->assertInstanceOf(
HashBagOStuff::class,
$this->getServiceContainer()->getObjectCacheFactory()->getInstance( ObjectCache::getAnythingId() ),
$ocf->getInstance( $ocf->getAnythingId() ),
'Use an available type (CACHE_ACCEL)'
);
}
public function testNewAnythingNoAccel() {
// Mock APC not being installed (T160519, T147161)
ObjectCache::$localServerCacheClass = EmptyBagOStuff::class;
ObjectCacheFactory::$localServerCacheClass = EmptyBagOStuff::class;
$this->setMainCache( CACHE_ACCEL );
$ocf = $this->getServiceContainer()->getObjectCacheFactory();
$this->assertInstanceOf(
SqlBagOStuff::class,
$this->getServiceContainer()->getObjectCacheFactory()->getInstance( ObjectCache::getAnythingId() ),
$ocf->getInstance( $ocf->getAnythingId() ),
'Fallback to DB if available types fall back to Empty'
);
}
@ -89,9 +93,10 @@ class ObjectCacheTest extends MediaWikiIntegrationTestCase {
$this->getServiceContainer()->disableStorage();
$ocf = $this->getServiceContainer()->getObjectCacheFactory();
$this->assertInstanceOf(
EmptyBagOStuff::class,
$this->getServiceContainer()->getObjectCacheFactory()->getInstance( ObjectCache::getAnythingId() ),
$ocf->getInstance( $ocf->getAnythingId() ),
'Fallback to none if available types and DB are unavailable'
);
}
@ -99,9 +104,10 @@ class ObjectCacheTest extends MediaWikiIntegrationTestCase {
public function testNewAnythingNothingNoDb() {
$this->getServiceContainer()->disableStorage();
$ocf = $this->getServiceContainer()->getObjectCacheFactory();
$this->assertInstanceOf(
EmptyBagOStuff::class,
$this->getServiceContainer()->getObjectCacheFactory()->getInstance( ObjectCache::getAnythingId() ),
$ocf->getInstance( $ocf->getAnythingId() ),
'No available types or DB. Fallback to none.'
);
}
@ -156,6 +162,7 @@ class ObjectCacheTest extends MediaWikiIntegrationTestCase {
$this->overrideConfigValues( [
MainConfigNames::MainCacheType => $mainCacheType
] );
$this->assertSame( $expected, ObjectCache::isDatabaseId( $id ) );
$ocf = $this->getServiceContainer()->getObjectCacheFactory();
$this->assertSame( $expected, $ocf->isDatabaseId( $id ) );
}
}

1
tests/phpunit/tests/MediaWikiIntegrationTestCaseTest.php
View file

@ -224,7 +224,6 @@ class MediaWikiIntegrationTestCaseTest extends MediaWikiIntegrationTestCase {
$cache = new HashBagOStuff();
$name = $this->setMainCache( $cache );
$this->assertSame( $cache, ObjectCache::getLocalClusterInstance() );
$this->getServiceContainer()->getObjectCacheFactory()->setInstanceForTesting( $name, $cache );
$this->assertSame( $cache, $this->getServiceContainer()->getObjectCacheFactory()->getInstance( $name ) );
// Our custom cache object should not replace an existing entry.