From a0cce5e4b63d9607f4561298d346695bd4d1c31c Mon Sep 17 00:00:00 2001 From: Timo Tijhof Date: Mon, 16 Nov 2015 21:41:10 +0000 Subject: [PATCH] objectcache: Implement check keys per cache key in WANObjectCache::getMulti() To allow batch queries for multiple keys that themselves have different check keys. Previously check keys always applied to all keys being retrieved. Change-Id: I9e5ba198d79020ce05a802a510762e29fcfb2f1b --- includes/libs/objectcache/WANObjectCache.php | 74 ++++++++++++----- .../libs/objectcache/WANObjectCacheTest.php | 79 +++++++++++++++++++ 2 files changed, 133 insertions(+), 20 deletions(-) diff --git a/includes/libs/objectcache/WANObjectCache.php b/includes/libs/objectcache/WANObjectCache.php index 7e3fa4fd47..128999a517 100644 --- a/includes/libs/objectcache/WANObjectCache.php +++ b/includes/libs/objectcache/WANObjectCache.php @@ -217,7 +217,8 @@ class WANObjectCache implements IExpiringStore, LoggerAwareInterface { * * @param array $keys List of cache keys * @param array $curTTLs Map of (key => approximate TTL left) for existing keys [returned] - * @param array $checkKeys List of "check" keys to apply to all of $keys + * @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. * @return array Map of (key => value) for keys that exist */ final public function getMulti( @@ -228,26 +229,32 @@ class WANObjectCache implements IExpiringStore, LoggerAwareInterface { $vPrefixLen = strlen( self::VALUE_KEY_PREFIX ); $valueKeys = self::prefixCacheKeys( $keys, self::VALUE_KEY_PREFIX ); - $checkKeys = self::prefixCacheKeys( $checkKeys, self::TIME_KEY_PREFIX ); + + $checksForAll = array(); + $checksByKey = array(); + $checkKeysFlat = array(); + foreach ( $checkKeys as $i => $keys ) { + $prefixed = self::prefixCacheKeys( (array)$keys, self::TIME_KEY_PREFIX ); + $checkKeysFlat = array_merge( $checkKeysFlat, $prefixed ); + // Is this check keys for a specific cache key, or for all keys being fetched? + if ( is_int( $i ) ) { + $checksForAll = array_merge( $checksForAll, $prefixed ); + } else { + $checksByKey[$i] = isset( $checksByKey[$i] ) + ? array_merge( $checksByKey[$i], $prefixed ) + : $prefixed; + } + } // Fetch all of the raw values - $wrappedValues = $this->cache->getMulti( array_merge( $valueKeys, $checkKeys ) ); + $wrappedValues = $this->cache->getMulti( array_merge( $valueKeys, $checkKeysFlat ) ); $now = microtime( true ); - // Get/initialize the timestamp of all the "check" keys - $checkKeyTimes = array(); - foreach ( $checkKeys as $checkKey ) { - $timestamp = isset( $wrappedValues[$checkKey] ) - ? self::parsePurgeValue( $wrappedValues[$checkKey] ) - : false; - if ( !is_float( $timestamp ) ) { - // Key is not set or invalid; regenerate - $this->cache->add( $checkKey, - self::PURGE_VAL_PREFIX . $now, self::CHECK_KEY_TTL ); - $timestamp = $now; - } - - $checkKeyTimes[] = $timestamp; + // Collect timestamps from all "check" keys + $checkKeyTimesForAll = $this->processCheckKeys( $checksForAll, $wrappedValues, $now ); + $checkKeyTimesByKey = array(); + foreach ( $checksByKey as $cacheKey => $checks ) { + $checkKeyTimesByKey[$cacheKey] = $this->processCheckKeys( $checks, $wrappedValues, $now ); } // Get the main cache value for each key and validate them @@ -261,22 +268,49 @@ class WANObjectCache implements IExpiringStore, LoggerAwareInterface { list( $value, $curTTL ) = $this->unwrap( $wrappedValues[$vKey], $now ); if ( $value !== false ) { $result[$key] = $value; + + // Force dependant keys to be invalid for a while after purging + // to reduce race conditions involving stale data getting cached + $checkKeyTimes = $checkKeyTimesForAll; + if ( isset( $checkKeyTimesByKey[$key] ) ) { + $checkKeyTimes = array_merge( $checkKeyTimes, $checkKeyTimesByKey[$key] ); + } foreach ( $checkKeyTimes as $checkKeyTime ) { - // Force dependant keys to be invalid for a while after purging - // to reduce race conditions involving stale data getting cached $safeTimestamp = $checkKeyTime + self::HOLDOFF_TTL; if ( $safeTimestamp >= $wrappedValues[$vKey][self::FLD_TIME] ) { $curTTL = min( $curTTL, $checkKeyTime - $now ); } } } - $curTTLs[$key] = $curTTL; } return $result; } + /** + * @since 1.27 + * @param array $timeKeys List of prefixed time check keys + * @param array $wrappedValues + * @param float $now + * @return array List of timestamps + */ + private function processCheckKeys( array $timeKeys, array $wrappedValues, $now ) { + $times = array(); + foreach ( $timeKeys as $timeKey ) { + $timestamp = isset( $wrappedValues[$timeKey] ) + ? self::parsePurgeValue( $wrappedValues[$timeKey] ) + : false; + if ( !is_float( $timestamp ) ) { + // Key is not set or invalid; regenerate + $this->cache->add( $timeKey, self::PURGE_VAL_PREFIX . $now, self::CHECK_KEY_TTL ); + $timestamp = $now; + } + $times[] = $timestamp; + } + return $times; + } + /** * Set the value of a key in cache * diff --git a/tests/phpunit/includes/libs/objectcache/WANObjectCacheTest.php b/tests/phpunit/includes/libs/objectcache/WANObjectCacheTest.php index d0ae8c990c..b301f284b0 100644 --- a/tests/phpunit/includes/libs/objectcache/WANObjectCacheTest.php +++ b/tests/phpunit/includes/libs/objectcache/WANObjectCacheTest.php @@ -291,6 +291,85 @@ class WANObjectCacheTest extends MediaWikiTestCase { $this->assertLessThan( 0, $curTTLs[$key2], 'Key 2 has negative current TTL' ); } + /** + * @covers WANObjectCache::getMulti() + * @covers WANObjectCache::processCheckKeys() + */ + public function testGetMultiCheckKeys() { + $cache = $this->cache; + + $checkAll = wfRandomString(); + $check1 = wfRandomString(); + $check2 = wfRandomString(); + $check3 = wfRandomString(); + $value1 = wfRandomString(); + $value2 = wfRandomString(); + + // Fake initial check key to be set in the past. Otherwise we'd have to sleep for + // several seconds during the test to assert the behaviour. + foreach ( array( $checkAll, $check1, $check2 ) as $checkKey ) { + $this->internalCache->set( $cache::TIME_KEY_PREFIX . $checkKey, + $cache::PURGE_VAL_PREFIX . microtime( true ) - $cache::HOLDOFF_TTL, $cache::CHECK_KEY_TTL ); + } + + $cache->set( 'key1', $value1, 10 ); + $cache->set( 'key2', $value2, 10 ); + + $curTTLs = array(); + $result = $cache->getMulti( array( 'key1', 'key2', 'key3' ), $curTTLs, array( + 'key1' => $check1, + $checkAll, + 'key2' => $check2, + 'key3' => $check3, + ) ); + $this->assertEquals( + array( 'key1' => $value1, 'key2' => $value2 ), + $result, + 'Initial values' + ); + $this->assertEquals( + array( 'key1' => 0, 'key2' => 0 ), + $curTTLs, + 'Initial ttls' + ); + + $cache->touchCheckKey( $check1 ); + usleep( 100 ); + + $curTTLs = array(); + $result = $cache->getMulti( array( 'key1', 'key2', 'key3' ), $curTTLs, array( + 'key1' => $check1, + $checkAll, + 'key2' => $check2, + 'key3' => $check3, + ) ); + $this->assertEquals( + array( 'key1' => $value1, 'key2' => $value2 ), + $result, + 'key1 expired by check1, but value still provided' + ); + $this->assertLessThan( 0, $curTTLs['key1'], 'key1 TTL expired' ); + $this->assertEquals( 0, $curTTLs['key2'], 'key2 still valid' ); + + $cache->touchCheckKey( $checkAll ); + usleep( 100 ); + + $curTTLs = array(); + $result = $cache->getMulti( array( 'key1', 'key2', 'key3' ), $curTTLs, array( + 'key1' => $check1, + $checkAll, + 'key2' => $check2, + 'key3' => $check3, + ) ); + $this->assertEquals( + array( 'key1' => $value1, 'key2' => $value2 ), + $result, + 'All keys expired by checkAll, but value still provided' + ); + $this->assertLessThan( 0, $curTTLs['key1'], 'key1 expired by checkAll' ); + $this->assertLessThan( 0, $curTTLs['key2'], 'key2 expired by checkAll' ); + } + /** * @covers WANObjectCache::delete() */ -- 2.20.1