From 1772c4fe347cc7fc133e5fbb7f01d1e2ddfc812f Mon Sep 17 00:00:00 2001 From: Aaron Schulz Date: Sat, 18 Nov 2017 13:49:32 -0800 Subject: [PATCH] objectcache: add some WAN cache preemptive refresh tests Added some extra sanity checks to WANObjectCache Change-Id: Iac511b0cc1fc8d57ac98e9d7f2cacbcddc1c6db9 --- includes/libs/objectcache/WANObjectCache.php | 32 ++++-- .../libs/objectcache/WANObjectCacheTest.php | 101 ++++++++++++++++++ 2 files changed, 124 insertions(+), 9 deletions(-) diff --git a/includes/libs/objectcache/WANObjectCache.php b/includes/libs/objectcache/WANObjectCache.php index a3c9d71a81..e63b32ed0f 100644 --- a/includes/libs/objectcache/WANObjectCache.php +++ b/includes/libs/objectcache/WANObjectCache.php @@ -207,7 +207,7 @@ class WANObjectCache implements IExpiringStore, LoggerAwareInterface { * @return WANObjectCache */ public static function newEmpty() { - return new self( [ + return new static( [ 'cache' => new EmptyBagOStuff(), 'pool' => 'empty' ] ); @@ -314,7 +314,7 @@ class WANObjectCache implements IExpiringStore, LoggerAwareInterface { $wrappedValues += $this->cache->getMulti( $keysGet ); } // Time used to compare/init "check" keys (derived after getMulti() to be pessimistic) - $now = microtime( true ); + $now = $this->getCurrentTime(); // Collect timestamps from all "check" keys $purgeValuesForAll = $this->processCheckKeys( $checkKeysForAll, $wrappedValues, $now ); @@ -443,7 +443,7 @@ class WANObjectCache implements IExpiringStore, LoggerAwareInterface { * @return bool Success */ final public function set( $key, $value, $ttl = 0, array $opts = [] ) { - $now = microtime( true ); + $now = $this->getCurrentTime(); $lockTSE = isset( $opts['lockTSE'] ) ? $opts['lockTSE'] : self::TSE_NONE; $staleTTL = isset( $opts['staleTTL'] ) ? $opts['staleTTL'] : self::STALE_TTL_NONE; $age = isset( $opts['since'] ) ? max( 0, $now - $opts['since'] ) : 0; @@ -594,7 +594,7 @@ class WANObjectCache implements IExpiringStore, LoggerAwareInterface { $time = $purge[self::FLD_TIME]; } else { // Casting assures identical floats for the next getCheckKeyTime() calls - $now = (string)microtime( true ); + $now = (string)$this->getCurrentTime(); $this->cache->add( $key, $this->makePurgeValue( $now, self::HOLDOFF_TTL ), self::CHECK_KEY_TTL @@ -972,7 +972,7 @@ class WANObjectCache implements IExpiringStore, LoggerAwareInterface { $cValue = $this->get( $key, $curTTL, $checkKeys, $asOf ); // current value $value = $cValue; // return value - $preCallbackTime = microtime( true ); + $preCallbackTime = $this->getCurrentTime(); // Determine if a cached value regeneration is needed or desired if ( $value !== false && $curTTL > 0 @@ -1048,7 +1048,7 @@ class WANObjectCache implements IExpiringStore, LoggerAwareInterface { // so use a special INTERIM key to pass the new value around threads. if ( ( $isTombstone && $lockTSE > 0 ) && $valueIsCacheable ) { $tempTTL = max( 1, (int)$lockTSE ); // set() expects seconds - $newAsOf = microtime( true ); + $newAsOf = $this->getCurrentTime(); $wrapped = $this->wrap( $value, $tempTTL, $newAsOf ); // Avoid using set() to avoid pointless mcrouter broadcasting $this->setInterimValue( $key, $wrapped, $tempTTL ); @@ -1081,7 +1081,7 @@ class WANObjectCache implements IExpiringStore, LoggerAwareInterface { */ protected function getInterimValue( $key, $versioned, $minTime, &$asOf ) { $wrapped = $this->cache->get( self::INTERIM_KEY_PREFIX . $key ); - list( $value ) = $this->unwrap( $wrapped, microtime( true ) ); + list( $value ) = $this->unwrap( $wrapped, $this->getCurrentTime() ); if ( $value !== false && $this->isValid( $value, $versioned, $asOf, $minTime ) ) { $asOf = $wrapped[self::FLD_TIME]; @@ -1539,7 +1539,7 @@ class WANObjectCache implements IExpiringStore, LoggerAwareInterface { if ( $this->purgeRelayer instanceof EventRelayerNull ) { // This handles the mcrouter and the single-DC case $ok = $this->cache->set( $key, - $this->makePurgeValue( microtime( true ), self::HOLDOFF_NONE ), + $this->makePurgeValue( $this->getCurrentTime(), self::HOLDOFF_NONE ), $ttl ); } else { @@ -1598,7 +1598,9 @@ class WANObjectCache implements IExpiringStore, LoggerAwareInterface { * @return bool */ protected function worthRefreshExpiring( $curTTL, $lowTTL ) { - if ( $curTTL >= $lowTTL ) { + if ( $lowTTL <= 0 ) { + return false; + } elseif ( $curTTL >= $lowTTL ) { return false; } elseif ( $curTTL <= 0 ) { return true; @@ -1625,6 +1627,10 @@ class WANObjectCache implements IExpiringStore, LoggerAwareInterface { * @return bool */ protected function worthRefreshPopular( $asOf, $ageNew, $timeTillRefresh, $now ) { + if ( $ageNew < 0 || $timeTillRefresh <= 0 ) { + return false; + } + $age = $now - $asOf; $timeOld = $age - $ageNew; if ( $timeOld <= 0 ) { @@ -1746,6 +1752,14 @@ class WANObjectCache implements IExpiringStore, LoggerAwareInterface { return isset( $parts[1] ) ? $parts[1] : $parts[0]; // sanity } + /** + * @return float UNIX timestamp + * @codeCoverageIgnore + */ + protected function getCurrentTime() { + return microtime( true ); + } + /** * @param string $value Wrapped value like "PURGED::" * @return array|bool Array containing a UNIX timestamp (float) and holdoff period (integer), diff --git a/tests/phpunit/includes/libs/objectcache/WANObjectCacheTest.php b/tests/phpunit/includes/libs/objectcache/WANObjectCacheTest.php index 934d49a8b2..592f72a09e 100644 --- a/tests/phpunit/includes/libs/objectcache/WANObjectCacheTest.php +++ b/tests/phpunit/includes/libs/objectcache/WANObjectCacheTest.php @@ -275,6 +275,66 @@ class WANObjectCacheTest extends PHPUnit_Framework_TestCase { ]; } + public function testPreemtiveRefresh() { + $value = 'KatCafe'; + $wasSet = 0; + $func = function ( $old, &$ttl, &$opts, $asOf ) use ( &$wasSet, $value ) + { + ++$wasSet; + return $value; + }; + + $cache = new NearExpiringWANObjectCache( [ + 'cache' => new HashBagOStuff(), + 'pool' => 'empty' + ] ); + + $wasSet = 0; + $key = wfRandomString(); + $opts = [ 'lowTTL' => 30 ]; + $v = $cache->getWithSetCallback( $key, 20, $func, $opts ); + $this->assertEquals( $value, $v, "Value returned" ); + $this->assertEquals( 1, $wasSet, "Value calculated" ); + $v = $cache->getWithSetCallback( $key, 20, $func, $opts ); + $this->assertEquals( 2, $wasSet, "Value re-calculated" ); + + $wasSet = 0; + $key = wfRandomString(); + $opts = [ 'lowTTL' => 1 ]; + $v = $cache->getWithSetCallback( $key, 30, $func, $opts ); + $this->assertEquals( $value, $v, "Value returned" ); + $this->assertEquals( 1, $wasSet, "Value calculated" ); + $v = $cache->getWithSetCallback( $key, 30, $func, $opts ); + $this->assertEquals( 1, $wasSet, "Value cached" ); + + $cache = new PopularityRefreshingWANObjectCache( [ + 'cache' => new HashBagOStuff(), + 'pool' => 'empty' + ] ); + + $now = microtime( true ); // reference time + $wasSet = 0; + $key = wfRandomString(); + $opts = [ 'hotTTR' => 900 ]; + $v = $cache->getWithSetCallback( $key, 60, $func, $opts ); + $this->assertEquals( $value, $v, "Value returned" ); + $this->assertEquals( 1, $wasSet, "Value calculated" ); + $cache->setTime( $now + 30 ); + $v = $cache->getWithSetCallback( $key, 60, $func, $opts ); + $this->assertEquals( 1, $wasSet, "Value cached" ); + + $wasSet = 0; + $key = wfRandomString(); + $opts = [ 'hotTTR' => 10 ]; + $cache->setTime( $now ); + $v = $cache->getWithSetCallback( $key, 60, $func, $opts ); + $this->assertEquals( $value, $v, "Value returned" ); + $this->assertEquals( 1, $wasSet, "Value calculated" ); + $cache->setTime( $now + 30 ); + $v = $cache->getWithSetCallback( $key, 60, $func, $opts ); + $this->assertEquals( 2, $wasSet, "Value re-calculated" ); + } + /** * @covers WANObjectCache::getWithSetCallback() * @covers WANObjectCache::doGetWithSetCallback() @@ -1346,3 +1406,44 @@ class WANObjectCacheTest extends PHPUnit_Framework_TestCase { $this->assertEquals( $class, $wanCache->determineKeyClass( $key ) ); } } + +class TimeAdjustableHashBagOStuff extends HashBagOStuff { + private $timeOverride = 0; + + public function setTime( $time ) { + $this->timeOverride = $time; + } + + protected function getCurrentTime() { + return $this->timeOverride ?: parent::getCurrentTime(); + } +} + +class TimeAdjustableWANObjectCache extends WANObjectCache { + private $timeOverride = 0; + + public function setTime( $time ) { + $this->timeOverride = $time; + if ( $this->cache instanceof TimeAdjustableHashBagOStuff ) { + $this->cache->setTime( $time ); + } + } + + protected function getCurrentTime() { + return $this->timeOverride ?: parent::getCurrentTime(); + } +} + +class NearExpiringWANObjectCache extends TimeAdjustableWANObjectCache { + const CLOCK_SKEW = 1; + + protected function worthRefreshExpiring( $curTTL, $lowTTL ) { + return ( ( $curTTL + self::CLOCK_SKEW ) < $lowTTL ); + } +} + +class PopularityRefreshingWANObjectCache extends TimeAdjustableWANObjectCache { + protected function worthRefreshPopular( $asOf, $ageNew, $timeTillRefresh, $now ) { + return ( ( $now - $asOf ) > $timeTillRefresh ); + } +} -- 2.20.1