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

objectcache: Reduce boilerplate and indirection around makeKey()

Browse source 
== Background

Most of this was introduced in commit 5c335f9d77 (I1eb897c2cea3f5b7).
The original motivation was:

* Ensure wrappers like MultiWriteBagOStuff naturally do the right
  thing. In practice, makeKey() results are interchangeable, with
  the most contrained one (Memcached) also generally used as the first
  tier. However, this is not intuitive and may change in the future.
  To make it more intuitive, the default implemention became known
  as "generic", with proxyCall() responsible for decoding these,
  and then re-encoding them with makeKey() from the respective
  underlying BagOStuff. This meant that MultiWriteBag would no longer
  use the result of the Memcached-formatted cache key and pass it
  to SqlBagOStuff.

* Allow extraction of the key group from a given key cache,
  for use in statistics.

Both motivations remains valid and addressed after this refactor.

== Change

* Remove boilerplate and indirection around makeKey from a dozen
  classes. E.g. copy-paste stubs for makeKey, makeKeyInternal, and
  convertGenericKey.

  Instead, let BagOStuff::makeKey and ::makeKeyInternal hold the
  defaults. I believe this makes the logic easier to find, understand,
  and refer to.

  The three non-default implementations (Memcached, WinCache, Sql)
  now naturally reflect what they are in terms of business logic,
  they are a method override.

  Introduce a single boolean requireConvertGenericKey() to let the
  three non-default implementations signal their need to convert
  keys before use.

* Further improve internal consistently of BagOStuff::makeKeyInternal.

  The logic of genericKeyFromComponents() was moved up into
  BagOStuff::makeKeyInternal. As a result of caling this directly
  from BagOStuff::makeKey(), this code now sees $keyspace and $components
  as separate arguments. To keep the behaviour the same, we would
  have to either unshift $keyspace into $components, or duplicate
  the strtr() call to escape it.

  Instead, excempt keyspace from escaping. This matches how the most
  commonly used BagOStuff implementations (MemcachedBag, and SqlBag)
  already worked for 10+ years, thus this does not introduce any new
  responsibility on callers. In particular, keyspace (not key group)
  is set by MediaWiki core in service wiring to the wiki ID, and so
  is not the concern of individual callers anyway.

* Docs: Explain in proxyCall() why this indirection and complexity
  exists. It lets wrapping classes decode and re-encode keys.

* Docs: Explain the cross-wiki and local-wiki semantics of makeKey
  and makeKeyGlobal, and centralise this and other important docs
  about this method in the place with the most eye balls where it is
  most likely seen and discovered, namely BagOStuff::makeKey.
  Remove partial docs from other places in favour of references to this one.

  Previously, there was no particular reason to follow `@see IStoreKeyEncoder`
  much less to know that it holds critical that communicate the
  responsibility to limit the key group to 48 chars.

* Docs: Consistently refer to the first component as the "key group",
  thus unifying what was known as "key class", "collection",
  "key collection name", or "collection name".

  The term "key group" seems to be what is used by developers in
  conversations for this concept, matching WMF on-boarding docs and
  WMF's Grafana dashboard for WANObjectCache.

Change-Id: I6b3167cac824d8bd8773bc66c386f41e4d380021
This commit is contained in:
Timo Tijhof 2023-07-17 23:15:57 +01:00 committed by Amir Sarabadani
parent 4b1fc2a3fc
commit 75aec3686a
17 changed files with 156 additions and 244 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

9
includes/libs/objectcache/APCUBagOStuff.php
View file

@ -132,13 +132,4 @@ class APCUBagOStuff extends MediumSpecificBagOStuff {
return $result;
}
protected function makeKeyInternal( $keyspace, $components ) {
return $this->genericKeyFromComponents( $keyspace, ...$components );
}
protected function convertGenericKey( $key ) {
// short-circuit; already uses "generic" keys
return $key;
}
}

138
includes/libs/objectcache/BagOStuff.php
View file

