From 4432e898be078eea73fbf072f5a383ec1ded0d67 Mon Sep 17 00:00:00 2001 From: Aaron Schulz Date: Wed, 18 Oct 2017 19:50:17 -0700 Subject: [PATCH] Add statsd metric support to WANObjectCache Bug: T178531 Change-Id: I3037281d09cd5195347789f544deae89711f128b --- includes/DefaultSettings.php | 4 ++- includes/libs/objectcache/WANObjectCache.php | 33 +++++++++++++++++-- includes/objectcache/ObjectCache.php | 5 ++- .../libs/objectcache/WANObjectCacheTest.php | 23 +++++++++++++ 4 files changed, 60 insertions(+), 5 deletions(-) diff --git a/includes/DefaultSettings.php b/includes/DefaultSettings.php index 040f1ce7bf..7127803b59 100644 --- a/includes/DefaultSettings.php +++ b/includes/DefaultSettings.php @@ -6351,7 +6351,9 @@ $wgStatsdMetricPrefix = 'MediaWiki'; * Rates are sampling probabilities (e.g. 0.1 means 1 in 10 events are sampled). * @since 1.28 */ -$wgStatsdSamplingRates = []; +$wgStatsdSamplingRates = [ + 'wanobjectcache:*' => 0.001 +]; /** * InfoAction retrieves a list of transclusion links (both to and from). diff --git a/includes/libs/objectcache/WANObjectCache.php b/includes/libs/objectcache/WANObjectCache.php index 0531d7f709..79f2e44199 100644 --- a/includes/libs/objectcache/WANObjectCache.php +++ b/includes/libs/objectcache/WANObjectCache.php @@ -19,6 +19,7 @@ * @ingroup Cache */ +use Liuggio\StatsdClient\Factory\StatsdDataFactoryInterface; use Psr\Log\LoggerAwareInterface; use Psr\Log\LoggerInterface; use Psr\Log\NullLogger; @@ -88,6 +89,8 @@ class WANObjectCache implements IExpiringStore, LoggerAwareInterface { protected $purgeRelayer; /** @var LoggerInterface */ protected $logger; + /** @var StatsdDataFactoryInterface */ + protected $stats; /** @var int ERR_* constant for the "last error" registry */ protected $lastRelayError = self::ERR_NONE; @@ -177,6 +180,7 @@ class WANObjectCache implements IExpiringStore, LoggerAwareInterface { * - channels : Map of (action => channel string). Actions include "purge". * - relayers : Map of (action => EventRelayer object). Actions include "purge". * - logger : LoggerInterface object + * - stats : LoggerInterface object */ public function __construct( array $params ) { $this->cache = $params['cache']; @@ -187,6 +191,7 @@ class WANObjectCache implements IExpiringStore, LoggerAwareInterface { ? $params['relayers']['purge'] : new EventRelayerNull( [] ); $this->setLogger( isset( $params['logger'] ) ? $params['logger'] : new NullLogger() ); + $this->stats = isset( $params['stats'] ) ? $params['stats'] : new NullStatsdDataFactory(); } public function setLogger( LoggerInterface $logger ) { @@ -239,7 +244,7 @@ class WANObjectCache implements IExpiringStore, LoggerAwareInterface { * Consider using getWithSetCallback() instead of get() and set() cycles. * That method has cache slam avoiding features for hot/expensive keys. * - * @param string $key Cache key + * @param string $key Cache key made from makeKey() or makeGlobalKey() * @param mixed &$curTTL Approximate TTL left on the key if present/tombstoned [returned] * @param array $checkKeys List of "check" keys * @param float &$asOf UNIX timestamp of cached value; null on failure [returned] @@ -260,7 +265,7 @@ class WANObjectCache implements IExpiringStore, LoggerAwareInterface { * * @see WANObjectCache::get() * - * @param array $keys List of cache keys + * @param array $keys List of cache keys made from makeKey() or makeGlobalKey() * @param array &$curTTLs Map of (key => approximate TTL left) for existing keys [returned] * @param array $checkKeys List of check keys to apply to all $keys. May also apply "check" * keys to specific cache keys only by using cache keys as keys in the $checkKeys array. @@ -799,7 +804,7 @@ class WANObjectCache implements IExpiringStore, LoggerAwareInterface { * @see WANObjectCache::get() * @see WANObjectCache::set() * - * @param string $key Cache key + * @param string $key Cache key made from makeKey() or makeGlobalKey() * @param int $ttl Seconds to live for key updates. Special values are: * - WANObjectCache::TTL_INDEFINITE: Cache forever * - WANObjectCache::TTL_UNCACHEABLE: Do not cache at all @@ -951,6 +956,9 @@ class WANObjectCache implements IExpiringStore, LoggerAwareInterface { $minTime = isset( $opts['minAsOf'] ) ? $opts['minAsOf'] : self::MIN_TIMESTAMP_NONE; $versioned = isset( $opts['version'] ); + // Get a collection name to describe this class of key + $kClass = $this->determineKeyClass( $key ); + // Get the current key value $curTTL = null; $cValue = $this->get( $key, $curTTL, $checkKeys, $asOf ); // current value @@ -964,6 +972,8 @@ class WANObjectCache implements IExpiringStore, LoggerAwareInterface { && !$this->worthRefreshExpiring( $curTTL, $lowTTL ) && !$this->worthRefreshPopular( $asOf, $ageNew, $popWindow, $preCallbackTime ) ) { + $this->stats->increment( "wanobjectcache.$kClass.hit.good" ); + return $value; } @@ -990,6 +1000,7 @@ class WANObjectCache implements IExpiringStore, LoggerAwareInterface { // Lock acquired; this thread should update the key $lockAcquired = true; } elseif ( $value !== false && $this->isValid( $value, $versioned, $asOf, $minTime ) ) { + $this->stats->increment( "wanobjectcache.$kClass.hit.stale" ); // If it cannot be acquired; then the stale value can be used return $value; } else { @@ -998,10 +1009,14 @@ class WANObjectCache implements IExpiringStore, LoggerAwareInterface { // use the INTERIM value from the last thread that regenerated it. $value = $this->getInterimValue( $key, $versioned, $minTime, $asOf ); if ( $value !== false ) { + $this->stats->increment( "wanobjectcache.$kClass.hit.volatile" ); + return $value; } // Use the busy fallback value if nothing else if ( $busyValue !== null ) { + $this->stats->increment( "wanobjectcache.$kClass.miss.busy" ); + return is_callable( $busyValue ) ? $busyValue() : $busyValue; } } @@ -1044,6 +1059,8 @@ class WANObjectCache implements IExpiringStore, LoggerAwareInterface { $this->cache->changeTTL( self::MUTEX_KEY_PREFIX . $key, (int)$preCallbackTime - 60 ); } + $this->stats->increment( "wanobjectcache.$kClass.miss.compute" ); + return $value; } @@ -1693,6 +1710,16 @@ class WANObjectCache implements IExpiringStore, LoggerAwareInterface { return $res; } + /** + * @param string $key String of the format :[:]... + * @return string + */ + protected function determineKeyClass( $key ) { + $parts = explode( ':', $key ); + + return isset( $parts[1] ) ? $parts[1] : $parts[0]; // sanity + } + /** * @param string $value Wrapped value like "PURGED::" * @return array|bool Array containing a UNIX timestamp (float) and holdoff period (integer), diff --git a/includes/objectcache/ObjectCache.php b/includes/objectcache/ObjectCache.php index 3370e5b9d1..6d6d3459d9 100644 --- a/includes/objectcache/ObjectCache.php +++ b/includes/objectcache/ObjectCache.php @@ -332,12 +332,15 @@ class ObjectCache { * @throws UnexpectedValueException */ public static function newWANCacheFromParams( array $params ) { - $erGroup = MediaWikiServices::getInstance()->getEventRelayerGroup(); + $services = MediaWikiServices::getInstance(); + + $erGroup = $services->getEventRelayerGroup(); foreach ( $params['channels'] as $action => $channel ) { $params['relayers'][$action] = $erGroup->getRelayer( $channel ); $params['channels'][$action] = $channel; } $params['cache'] = self::newFromParams( $params['store'] ); + $params['stats'] = $services->getStatsdDataFactory(); if ( isset( $params['loggroup'] ) ) { $params['logger'] = LoggerFactory::getInstance( $params['loggroup'] ); } else { diff --git a/tests/phpunit/includes/libs/objectcache/WANObjectCacheTest.php b/tests/phpunit/includes/libs/objectcache/WANObjectCacheTest.php index e23f3183df..934d49a8b2 100644 --- a/tests/phpunit/includes/libs/objectcache/WANObjectCacheTest.php +++ b/tests/phpunit/includes/libs/objectcache/WANObjectCacheTest.php @@ -1322,4 +1322,27 @@ class WANObjectCacheTest extends PHPUnit_Framework_TestCase { $this->assertSame( 'special', $wanCache->makeGlobalKey( 'a', 'b' ) ); } + + public static function statsKeyProvider() { + return [ + [ 'domain:page:5', 'page' ], + [ 'domain:main-key', 'main-key' ], + [ 'domain:page:history', 'page' ], + [ 'missingdomainkey', 'missingdomainkey' ] + ]; + } + + /** + * @dataProvider statsKeyProvider + * @covers WANObjectCache::determineKeyClass + */ + public function testStatsKeyClass( $key, $class ) { + $wanCache = TestingAccessWrapper::newFromObject( new WANObjectCache( [ + 'cache' => new HashBagOStuff, + 'pool' => 'testcache-hash', + 'relayer' => new EventRelayerNull( [] ) + ] ) ); + + $this->assertEquals( $class, $wanCache->determineKeyClass( $key ) ); + } } -- 2.20.1