diff --git a/includes/libs/objectcache/APCUBagOStuff.php b/includes/libs/objectcache/APCUBagOStuff.php index 6f2d8db93c4..d516aad3aaf 100644 --- a/includes/libs/objectcache/APCUBagOStuff.php +++ b/includes/libs/objectcache/APCUBagOStuff.php @@ -161,4 +161,12 @@ class APCUBagOStuff extends MediumSpecificBagOStuff { return $result; } + + public function makeKeyInternal( $keyspace, $components ) { + return $this->genericKeyFromComponents( $keyspace, ...$components ); + } + + protected function convertGenericKey( $key ) { + return $key; // short-circuit; already uses "generic" keys + } } diff --git a/includes/libs/objectcache/BagOStuff.php b/includes/libs/objectcache/BagOStuff.php index a9e88a84afb..a82296345ce 100644 --- a/includes/libs/objectcache/BagOStuff.php +++ b/includes/libs/objectcache/BagOStuff.php @@ -71,13 +71,16 @@ abstract class BagOStuff implements { /** @var LoggerInterface */ protected $logger; - /** @var callable|null */ protected $asyncHandler; + /** @var int[] Map of (ATTR_* class constant => QOS_* class constant) */ protected $attrMap = []; - /** @var bool */ + /** @var string Default keyspace; used by makeKey() */ + protected $keyspace; + + /** @var bool Whether to send debug log entries to the SPI logger instance */ protected $debugMode = false; /** @var float|null */ @@ -93,15 +96,40 @@ abstract class BagOStuff implements public const WRITE_PRUNE_SEGMENTS = 32; // delete all the segments if the value is partitioned public const WRITE_BACKGROUND = 64; // if supported, do not block on completion until the next read + /** @var string Global keyspace; used by makeGlobalKey() */ + protected const GLOBAL_KEYSPACE = 'global'; + /** @var string Precomputed global cache key prefix (needs no encoding) */ + protected const GLOBAL_PREFIX = 'global:'; + /** @var string Precomputed global cache key prefix length */ + protected const GLOBAL_PREFIX_LEN = 7; + + /** @var int Item is a single cache key */ + protected const ARG0_KEY = 0; + /** @var int Item is an array of cache keys */ + protected const ARG0_KEYARR = 1; + /** @var int Item is an array indexed by cache keys */ + protected const ARG0_KEYMAP = 2; + /** @var int Item does not involve any keys */ + protected const ARG0_NONKEY = 3; + + /** @var int Item is an array indexed by cache keys */ + protected const RES_KEYMAP = 0; + /** @var int Item does not involve any keys */ + protected const RES_NONKEY = 1; + /** * Parameters include: - * - logger: Psr\Log\LoggerInterface instance + * - keyspace: Keyspace to use for keys in makeKey(). [Default: "local"] * - asyncHandler: Callable to use for scheduling tasks after the web request ends. - * In CLI mode, it should run the task immediately. + * In CLI mode, it should run the task immediately. [Default: null] + * - logger: Psr\Log\LoggerInterface instance. [optional] * @param array $params - * @phan-param array{logger?:Psr\Log\LoggerInterface,asyncHandler?:callable} $params + * @codingStandardsIgnoreStart + * @phan-param array{keyspace?:string,logger?:Psr\Log\LoggerInterface,asyncHandler?:callable} $params + * @codingStandardsIgnoreEnd */ public function __construct( array $params = [] ) { + $this->keyspace = $params['keyspace'] ?? 'local'; $this->setLogger( $params['logger'] ?? new NullLogger() ); $this->asyncHandler = $params['asyncHandler'] ?? null; } @@ -310,13 +338,13 @@ abstract class BagOStuff implements } /** - * Delete all objects expiring before a certain date. + * Delete all objects expiring before a certain date + * * @param string|int $timestamp The reference date in MW or TS_UNIX format * @param callable|null $progress Optional, a function which will be called * regularly during long-running operations with the percentage progress * as the first parameter. [optional] * @param int $limit Maximum number of keys to delete [default: INF] - * * @return bool Success; false if unimplemented */ abstract public function deleteObjectsExpiringBefore( @@ -326,7 +354,8 @@ abstract class BagOStuff implements ); /** - * Get an associative array containing the item for each of the keys that have items. + * Get an associative array containing the item for each of the keys that have items + * * @param string[] $keys List of keys * @param int $flags Bitfield; supports READ_LATEST [optional] * @return mixed[] Map of (key => value) for existing keys @@ -340,13 +369,13 @@ abstract class BagOStuff implements * * WRITE_BACKGROUND can be used for bulk insertion where the response is not vital * - * @param mixed[] $data Map of (key => value) + * @param mixed[] $valueByKey Map of (key => value) * @param int $exptime Either an interval in seconds or a unix timestamp for expiry * @param int $flags Bitfield of BagOStuff::WRITE_* constants (since 1.33) * @return bool Success * @since 1.24 */ - abstract public function setMulti( array $data, $exptime = 0, $flags = 0 ); + abstract public function setMulti( array $valueByKey, $exptime = 0, $flags = 0 ); /** * Batch deletion @@ -377,6 +406,7 @@ abstract class BagOStuff implements /** * Increase stored value of $key by $value while preserving its TTL + * * @param string $key Key to increase * @param int $value Value to add to $key (default: 1) [optional] * @param int $flags Bit field of class WRITE_* constants [optional] @@ -386,6 +416,7 @@ abstract class BagOStuff implements /** * Decrease stored value of $key by $value while preserving its TTL + * * @param string $key * @param int $value Value to subtract from $key (default: 1) [optional] * @param int $flags Bit field of class WRITE_* constants [optional] @@ -446,36 +477,51 @@ abstract class BagOStuff implements abstract public function addBusyCallback( callable $workCallback ); /** - * Construct a cache key. + * Make a cache key for the given keyspace and components + * + * Long components might be converted to respective hashes due to size constraints. + * In extreme cases, all of them might be combined into a single hash component. * * @internal This method should not be used outside of BagOStuff (since 1.36) + * + * @param string $keyspace Keyspace component + * @param string[]|int[] $components Key components (key collection name first) + * @return string Keyspace-prepended list of encoded components as a colon-separated value * @since 1.27 - * @param string $keyspace - * @param array $components - * @return string Colon-delimited list of $keyspace followed by escaped components of $args */ abstract public function makeKeyInternal( $keyspace, $components ); /** - * Make a global cache key. + * Make a cache key for the default keyspace and given components * + * @param string $class Key collection name component + * @param string|int ...$components Key components for entity IDs + * @return string Keyspace-prepended list of encoded components as a colon-separated value * @since 1.27 - * @param string $class Key class - * @param string|int ...$components Key components (starting with a key collection name) - * @return string Colon-delimited list of $keyspace followed by escaped components */ abstract public function makeGlobalKey( $class, ...$components ); /** - * Make a cache key, scoped to this instance's keyspace. + * Make a cache key for the global keyspace and given components * + * @param string $class Key collection name component + * @param string|int ...$components Key components for entity IDs + * @return string Keyspace-prepended list of encoded components as a colon-separated value * @since 1.27 - * @param string $class Key class - * @param string|int ...$components Key components (starting with a key collection name) - * @return string Colon-delimited list of $keyspace followed by escaped components */ abstract public function makeKey( $class, ...$components ); + /** + * Check whether a cache key is in the global keyspace + * + * @param string $key + * @return bool + * @since 1.35 + */ + public function isKeyGlobal( $key ) { + return ( strncmp( $key, self::GLOBAL_PREFIX, self::GLOBAL_PREFIX_LEN ) === 0 ); + } + /** * @param int $flag ATTR_* class constant * @return int QOS_* class constant @@ -533,7 +579,7 @@ abstract class BagOStuff implements } /** - * Prepare values for storage and get their serialized sizes, or, estimate those sizes + * Make a "generic" reversible cache key from the given components * * All previously prepared values will be cleared. Each of the new prepared values will be * individually cleared as they get used by write operations for that key. This is done to @@ -559,6 +605,102 @@ abstract class BagOStuff implements */ abstract public function setNewPreparedValues( array $valueByKey ); + /** + * At a minimum, there must be a keyspace and collection name component + * + * @param string|int ...$components Key components for keyspace, collection name, and IDs + * @return string Keyspace-prepended list of encoded components as a colon-separated value + * @since 1.35 + */ + final protected function genericKeyFromComponents( ...$components ) { + if ( count( $components ) < 2 ) { + throw new InvalidArgumentException( "Missing keyspace or collection name" ); + } + + $key = ''; + foreach ( $components as $component ) { + if ( $key !== '' ) { + $key .= ':'; + } + // Escape delimiter (":") and escape ("%") characters + $key .= strtr( $component, [ '%' => '%25', ':' => '%3A' ] ); + } + + return $key; + } + + /** + * Extract the components from a "generic" reversible cache key + * + * @see BagOStuff::genericKeyFromComponents() + * + * @param string $key Keyspace-prepended list of encoded components as a colon-separated value + * @return string[] Key components for keyspace, collection name, and IDs + * @since 1.35 + */ + final protected function componentsFromGenericKey( $key ) { + // Note that the order of each corresponding search/replace pair matters + return str_replace( [ '%3A', '%25' ], [ ':', '%' ], explode( ':', $key ) ); + } + + /** + * Convert a "generic" reversible cache key into one for this cache + * + * @see BagOStuff::genericKeyFromComponents() + * + * @param string $key Keyspace-prepended list of encoded components as a colon-separated value + * @return string Keyspace-prepended list of encoded components as a colon-separated value + */ + abstract protected function convertGenericKey( $key ); + + /** + * Call a method on behalf of wrapper BagOStuff instance that uses "generic" keys + * + * @param string $method Name of a non-final public method that reads/changes keys + * @param int $arg0Sig BagOStuff::ARG0_* constant describing argument 0 + * @param int $resSig BagOStuff::RES_* constant describing the return value + * @param array $genericArgs Method arguments passed to the wrapper instance + * @return mixed Method result with any resulting cache keys remapped to "generic" keys + */ + protected function proxyCall( $method, $arg0Sig, $resSig, array $genericArgs ) { + // Get the corresponding store-specific cache keys... + $storeArgs = $genericArgs; + switch ( $arg0Sig ) { + case self::ARG0_KEY: + $storeArgs[0] = $this->convertGenericKey( $genericArgs[0] ); + break; + case self::ARG0_KEYARR: + foreach ( $genericArgs[0] as $i => $genericKey ) { + $storeArgs[0][$i] = $this->convertGenericKey( $genericKey ); + } + break; + case self::ARG0_KEYMAP: + $storeArgs[0] = []; + foreach ( $genericArgs[0] as $genericKey => $v ) { + $storeArgs[0][$this->convertGenericKey( $genericKey )] = $v; + } + break; + } + + // Result of invoking the method with the corresponding store-specific cache keys + $storeRes = $this->$method( ...$storeArgs ); + + // Convert any store-specific cache keys in the result back to generic cache keys + if ( $resSig === self::RES_KEYMAP ) { + // Map of (store-specific cache key => generic cache key) + $genericKeyByStoreKey = array_combine( $storeArgs[0], $genericArgs[0] ); + + $genericRes = []; + foreach ( $storeRes as $storeKey => $value ) { + $genericRes[$genericKeyByStoreKey[$storeKey]] = $value; + } + } else { + $genericRes = $storeRes; + } + + return $genericRes; + } + /** * @internal For testing only * @return float UNIX timestamp diff --git a/includes/libs/objectcache/CachedBagOStuff.php b/includes/libs/objectcache/CachedBagOStuff.php index 1b1760d836b..e53945b61b4 100644 --- a/includes/libs/objectcache/CachedBagOStuff.php +++ b/includes/libs/objectcache/CachedBagOStuff.php @@ -36,7 +36,7 @@ */ class CachedBagOStuff extends BagOStuff { /** @var BagOStuff */ - protected $backend; + protected $store; /** @var HashBagOStuff */ protected $procCache; @@ -46,65 +46,78 @@ class CachedBagOStuff extends BagOStuff { * @param array $params Parameters for HashBagOStuff */ public function __construct( BagOStuff $backend, $params = [] ) { + $params['keyspace'] = $backend->keyspace; parent::__construct( $params ); - $this->backend = $backend; + $this->store = $backend; $this->procCache = new HashBagOStuff( $params ); $this->attrMap = $backend->attrMap; } public function setDebug( $enabled ) { parent::setDebug( $enabled ); - $this->backend->setDebug( $enabled ); + $this->store->setDebug( $enabled ); } public function get( $key, $flags = 0 ) { $value = $this->procCache->get( $key, $flags ); - if ( $value === false && !$this->procCache->hasKey( $key ) ) { - $value = $this->backend->get( $key, $flags ); - $this->set( $key, $value, self::TTL_INDEFINITE, self::WRITE_CACHE_ONLY ); + if ( $value !== false || $this->procCache->hasKey( $key ) ) { + return $value; } + $value = $this->store->proxyCall( + __FUNCTION__, + self::ARG0_KEY, + self::RES_NONKEY, + func_get_args() + ); + $this->set( $key, $value, self::TTL_INDEFINITE, self::WRITE_CACHE_ONLY ); + return $value; } public function getMulti( array $keys, $flags = 0 ) { - $valuesByKeyCached = []; + $valueByKeyCached = []; - $keysMissing = []; + $keysFetch = []; foreach ( $keys as $key ) { $value = $this->procCache->get( $key, $flags ); if ( $value === false && !$this->procCache->hasKey( $key ) ) { - $keysMissing[] = $key; + $keysFetch[] = $key; } else { - $valuesByKeyCached[$key] = $value; + $valueByKeyCached[$key] = $value; } } - $valuesByKeyFetched = $this->backend->getMulti( $keysMissing, $flags ); - $this->setMulti( $valuesByKeyFetched, self::TTL_INDEFINITE, self::WRITE_CACHE_ONLY ); + $valueByKeyFetched = $this->store->proxyCall( + __FUNCTION__, + self::ARG0_KEYARR, + self::RES_KEYMAP, + [ $keysFetch, $flags ] + ); + $this->setMulti( $valueByKeyFetched, self::TTL_INDEFINITE, self::WRITE_CACHE_ONLY ); - return $valuesByKeyCached + $valuesByKeyFetched; + return $valueByKeyCached + $valueByKeyFetched; } public function set( $key, $value, $exptime = 0, $flags = 0 ) { $this->procCache->set( $key, $value, $exptime, $flags ); - if ( !$this->fieldHasFlags( $flags, self::WRITE_CACHE_ONLY ) ) { - $this->backend->set( $key, $value, $exptime, $flags ); + if ( $this->fieldHasFlags( $flags, self::WRITE_CACHE_ONLY ) ) { + return true; } - return true; + return $this->store->proxyCall( __FUNCTION__, self::ARG0_KEY, self::RES_NONKEY, func_get_args() ); } public function delete( $key, $flags = 0 ) { $this->procCache->delete( $key, $flags ); - if ( !$this->fieldHasFlags( $flags, self::WRITE_CACHE_ONLY ) ) { - $this->backend->delete( $key, $flags ); + if ( $this->fieldHasFlags( $flags, self::WRITE_CACHE_ONLY ) ) { + return true; } - return true; + return $this->store->proxyCall( __FUNCTION__, self::ARG0_KEY, self::RES_NONKEY, func_get_args() ); } public function add( $key, $value, $exptime = 0, $flags = 0 ) { @@ -121,21 +134,21 @@ class CachedBagOStuff extends BagOStuff { public function merge( $key, callable $callback, $exptime = 0, $attempts = 10, $flags = 0 ) { $this->procCache->delete( $key ); - return $this->backend->merge( $key, $callback, $exptime, $attempts, $flags ); + return $this->store->proxyCall( __FUNCTION__, self::ARG0_KEY, self::RES_NONKEY, func_get_args() ); } public function changeTTL( $key, $exptime = 0, $flags = 0 ) { $this->procCache->delete( $key ); - return $this->backend->changeTTL( $key, $exptime, $flags ); + return $this->store->proxyCall( __FUNCTION__, self::ARG0_KEY, self::RES_NONKEY, func_get_args() ); } public function lock( $key, $timeout = 6, $expiry = 6, $rclass = '' ) { - return $this->backend->lock( $key, $timeout, $expiry, $rclass ); + return $this->store->proxyCall( __FUNCTION__, self::ARG0_KEY, self::RES_NONKEY, func_get_args() ); } public function unlock( $key ) { - return $this->backend->unlock( $key ); + return $this->store->proxyCall( __FUNCTION__, self::ARG0_KEY, self::RES_NONKEY, func_get_args() ); } public function deleteObjectsExpiringBefore( @@ -145,89 +158,93 @@ class CachedBagOStuff extends BagOStuff { ) { $this->procCache->deleteObjectsExpiringBefore( $timestamp, $progress, $limit ); - return $this->backend->deleteObjectsExpiringBefore( $timestamp, $progress, $limit ); + return $this->store->proxyCall( __FUNCTION__, self::ARG0_NONKEY, self::RES_NONKEY, func_get_args() ); } public function makeKeyInternal( $keyspace, $components ) { - return $this->backend->makeKeyInternal( $keyspace, $components ); + return $this->genericKeyFromComponents( $keyspace, ...$components ); } public function makeKey( $class, ...$components ) { - return $this->backend->makeKey( $class, ...$components ); + return $this->genericKeyFromComponents( $this->keyspace, $class, ...$components ); } public function makeGlobalKey( $class, ...$components ) { - return $this->backend->makeGlobalKey( $class, ...$components ); + return $this->genericKeyFromComponents( self::GLOBAL_KEYSPACE, $class, ...$components ); + } + + protected function convertGenericKey( $key ) { + return $key; // short-circuit; already uses "generic" keys } public function getLastError() { - return $this->backend->getLastError(); + return $this->store->getLastError(); } public function clearLastError() { - return $this->backend->clearLastError(); + return $this->store->clearLastError(); } - public function setMulti( array $data, $exptime = 0, $flags = 0 ) { - $this->procCache->setMulti( $data, $exptime, $flags ); + public function setMulti( array $valueByKey, $exptime = 0, $flags = 0 ) { + $this->procCache->setMulti( $valueByKey, $exptime, $flags ); - if ( !$this->fieldHasFlags( $flags, self::WRITE_CACHE_ONLY ) ) { - return $this->backend->setMulti( $data, $exptime, $flags ); + if ( $this->fieldHasFlags( $flags, self::WRITE_CACHE_ONLY ) ) { + return true; } - return true; + return $this->store->proxyCall( __FUNCTION__, self::ARG0_KEYMAP, self::RES_NONKEY, func_get_args() ); } public function deleteMulti( array $keys, $flags = 0 ) { $this->procCache->deleteMulti( $keys, $flags ); - if ( !$this->fieldHasFlags( $flags, self::WRITE_CACHE_ONLY ) ) { - return $this->backend->deleteMulti( $keys, $flags ); + if ( $this->fieldHasFlags( $flags, self::WRITE_CACHE_ONLY ) ) { + return true; } - return true; + return $this->store->proxyCall( __FUNCTION__, self::ARG0_KEYARR, self::RES_NONKEY, func_get_args() ); } public function changeTTLMulti( array $keys, $exptime, $flags = 0 ) { $this->procCache->changeTTLMulti( $keys, $exptime, $flags ); - if ( !$this->fieldHasFlags( $flags, self::WRITE_CACHE_ONLY ) ) { - return $this->backend->changeTTLMulti( $keys, $exptime, $flags ); + if ( $this->fieldHasFlags( $flags, self::WRITE_CACHE_ONLY ) ) { + return true; } - return true; + return $this->store->proxyCall( __FUNCTION__, self::ARG0_KEYARR, self::RES_NONKEY, func_get_args() ); } public function incr( $key, $value = 1, $flags = 0 ) { $this->procCache->delete( $key ); - return $this->backend->incr( $key, $value, $flags ); + return $this->store->proxyCall( __FUNCTION__, self::ARG0_KEY, self::RES_NONKEY, func_get_args() ); } public function decr( $key, $value = 1, $flags = 0 ) { $this->procCache->delete( $key ); - return $this->backend->decr( $key, $value, $flags ); + return $this->store->proxyCall( __FUNCTION__, self::ARG0_KEY, self::RES_NONKEY, func_get_args() ); } public function incrWithInit( $key, $exptime, $value = 1, $init = null, $flags = 0 ) { $this->procCache->delete( $key ); - return $this->backend->incrWithInit( $key, $exptime, $value, $init, $flags ); + return $this->store->proxyCall( __FUNCTION__, self::ARG0_KEY, self::RES_NONKEY, func_get_args() ); } public function addBusyCallback( callable $workCallback ) { - $this->backend->addBusyCallback( $workCallback ); + $this->store->addBusyCallback( $workCallback ); } public function setNewPreparedValues( array $valueByKey ) { - return $this->backend->setNewPreparedValues( $valueByKey ); + return $this->store->proxyCall( __FUNCTION__, self::ARG0_KEYMAP, self::RES_NONKEY, func_get_args() ); } public function setMockTime( &$time ) { parent::setMockTime( $time ); $this->procCache->setMockTime( $time ); - $this->backend->setMockTime( $time ); + $this->store->setMockTime( $time ); } // @codeCoverageIgnoreEnd diff --git a/includes/libs/objectcache/EmptyBagOStuff.php b/includes/libs/objectcache/EmptyBagOStuff.php index 6ad258991f8..8731b953435 100644 --- a/includes/libs/objectcache/EmptyBagOStuff.php +++ b/includes/libs/objectcache/EmptyBagOStuff.php @@ -66,4 +66,12 @@ class EmptyBagOStuff extends MediumSpecificBagOStuff { public function merge( $key, callable $callback, $exptime = 0, $attempts = 10, $flags = 0 ) { return true; // faster } + + public function makeKeyInternal( $keyspace, $components ) { + return $this->genericKeyFromComponents( $keyspace, ...$components ); + } + + protected function convertGenericKey( $key ) { + return $key; // short-circuit; already uses "generic" keys + } } diff --git a/includes/libs/objectcache/HashBagOStuff.php b/includes/libs/objectcache/HashBagOStuff.php index e9043c1fd0d..64d12d7486e 100644 --- a/includes/libs/objectcache/HashBagOStuff.php +++ b/includes/libs/objectcache/HashBagOStuff.php @@ -177,4 +177,12 @@ class HashBagOStuff extends MediumSpecificBagOStuff { public function hasKey( $key ) { return isset( $this->bag[$key] ); } + + public function makeKeyInternal( $keyspace, $components ) { + return $this->genericKeyFromComponents( $keyspace, ...$components ); + } + + protected function convertGenericKey( $key ) { + return $key; // short-circuit; already uses "generic" keys + } } diff --git a/includes/libs/objectcache/IStoreKeyEncoder.php b/includes/libs/objectcache/IStoreKeyEncoder.php index 47732e50878..1e9a5aa8210 100644 --- a/includes/libs/objectcache/IStoreKeyEncoder.php +++ b/includes/libs/objectcache/IStoreKeyEncoder.php @@ -8,20 +8,20 @@ */ interface IStoreKeyEncoder { /** - * Make a global cache key. + * Make a cache key using the "global" keyspace for the given components * - * @param string $class Key class - * @param string|int ...$components Key components (starting with a key collection name) - * @return string Colon-delimited list of $keyspace followed by escaped components + * @param string $class Key collection name component + * @param string|int ...$components Key components for entity IDs + * @return string Keyspace-prepended list of encoded components as a colon-separated value */ public function makeGlobalKey( $class, ...$components ); /** - * Make a cache key, scoped to this instance's keyspace. + * Make a cache key using the default keyspace for the given components * - * @param string $class Key class - * @param string|int ...$components Key components (starting with a key collection name) - * @return string Colon-delimited list of $keyspace followed by escaped components + * @param string $class Key collection name component + * @param string|int ...$components Key components for entity IDs + * @return string Keyspace-prepended list of encoded components as a colon-separated value */ public function makeKey( $class, ...$components ); } diff --git a/includes/libs/objectcache/MediumSpecificBagOStuff.php b/includes/libs/objectcache/MediumSpecificBagOStuff.php index 64d40ced03b..84681c6b3df 100644 --- a/includes/libs/objectcache/MediumSpecificBagOStuff.php +++ b/includes/libs/objectcache/MediumSpecificBagOStuff.php @@ -36,8 +36,6 @@ abstract class MediumSpecificBagOStuff extends BagOStuff { protected $locks = []; /** @var int ERR_* class constant */ protected $lastError = self::ERR_NONE; - /** @var string */ - protected $keyspace = 'local'; /** @var int Seconds */ protected $syncTimeout; /** @var int Bytes; chunk size of segmented cache values */ @@ -68,7 +66,6 @@ abstract class MediumSpecificBagOStuff extends BagOStuff { * @see BagOStuff::__construct() * Additional $params options include: * - logger: Psr\Log\LoggerInterface instance - * - keyspace: Default keyspace for $this->makeKey() * - reportDupes: Whether to emit warning log messages for all keys that were * requested more than once (requires an asyncHandler). * - syncTimeout: How long to wait with WRITE_SYNC in seconds. @@ -80,16 +77,12 @@ abstract class MediumSpecificBagOStuff extends BagOStuff { * amount of I/O between application and cache servers that the network can handle. * @param array $params * @codingStandardsIgnoreStart - * @phan-param array{logger?:Psr\Log\LoggerInterface,asyncHandler?:callable,keyspace?:string,reportDupes?:bool,syncTimeout?:int,segmentationSize?:int,segmentedValueMaxSize?:int} $params + * @phan-param array{logger?:Psr\Log\LoggerInterface,asyncHandler?:callable,reportDupes?:bool,syncTimeout?:int,segmentationSize?:int,segmentedValueMaxSize?:int} $params * @codingStandardsIgnoreEnd */ public function __construct( array $params = [] ) { parent::__construct( $params ); - if ( isset( $params['keyspace'] ) ) { - $this->keyspace = $params['keyspace']; - } - if ( !empty( $params['reportDupes'] ) && is_callable( $this->asyncHandler ) ) { $this->reportDupes = true; } @@ -600,18 +593,18 @@ abstract class MediumSpecificBagOStuff extends BagOStuff { * * This does not support WRITE_ALLOW_SEGMENTS to avoid excessive read I/O * - * @param mixed[] $data Map of (key => value) + * @param mixed[] $valueByKey Map of (key => value) * @param int $exptime Either an interval in seconds or a unix timestamp for expiry * @param int $flags Bitfield of BagOStuff::WRITE_* constants (since 1.33) * @return bool Success * @since 1.24 */ - public function setMulti( array $data, $exptime = 0, $flags = 0 ) { + public function setMulti( array $valueByKey, $exptime = 0, $flags = 0 ) { if ( $this->fieldHasFlags( $flags, self::WRITE_ALLOW_SEGMENTS ) ) { throw new InvalidArgumentException( __METHOD__ . ' got WRITE_ALLOW_SEGMENTS' ); } - return $this->doSetMulti( $data, $exptime, $flags ); + return $this->doSetMulti( $valueByKey, $exptime, $flags ); } /** @@ -890,43 +883,26 @@ abstract class MediumSpecificBagOStuff extends BagOStuff { return ( $value === (string)$integer ); } - public function makeKeyInternal( $keyspace, $components ) { - $key = $keyspace; - foreach ( $components as $component ) { - $key .= ':' . str_replace( ':', '%3A', $component ); - } - return strtr( $key, ' ', '_' ); - } - - /** - * Make a global cache key. - * - * @param string $class Key class - * @param string|int ...$components Key components (starting with a key collection name) - * @return string Colon-delimited list of $keyspace followed by escaped components - * @since 1.27 - */ public function makeGlobalKey( $class, ...$components ) { - return $this->makeKeyInternal( 'global', func_get_args() ); + return $this->makeKeyInternal( self::GLOBAL_KEYSPACE, func_get_args() ); } - /** - * Make a cache key, scoped to this instance's keyspace. - * - * @param string $class Key class - * @param string|int ...$components Key components (starting with a key collection name) - * @return string Colon-delimited list of $keyspace followed by escaped components - * @since 1.27 - */ public function makeKey( $class, ...$components ) { return $this->makeKeyInternal( $this->keyspace, func_get_args() ); } - /** - * @param int $flag ATTR_* class constant - * @return int QOS_* class constant - * @since 1.28 - */ + protected function convertGenericKey( $key ) { + $components = $this->componentsFromGenericKey( $key ); + if ( count( $components ) < 2 ) { + // Legacy key not from makeKey()/makeGlobalKey(); keep it as-is + return $key; + } + + $keyspace = array_shift( $components ); + + return $this->makeKeyInternal( $keyspace, $components ); + } + public function getQoS( $flag ) { return $this->attrMap[$flag] ?? self::QOS_UNKNOWN; } diff --git a/includes/libs/objectcache/MultiWriteBagOStuff.php b/includes/libs/objectcache/MultiWriteBagOStuff.php index e0c535df9a5..c3ed54d0d4c 100644 --- a/includes/libs/objectcache/MultiWriteBagOStuff.php +++ b/includes/libs/objectcache/MultiWriteBagOStuff.php @@ -34,8 +34,9 @@ use Wikimedia\ObjectFactory; * @ingroup Cache */ class MultiWriteBagOStuff extends BagOStuff { - /** @var BagOStuff[] */ + /** @var BagOStuff[] Backing cache stores in order of highest to lowest tier */ protected $caches; + /** @var bool Use async secondary writes */ protected $asyncWrites = false; /** @var int[] List of all backing cache indexes */ @@ -111,17 +112,31 @@ class MultiWriteBagOStuff extends BagOStuff { } public function get( $key, $flags = 0 ) { + $args = func_get_args(); + if ( $this->fieldHasFlags( $flags, self::READ_LATEST ) ) { // If the latest write was a delete(), we do NOT want to fallback // to the other tiers and possibly see the old value. Also, this // is used by merge(), which only needs to hit the primary. - return $this->caches[0]->get( $key, $flags ); + return $this->callKeyMethodOnTierCache( + 0, + __FUNCTION__, + self::ARG0_KEY, + self::RES_NONKEY, + $args + ); } $value = false; $missIndexes = []; // backends checked - foreach ( $this->caches as $i => $cache ) { - $value = $cache->get( $key, $flags ); + foreach ( $this->cacheIndexes as $i ) { + $value = $this->callKeyMethodOnTierCache( + $i, + __FUNCTION__, + self::ARG0_KEY, + self::RES_NONKEY, + $args + ); if ( $value !== false ) { break; } @@ -134,11 +149,12 @@ class MultiWriteBagOStuff extends BagOStuff { $missIndexes ) { // Backfill the value to the higher (and often faster/smaller) cache tiers - $this->doWrite( + $this->callKeyWriteMethodOnTierCaches( $missIndexes, $this->asyncWrites, 'set', - // @TODO: consider using self::WRITE_ALLOW_SEGMENTS here? + self::ARG0_KEY, + self::RES_NONKEY, [ $key, $value, self::$UPGRADE_TTL ] ); } @@ -147,73 +163,99 @@ class MultiWriteBagOStuff extends BagOStuff { } public function set( $key, $value, $exptime = 0, $flags = 0 ) { - return $this->doWrite( + return $this->callKeyWriteMethodOnTierCaches( $this->cacheIndexes, - $this->usesAsyncWritesGivenFlags( $flags ), + $this->useAsyncSecondaryWrites( $flags ), __FUNCTION__, + self::ARG0_KEY, + self::RES_NONKEY, func_get_args() ); } public function delete( $key, $flags = 0 ) { - return $this->doWrite( + return $this->callKeyWriteMethodOnTierCaches( $this->cacheIndexes, - $this->usesAsyncWritesGivenFlags( $flags ), + $this->useAsyncSecondaryWrites( $flags ), __FUNCTION__, + self::ARG0_KEY, + self::RES_NONKEY, func_get_args() ); } public function add( $key, $value, $exptime = 0, $flags = 0 ) { // Try the write to the top-tier cache - $ok = $this->doWrite( - [ 0 ], - $this->usesAsyncWritesGivenFlags( $flags ), + $ok = $this->callKeyMethodOnTierCache( + 0, __FUNCTION__, + self::ARG0_KEY, + self::RES_NONKEY, func_get_args() ); if ( $ok ) { // Relay the add() using set() if it succeeded. This is meant to handle certain // migration scenarios where the same store might get written to twice for certain - // keys. In that case, it does not make sense to return false due to "self-conflicts". - return $this->doWrite( + // keys. In that case, it makes no sense to return false due to "self-conflicts". + $okSecondaries = $this->callKeyWriteMethodOnTierCaches( array_slice( $this->cacheIndexes, 1 ), - $this->usesAsyncWritesGivenFlags( $flags ), + $this->useAsyncSecondaryWrites( $flags ), 'set', + self::ARG0_KEY, + self::RES_NONKEY, [ $key, $value, $exptime, $flags ] ); + if ( $okSecondaries === false ) { + $ok = false; + } } - return false; + return $ok; } public function merge( $key, callable $callback, $exptime = 0, $attempts = 10, $flags = 0 ) { - return $this->doWrite( + return $this->callKeyWriteMethodOnTierCaches( $this->cacheIndexes, - $this->usesAsyncWritesGivenFlags( $flags ), + $this->useAsyncSecondaryWrites( $flags ), __FUNCTION__, + self::ARG0_KEY, + self::RES_NONKEY, func_get_args() ); } public function changeTTL( $key, $exptime = 0, $flags = 0 ) { - return $this->doWrite( + return $this->callKeyWriteMethodOnTierCaches( $this->cacheIndexes, - $this->usesAsyncWritesGivenFlags( $flags ), + $this->useAsyncSecondaryWrites( $flags ), __FUNCTION__, + self::ARG0_KEY, + self::RES_NONKEY, func_get_args() ); } public function lock( $key, $timeout = 6, $expiry = 6, $rclass = '' ) { // Only need to lock the first cache; also avoids deadlocks - return $this->caches[0]->lock( $key, $timeout, $expiry, $rclass ); + return $this->callKeyMethodOnTierCache( + 0, + __FUNCTION__, + self::ARG0_KEY, + self::RES_NONKEY, + func_get_args() + ); } public function unlock( $key ) { // Only the first cache is locked - return $this->caches[0]->unlock( $key ); + return $this->callKeyMethodOnTierCache( + 0, + __FUNCTION__, + self::ARG0_KEY, + self::RES_NONKEY, + func_get_args() + ); } public function deleteObjectsExpiringBefore( @@ -244,127 +286,103 @@ class MultiWriteBagOStuff extends BagOStuff { return $res; } - public function setMulti( array $data, $exptime = 0, $flags = 0 ) { - return $this->doWrite( + public function setMulti( array $valueByKey, $exptime = 0, $flags = 0 ) { + return $this->callKeyWriteMethodOnTierCaches( $this->cacheIndexes, - $this->usesAsyncWritesGivenFlags( $flags ), + $this->useAsyncSecondaryWrites( $flags ), __FUNCTION__, + self::ARG0_KEYMAP, + self::RES_NONKEY, func_get_args() ); } - public function deleteMulti( array $data, $flags = 0 ) { - return $this->doWrite( + public function deleteMulti( array $keys, $flags = 0 ) { + return $this->callKeyWriteMethodOnTierCaches( $this->cacheIndexes, - $this->usesAsyncWritesGivenFlags( $flags ), + $this->useAsyncSecondaryWrites( $flags ), __FUNCTION__, + self::ARG0_KEYARR, + self::RES_NONKEY, func_get_args() ); } public function changeTTLMulti( array $keys, $exptime, $flags = 0 ) { - return $this->doWrite( + return $this->callKeyWriteMethodOnTierCaches( $this->cacheIndexes, - $this->usesAsyncWritesGivenFlags( $flags ), + $this->useAsyncSecondaryWrites( $flags ), __FUNCTION__, + self::ARG0_KEYARR, + self::RES_NONKEY, func_get_args() ); } public function incr( $key, $value = 1, $flags = 0 ) { - return $this->doWrite( + return $this->callKeyWriteMethodOnTierCaches( $this->cacheIndexes, - $this->asyncWrites, + $this->useAsyncSecondaryWrites( $flags ), __FUNCTION__, + self::ARG0_KEY, + self::RES_NONKEY, func_get_args() ); } public function decr( $key, $value = 1, $flags = 0 ) { - return $this->doWrite( + return $this->callKeyWriteMethodOnTierCaches( $this->cacheIndexes, - $this->asyncWrites, + $this->useAsyncSecondaryWrites( $flags ), __FUNCTION__, + self::ARG0_KEY, + self::RES_NONKEY, func_get_args() ); } public function incrWithInit( $key, $exptime, $value = 1, $init = null, $flags = 0 ) { - return $this->doWrite( + return $this->callKeyWriteMethodOnTierCaches( $this->cacheIndexes, - $this->asyncWrites, + $this->useAsyncSecondaryWrites( $flags ), __FUNCTION__, + self::ARG0_KEY, + self::RES_NONKEY, func_get_args() ); } public function getLastError() { - return $this->caches[0]->getLastError(); - } - - public function clearLastError() { - $this->caches[0]->clearLastError(); - } - - /** - * Apply a write method to the backing caches specified by $indexes (in order) - * - * @param int[] $indexes List of backing cache indexes - * @param bool $asyncWrites - * @param string $method Method name of backing caches - * @param array $args Arguments to the method of backing caches - * @return bool - */ - protected function doWrite( $indexes, $asyncWrites, $method, array $args ) { - $ret = true; - - if ( array_diff( $indexes, [ 0 ] ) && $asyncWrites && $method !== 'merge' ) { - // Deep-clone $args to prevent misbehavior when something writes an - // object to the BagOStuff then modifies it afterwards, e.g. T168040. - $args = unserialize( serialize( $args ) ); - } - - foreach ( $indexes as $i ) { - $cache = $this->caches[$i]; - if ( $i == 0 || !$asyncWrites ) { - // First store or in sync mode: write now and get result - if ( !$cache->$method( ...$args ) ) { - $ret = false; - } - } else { - // Secondary write in async mode: do not block this HTTP request - $logger = $this->logger; - ( $this->asyncHandler )( - function () use ( $cache, $method, $args, $logger ) { - if ( !$cache->$method( ...$args ) ) { - $logger->warning( "Async $method op failed" ); - } - } - ); + foreach ( $this->caches as $cache ) { + $error = $cache->getLastError(); + if ( $error !== self::ERR_NONE ) { + return $error; } } - return $ret; + return self::ERR_NONE; } - /** - * @param int $flags - * @return bool - */ - protected function usesAsyncWritesGivenFlags( $flags ) { - return $this->fieldHasFlags( $flags, self::WRITE_SYNC ) ? false : $this->asyncWrites; + public function clearLastError() { + foreach ( $this->caches as $cache ) { + $cache->clearLastError(); + } } public function makeKeyInternal( $keyspace, $components ) { - return $this->caches[0]->makeKeyInternal( $keyspace, $components ); + return $this->genericKeyFromComponents( $keyspace, ...$components ); } public function makeKey( $class, ...$components ) { - return $this->caches[0]->makeKey( ...func_get_args() ); + return $this->genericKeyFromComponents( $this->keyspace, $class, ...$components ); } public function makeGlobalKey( $class, ...$components ) { - return $this->caches[0]->makeGlobalKey( ...func_get_args() ); + return $this->genericKeyFromComponents( self::GLOBAL_KEYSPACE, $class, ...$components ); + } + + protected function convertGenericKey( $key ) { + return $key; // short-circuit; already uses "generic" keys } public function addBusyCallback( callable $workCallback ) { @@ -372,7 +390,13 @@ class MultiWriteBagOStuff extends BagOStuff { } public function setNewPreparedValues( array $valueByKey ) { - return $this->caches[0]->setNewPreparedValues( $valueByKey ); + return $this->callKeyMethodOnTierCache( + 0, + __FUNCTION__, + self::ARG0_KEYMAP, + self::RES_NONKEY, + func_get_args() + ); } public function setMockTime( &$time ) { @@ -381,4 +405,77 @@ class MultiWriteBagOStuff extends BagOStuff { $cache->setMockTime( $time ); } } + + /** + * Call a method on the cache instance for the given cache tier (index) + * + * @param int $index Cache tier + * @param string $method Method name + * @param int $arg0Sig BagOStuff::A0_* constant describing argument 0 + * @param int $rvSig BagOStuff::RV_* constant describing the return value + * @param array $args Method arguments + * @return mixed The result of calling the given method + */ + private function callKeyMethodOnTierCache( $index, $method, $arg0Sig, $rvSig, array $args ) { + return $this->caches[$index]->proxyCall( $method, $arg0Sig, $rvSig, $args ); + } + + /** + * Call a write method on the cache instances, in order, for the given tiers (indexes) + * + * @param int[] $indexes List of cache tiers + * @param bool $asyncSecondary Whether to use asynchronous writes for secondary tiers + * @param string $method Method name + * @param int $arg0Sig BagOStuff::ARG0_* constant describing argument 0 + * @param int $resSig BagOStuff::RES_* constant describing the return value + * @param array $args Method arguments + * @return mixed First synchronous result or false if any failed; null if all asynchronous + */ + private function callKeyWriteMethodOnTierCaches( + array $indexes, + $asyncSecondary, + $method, + $arg0Sig, + $resSig, + array $args + ) { + $res = null; + + if ( $asyncSecondary && array_diff( $indexes, [ 0 ] ) && $method !== 'merge' ) { + // Deep-clone $args to prevent misbehavior when something writes an + // object to the BagOStuff then modifies it afterwards, e.g. T168040. + $args = unserialize( serialize( $args ) ); + } + + foreach ( $indexes as $i ) { + $cache = $this->caches[$i]; + + if ( $i == 0 || !$asyncSecondary ) { + // Tier 0 store or in sync mode: write synchronously and get result + $storeRes = $cache->proxyCall( $method, $arg0Sig, $resSig, $args ); + if ( $storeRes === false ) { + $res = false; + } elseif ( $res === null ) { + $res = $storeRes; // first synchronous result + } + } else { + // Secondary write in async mode: do not block this HTTP request + ( $this->asyncHandler )( + function () use ( $cache, $method, $arg0Sig, $resSig, $args ) { + $cache->proxyCall( $method, $arg0Sig, $resSig, $args ); + } + ); + } + } + + return $res; + } + + /** + * @param int $flags + * @return bool + */ + private function useAsyncSecondaryWrites( $flags ) { + return $this->fieldHasFlags( $flags, self::WRITE_SYNC ) ? false : $this->asyncWrites; + } } diff --git a/includes/libs/objectcache/RESTBagOStuff.php b/includes/libs/objectcache/RESTBagOStuff.php index 266fa06aaba..c951d07e5e0 100644 --- a/includes/libs/objectcache/RESTBagOStuff.php +++ b/includes/libs/objectcache/RESTBagOStuff.php @@ -222,6 +222,14 @@ class RESTBagOStuff extends MediumSpecificBagOStuff { return $this->incr( $key, -$value, $flags ); } + public function makeKeyInternal( $keyspace, $components ) { + return $this->genericKeyFromComponents( $keyspace, ...$components ); + } + + protected function convertGenericKey( $key ) { + return $key; // short-circuit; already uses "generic" keys + } + /** * Processes the response body. * diff --git a/includes/libs/objectcache/RedisBagOStuff.php b/includes/libs/objectcache/RedisBagOStuff.php index 328f5cfa782..e8cef0b562b 100644 --- a/includes/libs/objectcache/RedisBagOStuff.php +++ b/includes/libs/objectcache/RedisBagOStuff.php @@ -521,4 +521,12 @@ class RedisBagOStuff extends MediumSpecificBagOStuff { public function logRequest( $op, $keys, $server, $e = null ) { $this->debug( "$op($keys) on $server: " . ( $e ? "failure" : "success" ) ); } + + public function makeKeyInternal( $keyspace, $components ) { + return $this->genericKeyFromComponents( $keyspace, ...$components ); + } + + protected function convertGenericKey( $key ) { + return $key; // short-circuit; already uses "generic" keys + } } diff --git a/includes/libs/objectcache/ReplicatedBagOStuff.php b/includes/libs/objectcache/ReplicatedBagOStuff.php index 7dc4fe33385..8cceb310334 100644 --- a/includes/libs/objectcache/ReplicatedBagOStuff.php +++ b/includes/libs/objectcache/ReplicatedBagOStuff.php @@ -85,50 +85,89 @@ class ReplicatedBagOStuff extends BagOStuff { } public function get( $key, $flags = 0 ) { - return ( + $store = ( $this->hadRecentSessionWrite( [ $key ] ) || $this->fieldHasFlags( $flags, self::READ_LATEST ) ) - ? $this->writeStore->get( $key, $flags ) - : $this->readStore->get( $key, $flags ); + // Try to maintain session consistency and respect READ_LATEST + ? $this->writeStore + // Otherwise, just use the default "read" store + : $this->readStore; + + return $store->proxyCall( __FUNCTION__, self::ARG0_KEY, self::RES_NONKEY, func_get_args() ); } public function set( $key, $value, $exptime = 0, $flags = 0 ) { $this->remarkRecentSessionWrite( [ $key ] ); - return $this->writeStore->set( $key, $value, $exptime, $flags ); + return $this->writeStore->proxyCall( + __FUNCTION__, + self::ARG0_KEY, + self::RES_NONKEY, + func_get_args() + ); } public function delete( $key, $flags = 0 ) { $this->remarkRecentSessionWrite( [ $key ] ); - return $this->writeStore->delete( $key, $flags ); + return $this->writeStore->proxyCall( + __FUNCTION__, + self::ARG0_KEY, + self::RES_NONKEY, + func_get_args() + ); } public function add( $key, $value, $exptime = 0, $flags = 0 ) { $this->remarkRecentSessionWrite( [ $key ] ); - return $this->writeStore->add( $key, $value, $exptime, $flags ); + return $this->writeStore->proxyCall( + __FUNCTION__, + self::ARG0_KEY, + self::RES_NONKEY, + func_get_args() + ); } public function merge( $key, callable $callback, $exptime = 0, $attempts = 10, $flags = 0 ) { $this->remarkRecentSessionWrite( [ $key ] ); - return $this->writeStore->merge( $key, $callback, $exptime, $attempts, $flags ); + return $this->writeStore->proxyCall( + __FUNCTION__, + self::ARG0_KEY, + self::RES_NONKEY, + func_get_args() + ); } public function changeTTL( $key, $exptime = 0, $flags = 0 ) { $this->remarkRecentSessionWrite( [ $key ] ); - return $this->writeStore->changeTTL( $key, $exptime, $flags ); + return $this->writeStore->proxyCall( + __FUNCTION__, + self::ARG0_KEY, + self::RES_NONKEY, + func_get_args() + ); } public function lock( $key, $timeout = 6, $expiry = 6, $rclass = '' ) { - return $this->writeStore->lock( $key, $timeout, $expiry, $rclass ); + return $this->writeStore->proxyCall( + __FUNCTION__, + self::ARG0_KEY, + self::RES_NONKEY, + func_get_args() + ); } public function unlock( $key ) { - return $this->writeStore->unlock( $key ); + return $this->writeStore->proxyCall( + __FUNCTION__, + self::ARG0_KEY, + self::RES_NONKEY, + func_get_args() + ); } public function deleteObjectsExpiringBefore( @@ -136,52 +175,91 @@ class ReplicatedBagOStuff extends BagOStuff { callable $progress = null, $limit = INF ) { - return $this->writeStore->deleteObjectsExpiringBefore( $timestamp, $progress, $limit ); + return $this->writeStore->proxyCall( + __FUNCTION__, + self::ARG0_NONKEY, + self::RES_NONKEY, + func_get_args() + ); } public function getMulti( array $keys, $flags = 0 ) { - return ( + $store = ( $this->hadRecentSessionWrite( $keys ) || $this->fieldHasFlags( $flags, self::READ_LATEST ) ) - ? $this->writeStore->getMulti( $keys, $flags ) - : $this->readStore->getMulti( $keys, $flags ); + // Try to maintain session consistency and respect READ_LATEST + ? $this->writeStore + // Otherwise, just use the default "read" store + : $this->readStore; + + return $store->proxyCall( __FUNCTION__, self::ARG0_KEYARR, self::RES_KEYMAP, func_get_args() ); } - public function setMulti( array $data, $exptime = 0, $flags = 0 ) { - $this->remarkRecentSessionWrite( array_keys( $data ) ); + public function setMulti( array $valueByKey, $exptime = 0, $flags = 0 ) { + $this->remarkRecentSessionWrite( array_keys( $valueByKey ) ); - return $this->writeStore->setMulti( $data, $exptime, $flags ); + return $this->writeStore->proxyCall( + __FUNCTION__, + self::ARG0_KEYMAP, + self::RES_KEYMAP, + func_get_args() + ); } public function deleteMulti( array $keys, $flags = 0 ) { $this->remarkRecentSessionWrite( $keys ); - return $this->writeStore->deleteMulti( $keys, $flags ); + return $this->writeStore->proxyCall( + __FUNCTION__, + self::ARG0_KEYARR, + self::RES_NONKEY, + func_get_args() + ); } public function changeTTLMulti( array $keys, $exptime, $flags = 0 ) { $this->remarkRecentSessionWrite( $keys ); - return $this->writeStore->changeTTLMulti( $keys, $exptime, $flags ); + return $this->writeStore->proxyCall( + __FUNCTION__, + self::ARG0_KEYARR, + self::RES_NONKEY, + func_get_args() + ); } public function incr( $key, $value = 1, $flags = 0 ) { $this->remarkRecentSessionWrite( [ $key ] ); - return $this->writeStore->incr( $key, $value, $flags ); + return $this->writeStore->proxyCall( + __FUNCTION__, + self::ARG0_KEY, + self::RES_NONKEY, + func_get_args() + ); } public function decr( $key, $value = 1, $flags = 0 ) { $this->remarkRecentSessionWrite( [ $key ] ); - return $this->writeStore->decr( $key, $value, $flags ); + return $this->writeStore->proxyCall( + __FUNCTION__, + self::ARG0_KEY, + self::RES_NONKEY, + func_get_args() + ); } public function incrWithInit( $key, $exptime, $value = 1, $init = null, $flags = 0 ) { $this->remarkRecentSessionWrite( [ $key ] ); - return $this->writeStore->incrWithInit( $key, $exptime, $value, $init, $flags ); + return $this->writeStore->proxyCall( + __FUNCTION__, + self::ARG0_KEY, + self::RES_NONKEY, + func_get_args() + ); } public function getLastError() { @@ -196,23 +274,32 @@ class ReplicatedBagOStuff extends BagOStuff { } public function makeKeyInternal( $keyspace, $components ) { - return $this->writeStore->makeKeyInternal( ...func_get_args() ); + return $this->genericKeyFromComponents( $keyspace, ...$components ); } public function makeKey( $class, ...$components ) { - return $this->writeStore->makeKey( ...func_get_args() ); + return $this->genericKeyFromComponents( $this->keyspace, $class, ...$components ); } public function makeGlobalKey( $class, ...$components ) { - return $this->writeStore->makeGlobalKey( ...func_get_args() ); + return $this->genericKeyFromComponents( self::GLOBAL_KEYSPACE, $class, ...$components ); + } + + protected function convertGenericKey( $key ) { + return $key; // short-circuit; already uses "generic" keys } public function addBusyCallback( callable $workCallback ) { - $this->writeStore->addBusyCallback( $workCallback ); + return $this->writeStore->addBusyCallback( $workCallback ); } public function setNewPreparedValues( array $valueByKey ) { - return $this->writeStore->setNewPreparedValues( $valueByKey ); + return $this->writeStore->proxyCall( + __FUNCTION__, + self::ARG0_KEYMAP, + self::RES_NONKEY, + func_get_args() + ); } public function setMockTime( &$time ) { diff --git a/includes/libs/objectcache/wancache/WANObjectCache.php b/includes/libs/objectcache/wancache/WANObjectCache.php index dd661257ecd..d01a47493c7 100644 --- a/includes/libs/objectcache/wancache/WANObjectCache.php +++ b/includes/libs/objectcache/wancache/WANObjectCache.php @@ -2161,7 +2161,7 @@ class WANObjectCache implements /** * @see BagOStuff::makeKey() - * @param string $class Key class + * @param string $class Key collection name * @param string|int ...$components Key components (starting with a key collection name) * @return string Colon-delimited list of $keyspace followed by escaped components * @since 1.27 @@ -2172,7 +2172,7 @@ class WANObjectCache implements /** * @see BagOStuff::makeGlobalKey() - * @param string $class Key class + * @param string $class Key collection name * @param string|int ...$components Key components (starting with a key collection name) * @return string Colon-delimited list of $keyspace followed by escaped components * @since 1.27 diff --git a/tests/phpunit/includes/libs/objectcache/BagOStuffTestBase.php b/tests/phpunit/includes/libs/objectcache/BagOStuffTestBase.php index f8f6d355e6b..181f8a743d0 100644 --- a/tests/phpunit/includes/libs/objectcache/BagOStuffTestBase.php +++ b/tests/phpunit/includes/libs/objectcache/BagOStuffTestBase.php @@ -62,6 +62,19 @@ abstract class BagOStuffTestBase extends MediaWikiIntegrationTestCase { ); } + /** + * @covers MediumSpecificBagOStuff::isKeyGlobal + */ + public function testKeyIsGlobal() { + $cache = new HashBagOStuff(); + + $localKey = $cache->makeKey( 'first', 'second', 'third' ); + $globalKey = $cache->makeGlobalKey( 'first', 'second', 'third' ); + + $this->assertFalse( $cache->isKeyGlobal( $localKey ) ); + $this->assertTrue( $cache->isKeyGlobal( $globalKey ) ); + } + /** * @covers MediumSpecificBagOStuff::merge * @covers MediumSpecificBagOStuff::mergeViaCas diff --git a/tests/phpunit/includes/libs/objectcache/CachedBagOStuffTest.php b/tests/phpunit/includes/libs/objectcache/CachedBagOStuffTest.php index 5445e25ec9d..62ab00812be 100644 --- a/tests/phpunit/includes/libs/objectcache/CachedBagOStuffTest.php +++ b/tests/phpunit/includes/libs/objectcache/CachedBagOStuffTest.php @@ -126,19 +126,20 @@ class CachedBagOStuffTest extends PHPUnit\Framework\TestCase { */ public function testMakeKey() { $backend = $this->getMockBuilder( HashBagOStuff::class ) + ->setConstructorArgs( [ [ 'keyspace' => 'magic' ] ] ) ->setMethods( [ 'makeKey' ] ) ->getMock(); $backend->method( 'makeKey' ) ->willReturn( 'special/logic' ); - // CachedBagOStuff wraps any backend with a process cache - // using HashBagOStuff. Hash has no special key limitations, - // but backends often do. Make sure it uses the backend's - // makeKey() logic, not the one inherited from HashBagOStuff $cache = new CachedBagOStuff( $backend ); - $this->assertEquals( 'special/logic', $backend->makeKey( 'special', 'logic' ) ); - $this->assertEquals( 'special/logic', $cache->makeKey( 'special', 'logic' ) ); + $this->assertSame( 'special/logic', $backend->makeKey( 'special', 'logic' ) ); + $this->assertSame( + 'magic:special:logic', + $cache->makeKey( 'special', 'logic' ), + "Backend keyspace used" + ); } /** @@ -146,6 +147,7 @@ class CachedBagOStuffTest extends PHPUnit\Framework\TestCase { */ public function testMakeGlobalKey() { $backend = $this->getMockBuilder( HashBagOStuff::class ) + ->setConstructorArgs( [ [ 'keyspace' => 'magic' ] ] ) ->setMethods( [ 'makeGlobalKey' ] ) ->getMock(); $backend->method( 'makeGlobalKey' ) @@ -153,7 +155,7 @@ class CachedBagOStuffTest extends PHPUnit\Framework\TestCase { $cache = new CachedBagOStuff( $backend ); - $this->assertEquals( 'special/logic', $backend->makeGlobalKey( 'special', 'logic' ) ); - $this->assertEquals( 'special/logic', $cache->makeGlobalKey( 'special', 'logic' ) ); + $this->assertSame( 'special/logic', $backend->makeGlobalKey( 'special', 'logic' ) ); + $this->assertSame( 'global:special:logic', $cache->makeGlobalKey( 'special', 'logic' ) ); } } diff --git a/tests/phpunit/includes/libs/objectcache/MultiWriteBagOStuffTest.php b/tests/phpunit/includes/libs/objectcache/MultiWriteBagOStuffTest.php index 9af4dd54f6c..8eec8bf51cd 100644 --- a/tests/phpunit/includes/libs/objectcache/MultiWriteBagOStuffTest.php +++ b/tests/phpunit/includes/libs/objectcache/MultiWriteBagOStuffTest.php @@ -25,17 +25,31 @@ class MultiWriteBagOStuffTest extends MediaWikiIntegrationTestCase { /** * @covers MultiWriteBagOStuff::set - * @covers MultiWriteBagOStuff::doWrite */ - public function testSetImmediate() { + public function testSet() { $key = 'key'; $value = 'value'; $this->cache->set( $key, $value ); // Set in tier 1 - $this->assertEquals( $value, $this->cache1->get( $key ), 'Written to tier 1' ); + $this->assertSame( $value, $this->cache1->get( $key ), 'Written to tier 1' ); // Set in tier 2 - $this->assertEquals( $value, $this->cache2->get( $key ), 'Written to tier 2' ); + $this->assertSame( $value, $this->cache2->get( $key ), 'Written to tier 2' ); + } + + /** + * @covers MultiWriteBagOStuff::add + */ + public function testAdd() { + $key = 'key'; + $value = 'value'; + $ok = $this->cache->add( $key, $value ); + + $this->assertTrue( $ok ); + // Set in tier 1 + $this->assertSame( $value, $this->cache1->get( $key ), 'Written to tier 1' ); + // Set in tier 2 + $this->assertSame( $value, $this->cache2->get( $key ), 'Written to tier 2' ); } /** @@ -109,15 +123,18 @@ class MultiWriteBagOStuffTest extends MediaWikiIntegrationTestCase { public function testMakeKey() { $cache1 = $this->getMockBuilder( HashBagOStuff::class ) ->setMethods( [ 'makeKey' ] )->getMock(); - $cache1->expects( $this->once() )->method( 'makeKey' ) - ->willReturn( 'special' ); + $cache1->expects( $this->never() )->method( 'makeKey' ); $cache2 = $this->getMockBuilder( HashBagOStuff::class ) ->setMethods( [ 'makeKey' ] )->getMock(); $cache2->expects( $this->never() )->method( 'makeKey' ); - $cache = new MultiWriteBagOStuff( [ 'caches' => [ $cache1, $cache2 ] ] ); - $this->assertSame( 'special', $cache->makeKey( 'a', 'b' ) ); + $cache = new MultiWriteBagOStuff( [ + 'keyspace' => 'generic', + 'caches' => [ $cache1, $cache2 ] + ] ); + + $this->assertSame( 'generic:a:b', $cache->makeKey( 'a', 'b' ) ); } /** @@ -126,8 +143,7 @@ class MultiWriteBagOStuffTest extends MediaWikiIntegrationTestCase { public function testMakeGlobalKey() { $cache1 = $this->getMockBuilder( HashBagOStuff::class ) ->setMethods( [ 'makeGlobalKey' ] )->getMock(); - $cache1->expects( $this->once() )->method( 'makeGlobalKey' ) - ->willReturn( 'special' ); + $cache1->expects( $this->never() )->method( 'makeGlobalKey' ); $cache2 = $this->getMockBuilder( HashBagOStuff::class ) ->setMethods( [ 'makeGlobalKey' ] )->getMock(); @@ -135,7 +151,7 @@ class MultiWriteBagOStuffTest extends MediaWikiIntegrationTestCase { $cache = new MultiWriteBagOStuff( [ 'caches' => [ $cache1, $cache2 ] ] ); - $this->assertSame( 'special', $cache->makeGlobalKey( 'a', 'b' ) ); + $this->assertSame( 'global:a:b', $cache->makeGlobalKey( 'a', 'b' ) ); } /** @@ -149,4 +165,50 @@ class MultiWriteBagOStuffTest extends MediaWikiIntegrationTestCase { $this->assertTrue( $cache->add( 'key', 1, 30 ) ); } + + /** + * @covers MultiWriteBagOStuff::incr + */ + public function testIncr() { + $key = $this->cache->makeKey( 'key' ); + + $this->cache->add( $key, 7, 30 ); + + $value = $this->cache->incr( $key ); + $this->assertSame( 8, $value, 'Value after incrementing' ); + + $value = $this->cache->get( $key ); + $this->assertSame( 8, $value, 'Value after incrementing' ); + } + + /** + * @covers MultiWriteBagOStuff::decr + */ + public function testDecr() { + $key = $this->cache->makeKey( 'key' ); + + $this->cache->add( $key, 10, 30 ); + + $value = $this->cache->decr( $key ); + $this->assertSame( 9, $value, 'Value after decrementing' ); + + $value = $this->cache->get( $key ); + $this->assertSame( 9, $value, 'Value after decrementing' ); + } + + /** + * @covers MultiWriteBagOStuff::incrWithInit + */ + public function testIncrWithInit() { + $key = $this->cache->makeKey( 'key' ); + $val = $this->cache->incrWithInit( $key, 0, 1, 3 ); + $this->assertSame( 3, $val, "Correct init value" ); + + $val = $this->cache->incrWithInit( $key, 0, 1, 3 ); + $this->assertSame( 4, $val, "Correct init value" ); + $this->cache->delete( $key ); + + $val = $this->cache->incrWithInit( $key, 0, 5 ); + $this->assertSame( 5, $val, "Correct init value" ); + } } diff --git a/tests/phpunit/includes/session/TestBagOStuff.php b/tests/phpunit/includes/session/TestBagOStuff.php index 64148b02e46..2f6f025be5f 100644 --- a/tests/phpunit/includes/session/TestBagOStuff.php +++ b/tests/phpunit/includes/session/TestBagOStuff.php @@ -72,7 +72,7 @@ class TestBagOStuff extends CachedBagOStuff { * @return mixed */ public function getSessionFromBackend( $id ) { - return $this->backend->get( $this->makeKey( 'MWSession', $id ) ); + return $this->store->get( $this->makeKey( 'MWSession', $id ) ); } /** diff --git a/tests/phpunit/unit/includes/libs/objectcache/ReplicatedBagOStuffTest.php b/tests/phpunit/unit/includes/libs/objectcache/ReplicatedBagOStuffTest.php index 446eef35161..bcea31bcf9f 100644 --- a/tests/phpunit/unit/includes/libs/objectcache/ReplicatedBagOStuffTest.php +++ b/tests/phpunit/unit/includes/libs/objectcache/ReplicatedBagOStuffTest.php @@ -14,6 +14,7 @@ class ReplicatedBagOStuffTest extends \MediaWikiUnitTestCase { $this->writeCache = new HashBagOStuff(); $this->readCache = new HashBagOStuff(); $this->cache = new ReplicatedBagOStuff( [ + 'keyspace' => 'repl_local', 'writeFactory' => $this->writeCache, 'readFactory' => $this->readCache, ] ); @@ -23,7 +24,68 @@ class ReplicatedBagOStuffTest extends \MediaWikiUnitTestCase { * @covers ReplicatedBagOStuff::set */ public function testSet() { - $key = 'a key'; + $key = $this->cache->makeKey( 'a', 'key' ); + $value = 'a value'; + + $this->cache->set( $key, $value ); + + $this->assertSame( $value, $this->writeCache->get( $key ), 'Written' ); + $this->assertFalse( $this->readCache->get( $key ), 'Async replication' ); + } + + /** + * @covers ReplicatedBagOStuff::get + */ + public function testGet() { + $key = $this->cache->makeKey( 'a', 'key' ); + + $write = 'new value'; + $this->writeCache->set( $key, $write ); + $read = 'old value'; + $this->readCache->set( $key, $read ); + + $this->assertSame( $read, $this->cache->get( $key ), 'Async replication' ); + } + + /** + * @covers ReplicatedBagOStuff::get + */ + public function testGetAbsent() { + $key = $this->cache->makeKey( 'a', 'key' ); + $value = 'a value'; + $this->writeCache->set( $key, $value ); + + $this->assertFalse( $this->cache->get( $key ), 'Async replication' ); + } + + /** + * @covers ReplicatedBagOStuff::setMulti + * @covers ReplicatedBagOStuff::getMulti + */ + public function testGetSetMulti() { + $keyA = $this->cache->makeKey( 'key', 'a' ); + $keyB = $this->cache->makeKey( 'key', 'b' ); + $valueAOld = 'one old value'; + $valueBOld = 'another old value'; + $valueANew = 'one new value'; + $valueBNew = 'another new value'; + + $this->writeCache->setMulti( [ $keyA => $valueANew, $keyB => $valueBNew ] ); + $this->readCache->setMulti( [ $keyA => $valueAOld, $keyB => $valueBOld ] ); + + $this->assertEquals( + [ $keyA => $valueAOld, $keyB => $valueBOld ], + $this->cache->getMulti( [ $keyA, $keyB ] ), + 'Async replication' + ); + } + + /** + * @covers ReplicatedBagOStuff::get + * @covers ReplicatedBagOStuff::set + */ + public function testGetSetRaw() { + $key = 'a:key'; $value = 'a value'; $this->cache->set( $key, $value ); @@ -32,31 +94,4 @@ class ReplicatedBagOStuffTest extends \MediaWikiUnitTestCase { // Don't write to replica. Replication is deferred to backend. $this->assertFalse( $this->readCache->get( $key ) ); } - - /** - * @covers ReplicatedBagOStuff::get - */ - public function testGet() { - $key = 'a key'; - - $write = 'one value'; - $this->writeCache->set( $key, $write ); - $read = 'another value'; - $this->readCache->set( $key, $read ); - - // Read from replica. - $this->assertEquals( $read, $this->cache->get( $key ) ); - } - - /** - * @covers ReplicatedBagOStuff::get - */ - public function testGetAbsent() { - $key = 'a key'; - $value = 'a value'; - $this->writeCache->set( $key, $value ); - - // Don't read from master. No failover if value is absent. - $this->assertFalse( $this->cache->get( $key ) ); - } }