@ -500,28 +500,51 @@ abstract class BagOStuff implements
}
/**
* Make a cache key for the default keyspace and given components
* Make a cache key from the given components, in the "global" keyspace
*
* @see IStoreKeyEncoder::makeGlobalKey()
* Global keys are shared with and visible to all sites hosted in the same
* infrastructure (e.g. cross-wiki within the same wiki farm). Others sites
* may read the stored value from their requests, and they must be able to
* correctly compute new values from their own request context.
*
* @param string $collection Key collection name component
* @see BagOStuff::makeKeyInternal
* @since 1.27
* @param string $keygroup Key group component, should be under 48 characters.
* @param string|int ...$components Additional, ordered, key components for entity IDs
* @return string Colon-separated, keyspace-prepended, ordered list of encoded components
* @since 1.27
*/
abstract public function makeGlobalKey( $collection, ...$components );
public function makeGlobalKey( $keygroup, ...$components ) {
return $this->makeKeyInternal( self::GLOBAL_KEYSPACE, func_get_args() );
}
/**
* Make a cache key for the global keyspace and given components
* Make a cache key from the given components, in the default keyspace
*
* @see IStoreKeyEncoder::makeKey()
* The default keyspace is unique to a given site. Subsequent web requests
* to the same site (e.g. local wiki, or same domain name) will interact
* with the same keyspace.
*
* @param string $collection Key collection name component
* Requests to other sites hosted on the same infrastructure (e.g. cross-wiki
* or cross-domain), have their own keyspace that naturally avoids conflicts.
*
* As caller you are responsible for:
* - Limit the key group (first component) to 48 characters
*
* Internally, the colon is used as delimiter (":"), and this is
* automatically escaped in supplied components to avoid ambiguity or
* key conflicts. BagOStuff subclasses are responsible for applying any
* additional escaping or limits as-needed before sending commands over
* the network.
*
* @see BagOStuff::makeKeyInternal
* @since 1.27
* @param string $keygroup Key group component, should be under 48 characters.
* @param string|int ...$components Additional, ordered, key components for entity IDs
* @return string Colon-separated, keyspace-prepended, ordered list of encoded components
* @since 1.27
*/
abstract public function makeKey( $collection, ...$components );
public function makeKey( $keygroup, ...$components ) {
return $this->makeKeyInternal( $this->keyspace, func_get_args() );
}
/**
* Check whether a cache key is in the global keyspace
@ -617,55 +640,86 @@ abstract class BagOStuff implements
}
/**
* At a minimum, there must be a keyspace and collection name component
* Make a cache key for the given keyspace and components
*
* @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
* Subclasses may override this method in order to apply different escaping,
* or to deal with size constraints (such as MemcachedBagOStuff). For example
* by converting long components into hashes.
*
* If you overrride this method, you MUST override ::requireConvertGenericKey()
* to return true. This ensures that wrapping classes (e.g. MultiWriteBagOStuff)
* know to re-encode keys before calling read/write methods. See also ::proxyCall().
*
* @see BagOStuff::proxyCall
* @since 1.27
* @param string $keyspace
* @param string[]|int[] $components Key group and other components
* @return string
*/
final protected function genericKeyFromComponents( ...$components ) {
if ( count( $components ) < 2 ) {
throw new InvalidArgumentException( "Missing keyspace or collection name" );
protected function makeKeyInternal( $keyspace, $components ) {
if ( count( $components ) < 1 ) {
throw new InvalidArgumentException( "Missing key group" );
}
$key = '';
$key = $keyspace;
foreach ( $components as $i => $component ) {
if ( $i > 0 ) {
$key .= ':';
}
// Escape delimiter (":") and escape ("%") characters
$key .= strtr( $component, [ '%' => '%25', ':' => '%3A' ] );
$key .= ':' . strtr( $component, [ '%' => '%25', ':' => '%3A' ] );
}
return $key;
}
/**
* Extract the components from a "generic" reversible cache key
* Whether ::proxyCall() must re-encode cache keys before calling read/write methods.
*
* @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
* @stable to override
* @see BagOStuff::makeKeyInternal
* @see BagOStuff::proxyCall
* @since 1.41
* @return bool
*/
final protected function componentsFromGenericKey( $key ) {
// Note that the order of each corresponding search/replace pair matters
return str_replace( [ '%3A', '%25' ], [ ':', '%' ], explode( ':', $key ) );
protected function requireConvertGenericKey(): bool {
return false;
}
/**
* Convert a "generic" reversible cache key into one for this cache
* Convert a key from BagOStuff::makeKeyInternal into one for the current subclass
*
* @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
* @see BagOStuff::proxyCall
* @param string $key Result from BagOStuff::makeKeyInternal
* @return string Result from current subclass override of BagOStuff::makeKeyInternal
*/
abstract protected function convertGenericKey( $key );
private function convertGenericKey( $key ) {
if ( !$this->requireConvertGenericKey() ) {
// If subclass doesn't overwrite makeKeyInternal, no re-encoding is needed.
return $key;
}
// Extract the components from a "generic" key formatted by BagOStuff::makeKeyInternal()
// Note that the order of each corresponding search/replace pair matters!
$components = str_replace( [ '%3A', '%25' ], [ ':', '%' ], explode( ':', $key ) );
if ( count( $components ) < 2 ) {
// Legacy key, not even formatted by makeKey()/makeGlobalKey(). Keep as-is.
return $key;
}
$keyspace = array_shift( $components );
return $this->makeKeyInternal( $keyspace, $components );
}
/**
* Call a method on behalf of wrapper BagOStuff instance that uses "generic" keys
* Call a method on behalf of wrapper BagOStuff instance
*
* The "wrapper" BagOStuff subclass that calls proxyCall() MUST NOT override
* the default makeKeyInternal() implementation, because proxyCall() needs
* to turn the "generic" key back into an array, and re-format it according
* to the backend-specific BagOStuff::makeKey implementation.
*
* For example, when using MultiWriteBagOStuff with Memcached as a backend,
* writes will go via MemcachedBagOStuff::proxyCall(), which then reformats
* the "generic" result of BagOStuff::makeKey (called as MultiWriteBagOStuff::makeKey)
* using MemcachedBagOStuff::makeKeyInternal.
*
* @param string $method Name of a non-final public method that reads/changes keys
* @param int $arg0Sig BagOStuff::ARG0_* constant describing argument 0
@ -725,7 +779,7 @@ abstract class BagOStuff implements
}
/**
* @param string $key Key generated by an IStoreKeyEncoder instance
* @param string $key Key generated by BagOStuff::makeKeyInternal
* @return string A stats prefix to describe this class of key (e.g. "objectcache.file")
*/
protected function determineKeyPrefixForStats( $key ) {
@ -733,11 +787,11 @@ abstract class BagOStuff implements
// and thus has the format of "<scope>:<collection>[:<constant or variable>]..."
$components = explode( ':', $key, 3 );
// Handle legacy callers that fail to use the key building methods
$collection = $components[1] ?? 'UNKNOWN';
$keygroup = $components[1] ?? 'UNKNOWN';
$statsGroup = 'objectcache';
// Replace dots because they are special in StatsD (T232907)
return $statsGroup . '.' . strtr( $collection, '.', '_' );
return $statsGroup . '.' . strtr( $keygroup, '.', '_' );
}
/**

17
includes/libs/objectcache/CachedBagOStuff.php
View file

@ -203,23 +203,6 @@ class CachedBagOStuff extends BagOStuff {
);
}
protected function makeKeyInternal( $keyspace, $components ) {
return $this->genericKeyFromComponents( $keyspace, ...$components );
}
public function makeKey( $collection, ...$components ) {
return $this->genericKeyFromComponents( $this->keyspace, $collection, ...$components );
}
public function makeGlobalKey( $collection, ...$components ) {
return $this->genericKeyFromComponents( self::GLOBAL_KEYSPACE, $collection, ...$components );
}
protected function convertGenericKey( $key ) {
// short-circuit; already uses "generic" keys
return $key;
}
public function setMulti( array $valueByKey, $exptime = 0, $flags = 0 ) {
$this->procCache->setMulti( $valueByKey, $exptime, $flags );

9
includes/libs/objectcache/EmptyBagOStuff.php
View file

@ -60,13 +60,4 @@ class EmptyBagOStuff extends MediumSpecificBagOStuff {
// faster
return true;
}
protected function makeKeyInternal( $keyspace, $components ) {
return $this->genericKeyFromComponents( $keyspace, ...$components );
}
protected function convertGenericKey( $key ) {
// short-circuit; already uses "generic" keys
return $key;
}
}

9
includes/libs/objectcache/HashBagOStuff.php
View file

@ -165,13 +165,4 @@ class HashBagOStuff extends MediumSpecificBagOStuff {
public function hasKey( $key ) {
return isset( $this->bag[$key] );
}
protected function makeKeyInternal( $keyspace, $components ) {
return $this->genericKeyFromComponents( $keyspace, ...$components );
}
protected function convertGenericKey( $key ) {
// short-circuit; already uses "generic" keys
return $key;
}
}

38
includes/libs/objectcache/IStoreKeyEncoder.php
View file

@ -1,41 +1,25 @@
<?php
/**
* Generic interface for object stores with key encoding methods.
* Key-encoding methods for object caching (BagOStuff and WANObjectCache)
*
* @ingroup Cache
* @since 1.34
*/
interface IStoreKeyEncoder {
/**
* Make a cache key using the "global" keyspace for the given components
*
* Callers should:
* - Limit the collection name (first component) to 48 characters
* - Use hashes for any components based on user-supplied input
*
* Encoding is limited to the escaping of delimiter (":") and escape ("%") characters.
* Any backend-specific encoding should be delegated to methods that use the network.
*
* @param string $collection Key collection name component
* @param string|int ...$components Additional, ordered, key components for entity IDs
* @return string Colon-separated, keyspace-prepended, ordered list of encoded components
* @see BagOStuff::makeGlobalKey
* @param string $keygroup
* @param string|int ...$components
* @return string
*/
public function makeGlobalKey( $collection, ...$components );
public function makeGlobalKey( $keygroup, ...$components );
/**
* Make a cache key using the default keyspace for the given components
*
* Callers should:
* - Limit the collection name (first component) to 48 characters
* - Use hashes for any components based on user-supplied input
*
* Encoding is limited to the escaping of delimiter (":") and escape ("%") characters.
* Any backend-specific encoding should be delegated to methods that use the network.
*
* @param string $collection Key collection name component
* @param string|int ...$components Additional, ordered, key components for entity IDs
* @return string Colon-separated, keyspace-prepended, ordered list of encoded components
* @see BagOStuff::makeKey
* @param string $keygroup
* @param string|int ...$components
* @return string
*/
public function makeKey( $collection, ...$components );
public function makeKey( $keygroup, ...$components );
}

33
includes/libs/objectcache/MediumSpecificBagOStuff.php
View file

@ -988,39 +988,6 @@ abstract class MediumSpecificBagOStuff extends BagOStuff {
return ( $value === (string)$integer );
}
public function makeGlobalKey( $collection, ...$components ) {
return $this->makeKeyInternal( self::GLOBAL_KEYSPACE, func_get_args() );
}
public function makeKey( $collection, ...$components ) {
return $this->makeKeyInternal( $this->keyspace, func_get_args() );
}
/**
* 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.
*
* @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
*/
abstract protected function makeKeyInternal( $keyspace, $components );
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;
}

9
includes/libs/objectcache/MemcachedBagOStuff.php
View file

@ -50,11 +50,12 @@ abstract class MemcachedBagOStuff extends MediumSpecificBagOStuff {
}
/**
* Construct a cache key.
* Format a cache key.
*
* @since 1.27
* @see BagOStuff::makeKeyInternal
* @param string $keyspace
* @param array $components
* @param string[]|int[] $components
* @return string
*/
protected function makeKeyInternal( $keyspace, $components ) {
@ -91,6 +92,10 @@ abstract class MemcachedBagOStuff extends MediumSpecificBagOStuff {
return $keyspace . ':' . implode( ':', $components );
}
protected function requireConvertGenericKey(): bool {
return true;
}
/**
* Ensure that a key is safe to use (contains no control characters and no
* characters above the ASCII range.)

17
includes/libs/objectcache/MultiWriteBagOStuff.php
View file

@ -306,23 +306,6 @@ class MultiWriteBagOStuff extends BagOStuff {
);
}
protected function makeKeyInternal( $keyspace, $components ) {
return $this->genericKeyFromComponents( $keyspace, ...$components );
}
public function makeKey( $collection, ...$components ) {
return $this->genericKeyFromComponents( $this->keyspace, $collection, ...$components );
}
public function makeGlobalKey( $collection, ...$components ) {
return $this->genericKeyFromComponents( self::GLOBAL_KEYSPACE, $collection, ...$components );
}
protected function convertGenericKey( $key ) {
// short-circuit; already uses "generic" keys
return $key;
}
public function setMockTime( &$time ) {
parent::setMockTime( $time );
foreach ( $this->caches as $cache ) {

9
includes/libs/objectcache/RESTBagOStuff.php
View file

@ -267,15 +267,6 @@ class RESTBagOStuff extends MediumSpecificBagOStuff {
return $newValue;
}
protected function makeKeyInternal( $keyspace, $components ) {
return $this->genericKeyFromComponents( $keyspace, ...$components );
}
protected function convertGenericKey( $key ) {
// short-circuit; already uses "generic" keys
return $key;
}
/**
* Processes the response body.
*

9
includes/libs/objectcache/RedisBagOStuff.php
View file

@ -556,13 +556,4 @@ LUA;
public function logRequest( $op, $keys, $server, $e = null ) {
$this->debug( "$op($keys) on $server: " . ( $e ? "failure" : "success" ) );
}
protected function makeKeyInternal( $keyspace, $components ) {
return $this->genericKeyFromComponents( $keyspace, ...$components );
}
protected function convertGenericKey( $key ) {
// short-circuit; already uses "generic" keys
return $key;
}
}

17
includes/libs/objectcache/ReplicatedBagOStuff.php
View file

@ -259,23 +259,6 @@ class ReplicatedBagOStuff extends BagOStuff {
);
}
protected function makeKeyInternal( $keyspace, $components ) {
return $this->genericKeyFromComponents( $keyspace, ...$components );
}
public function makeKey( $collection, ...$components ) {
return $this->genericKeyFromComponents( $this->keyspace, $collection, ...$components );
}
public function makeGlobalKey( $collection, ...$components ) {
return $this->genericKeyFromComponents( self::GLOBAL_KEYSPACE, $collection, ...$components );
}
protected function convertGenericKey( $key ) {
// short-circuit; already uses "generic" keys
return $key;
}
public function setMockTime( &$time ) {
parent::setMockTime( $time );
$this->writeStore->setMockTime( $time );

4
includes/libs/objectcache/WinCacheBagOStuff.php
View file

@ -142,6 +142,10 @@ class WinCacheBagOStuff extends MediumSpecificBagOStuff {
return $keyspace . ':' . implode( ':', $components );
}
protected function requireConvertGenericKey(): bool {
return true;
}
public function doIncrWithInit( $key, $exptime, $step, $init, $flags ) {
// optimize with FIFO lock
if ( !wincache_lock( $key ) ) {

63
includes/libs/objectcache/wancache/WANObjectCache.php
View file

@ -760,7 +760,7 @@ class WANObjectCache implements
* @return bool Success
*/
final public function set( $key, $value, $ttl = self::TTL_INDEFINITE, array $opts = [] ) {
$kClass = $this->determineKeyClassForStats( $key );
$keygroup = $this->determineKeyGroupForStats( $key );
$ok = $this->setMainValue(
$key,
@ -777,7 +777,7 @@ class WANObjectCache implements
$opts['creating'] ?? false
);
$this->stats->increment( "wanobjectcache.$kClass.set." . ( $ok ? 'ok' : 'error' ) );
$this->stats->increment( "wanobjectcache.$keygroup.set." . ( $ok ? 'ok' : 'error' ) );
return $ok;
}
@ -1029,8 +1029,8 @@ class WANObjectCache implements
$ok = $this->relayVolatilePurge( $valueSisterKey, $purge, $ttl );
}
$kClass = $this->determineKeyClassForStats( $key );
$this->stats->increment( "wanobjectcache.$kClass.delete." . ( $ok ? 'ok' : 'error' ) );
$keygroup = $this->determineKeyGroupForStats( $key );
$this->stats->increment( "wanobjectcache.$keygroup.delete." . ( $ok ? 'ok' : 'error' ) );
return $ok;
}
@ -1188,8 +1188,8 @@ class WANObjectCache implements
$purge = $this->makeCheckPurgeValue( $now, $holdoff );
$ok = $this->relayVolatilePurge( $checkSisterKey, $purge, self::CHECK_KEY_TTL );
$kClass = $this->determineKeyClassForStats( $key );
$this->stats->increment( "wanobjectcache.$kClass.ck_touch." . ( $ok ? 'ok' : 'error' ) );
$keygroup = $this->determineKeyGroupForStats( $key );
$this->stats->increment( "wanobjectcache.$keygroup.ck_touch." . ( $ok ? 'ok' : 'error' ) );
return $ok;
}
@ -1225,8 +1225,8 @@ class WANObjectCache implements
$checkSisterKey = $this->makeSisterKey( $key, self::TYPE_TIMESTAMP );
$ok = $this->relayNonVolatilePurge( $checkSisterKey );
$kClass = $this->determineKeyClassForStats( $key );
$this->stats->increment( "wanobjectcache.$kClass.ck_reset." . ( $ok ? 'ok' : 'error' ) );
$keygroup = $this->determineKeyGroupForStats( $key );
$this->stats->increment( "wanobjectcache.$keygroup.ck_reset." . ( $ok ? 'ok' : 'error' ) );
return $ok;
}
@ -1601,7 +1601,7 @@ class WANObjectCache implements
$touchedCb = $opts['touchedCallback'] ?? null;
$startTime = $this->getCurrentTime();
$kClass = $this->determineKeyClassForStats( $key );
$keygroup = $this->determineKeyGroupForStats( $key );
// Get the current key value and its metadata
$curState = $this->fetchKeys( [ $key ], $checkKeys, $touchedCb )[$key];
@ -1610,7 +1610,7 @@ class WANObjectCache implements
if ( $this->isAcceptablyFreshValue( $curState, $graceTTL, $minAsOf ) ) {
if ( !$this->isLotteryRefreshDue( $curState, $lowTTL, $ageNew, $hotTTR, $startTime ) ) {
$this->stats->timing(
"wanobjectcache.$kClass.hit.good",
"wanobjectcache.$keygroup.hit.good",
1e3 * ( $this->getCurrentTime() - $startTime )
);
@ -1618,7 +1618,7 @@ class WANObjectCache implements
} elseif ( $this->scheduleAsyncRefresh( $key, $ttl, $callback, $opts, $cbParams ) ) {
$this->logger->debug( "fetchOrRegenerate($key): hit with async refresh" );
$this->stats->timing(
"wanobjectcache.$kClass.hit.refresh",
"wanobjectcache.$keygroup.hit.refresh",
1e3 * ( $this->getCurrentTime() - $startTime )
);
@ -1653,7 +1653,7 @@ class WANObjectCache implements
if ( $this->isExtremelyNewValue( $volState, $safeMinAsOf, $startTime ) ) {
$this->logger->debug( "fetchOrRegenerate($key): volatile hit" );
$this->stats->timing(
"wanobjectcache.$kClass.hit.volatile",
"wanobjectcache.$keygroup.hit.volatile",
1e3 * ( $this->getCurrentTime() - $startTime )
);
@ -1696,7 +1696,7 @@ class WANObjectCache implements
if ( $this->isValid( $volValue, $volState[self::RES_AS_OF], $minAsOf ) ) {
$this->logger->debug( "fetchOrRegenerate($key): returning stale value" );
$this->stats->timing(
"wanobjectcache.$kClass.hit.stale",
"wanobjectcache.$keygroup.hit.stale",
1e3 * ( $this->getCurrentTime() - $startTime )
);
@ -1705,7 +1705,7 @@ class WANObjectCache implements
$miss = is_infinite( $minAsOf ) ? 'renew' : 'miss';
$this->logger->debug( "fetchOrRegenerate($key): busy $miss" );
$this->stats->timing(
"wanobjectcache.$kClass.$miss.busy",
"wanobjectcache.$keygroup.$miss.busy",
1e3 * ( $this->getCurrentTime() - $startTime )
);
$placeholderValue = $this->resolveBusyValue( $busyValue );
@ -1738,7 +1738,7 @@ class WANObjectCache implements
// How long it took to generate the value
$walltime = max( $postCallbackTime - $preCallbackTime, 0.0 );
$this->stats->timing( "wanobjectcache.$kClass.regen_walltime", 1e3 * $walltime );
$this->stats->timing( "wanobjectcache.$keygroup.regen_walltime", 1e3 * $walltime );
// Attempt to save the newly generated value if applicable
if (
@ -1747,7 +1747,7 @@ class WANObjectCache implements
// Current thread was not raced out of a regeneration lock or key is tombstoned
( !$useRegenerationLock || $hasLock || $isKeyTombstoned )
) {
$this->stats->timing( "wanobjectcache.$kClass.regen_set_delay", 1e3 * $elapsed );
$this->stats->timing( "wanobjectcache.$keygroup.regen_set_delay", 1e3 * $elapsed );
// If the key is write-holed then use the (volatile) interim key as an alternative
if ( $isKeyTombstoned ) {
$this->setInterimValue(
@ -1784,7 +1784,7 @@ class WANObjectCache implements
$miss = is_infinite( $minAsOf ) ? 'renew' : 'miss';
$this->logger->debug( "fetchOrRegenerate($key): $miss, new value computed" );
$this->stats->timing(
"wanobjectcache.$kClass.$miss.compute",
"wanobjectcache.$keygroup.$miss.compute",
1e3 * ( $this->getCurrentTime() - $startTime )
);
@ -2211,31 +2211,25 @@ class WANObjectCache implements
}
/**
* Make a cache key for the global keyspace and given components
*
* @see IStoreKeyEncoder::makeGlobalKey()
*
* @param string $collection Key collection name component
* @see BagOStuff::makeGlobalKey()
* @since 1.27
* @param string $keygroup Key group component, should be under 48 characters.
* @param string|int ...$components Additional, ordered, key components for entity IDs
* @return string Colon-separated, keyspace-prepended, ordered list of encoded components
* @since 1.27
*/
public function makeGlobalKey( $collection, ...$components ) {
public function makeGlobalKey( $keygroup, ...$components ) {
// @phan-suppress-next-line PhanParamTooFewUnpack Should infer non-emptiness
return $this->cache->makeGlobalKey( ...func_get_args() );
}
/**
* Make a cache key using the "global" keyspace for the given components
*
* @see IStoreKeyEncoder::makeKey()
*
* @param string $collection Key collection name component
* @see BagOStuff::makeKey()
* @since 1.27
* @param string $keygroup Key group component, should be under 48 characters.
* @param string|int ...$components Additional, ordered, key components for entity IDs
* @return string Colon-separated, keyspace-prepended, ordered list of encoded components
* @since 1.27
*/
public function makeKey( $collection, ...$components ) {
public function makeKey( $keygroup, ...$components ) {
// @phan-suppress-next-line PhanParamTooFewUnpack Should infer non-emptiness
return $this->cache->makeKey( ...func_get_args() );
}
@ -2879,10 +2873,11 @@ class WANObjectCache implements
}
/**
* @param string $key String of the format <scope>:<collection>[:<constant or variable>]...
* @return string A collection name to describe this class of key
* @param string $key Cache key in the format `<keyspace>:<keygroup>[:<other components>]...`
* as formatted by WANObjectCache::makeKey() or ::makeKeyGlobal.
* @return string The key group of this cache key
*/
private function determineKeyClassForStats( $key ) {
private function determineKeyGroupForStats( $key ) {
$parts = explode( ':', $key, 3 );
// Fallback in case the key was not made by makeKey.
// Replace dots because they are special in StatsD (T232907)

4
includes/objectcache/SqlBagOStuff.php
View file

@ -1593,6 +1593,10 @@ class SqlBagOStuff extends MediumSpecificBagOStuff {
return $keyspace . ':' . implode( ':', $components );
}
protected function requireConvertGenericKey(): bool {
return true;
}
protected function serialize( $value ) {
if ( is_int( $value ) ) {
return $value;

13
tests/phpunit/includes/libs/objectcache/MultiWriteBagOStuffTest.php
View file

@ -147,10 +147,8 @@ class MultiWriteBagOStuffTest extends MediaWikiIntegrationTestCase {
return $keyspace . ':short-one-way';
}
protected function convertGenericKey( $key ) {
$components = $this->componentsFromGenericKey( $key );
$keyspace = array_shift( $components );
return $this->makeKeyInternal( $keyspace, $components );
protected function requireConvertGenericKey(): bool {
return true;
}
};
$cache2 = new class extends HashBagOStuff {
@ -158,12 +156,9 @@ class MultiWriteBagOStuffTest extends MediaWikiIntegrationTestCase {
return $keyspace . ':short-another-way';
}
protected function convertGenericKey( $key ) {
$components = $this->componentsFromGenericKey( $key );
$keyspace = array_shift( $components );
return $this->makeKeyInternal( $keyspace, $components );
protected function requireConvertGenericKey(): bool {
return true;
}
};
$cache = new MultiWriteBagOStuff( [

2
tests/phpunit/unit/includes/libs/objectcache/WANObjectCacheTest.php
View file

@ -2194,7 +2194,7 @@ class WANObjectCacheTest extends MediaWikiUnitTestCase {
'cache' => new HashBagOStuff
] ) );
$this->assertSame( $class, $wanCache->determineKeyClassForStats( $key ) );
$this->assertSame( $class, $wanCache->determineKeyGroupForStats( $key ) );
}
/**