From 300c65537711b10d4765bbe20f7add01307cd188 Mon Sep 17 00:00:00 2001 From: Aaron Schulz Date: Mon, 27 Nov 2017 10:51:32 -0800 Subject: [PATCH] Use time forcing methods to avoid WANObjectCacheTest flakeiness Use of microtime() is now just for baselines, and it is no longer assumed to be increasing with each call. Such an assumption is particuliarly bad on Windows. I've done 100X run rounds with now failures on Windows. Change-Id: Ica2a47982495bc95b10ca507414972744ea9507e --- includes/libs/objectcache/WANObjectCache.php | 3 + .../libs/objectcache/WANObjectCacheTest.php | 164 +++++++++++------- 2 files changed, 102 insertions(+), 65 deletions(-) diff --git a/includes/libs/objectcache/WANObjectCache.php b/includes/libs/objectcache/WANObjectCache.php index b8d90d99fd..fc3a727d31 100644 --- a/includes/libs/objectcache/WANObjectCache.php +++ b/includes/libs/objectcache/WANObjectCache.php @@ -1662,6 +1662,9 @@ class WANObjectCache implements IExpiringStore, LoggerAwareInterface { $ageStale = abs( $curTTL ); // seconds of staleness $curGTTL = ( $graceTTL - $ageStale ); // current grace-time-to-live + if ( $curGTTL <= 0 ) { + return false; // already out of grace period + } // Chance of using a stale value is the complement of the chance of refreshing it return !$this->worthRefreshExpiring( $curGTTL, $graceTTL ); diff --git a/tests/phpunit/includes/libs/objectcache/WANObjectCacheTest.php b/tests/phpunit/includes/libs/objectcache/WANObjectCacheTest.php index d94c546160..a518ceea9b 100644 --- a/tests/phpunit/includes/libs/objectcache/WANObjectCacheTest.php +++ b/tests/phpunit/includes/libs/objectcache/WANObjectCacheTest.php @@ -17,7 +17,7 @@ use Wikimedia\TestingAccessWrapper; * @covers WANObjectCache::setInterimValue */ class WANObjectCacheTest extends PHPUnit_Framework_TestCase { - /** @var WANObjectCache */ + /** @var TimeAdjustableWANObjectCache */ private $cache; /** @var BagOStuff */ private $internalCache; @@ -25,8 +25,8 @@ class WANObjectCacheTest extends PHPUnit_Framework_TestCase { protected function setUp() { parent::setUp(); - $this->cache = new WANObjectCache( [ - 'cache' => new HashBagOStuff(), + $this->cache = new TimeAdjustableWANObjectCache( [ + 'cache' => new TimeAdjustableHashBagOStuff(), 'pool' => 'testcache-hash', 'relayer' => new EventRelayerNull( [] ) ] ); @@ -215,15 +215,16 @@ class WANObjectCacheTest extends PHPUnit_Framework_TestCase { $this->assertGreaterThanOrEqual( 19, $curTTL, 'Current TTL between 19-20 (overriden)' ); $wasSet = 0; - $v = $cache->getWithSetCallback( $key, 30, $func, [ - 'lowTTL' => 0, - 'lockTSE' => 5, - ] + $extOpts ); + $v = $cache->getWithSetCallback( + $key, 30, $func, [ 'lowTTL' => 0, 'lockTSE' => 5 ] + $extOpts ); $this->assertEquals( $value, $v, "Value returned" ); $this->assertEquals( 0, $wasSet, "Value not regenerated" ); - $priorTime = microtime( true ); - usleep( 1 ); + $priorTime = microtime( true ); // reference time + $cache->setTime( $priorTime ); + + $cache->addTime( 1 ); + $wasSet = 0; $v = $cache->getWithSetCallback( $key, 30, $func, [ 'checkKeys' => [ $cKey1, $cKey2 ] ] + $extOpts @@ -237,7 +238,8 @@ class WANObjectCacheTest extends PHPUnit_Framework_TestCase { $t2 = $cache->getCheckKeyTime( $cKey2 ); $this->assertGreaterThanOrEqual( $priorTime, $t2, 'Check keys generated on miss' ); - $priorTime = microtime( true ); + $priorTime = $cache->addTime( 0.01 ); + $wasSet = 0; $v = $cache->getWithSetCallback( $key, 30, $func, [ 'checkKeys' => [ $cKey1, $cKey2 ] ] + $extOpts @@ -267,11 +269,6 @@ class WANObjectCacheTest extends PHPUnit_Framework_TestCase { $this->assertEquals( $value, $v, "Value still returned after deleted" ); $this->assertEquals( 1, $wasSet, "Value process cached while deleted" ); - $timeyCache = new TimeAdjustableWANObjectCache( [ - 'cache' => new TimeAdjustableHashBagOStuff(), - 'pool' => 'empty' - ] ); - $oldValReceived = -1; $oldAsOfReceived = -1; $checkFunc = function ( $oldVal, &$ttl, array $setOpts, $oldAsOf ) @@ -287,23 +284,22 @@ class WANObjectCacheTest extends PHPUnit_Framework_TestCase { $wasSet = 0; $key = wfRandomString(); - $timeyCache->setTime( $now ); - $v = $timeyCache->getWithSetCallback( + $v = $cache->getWithSetCallback( $key, 30, $checkFunc, [ 'staleTTL' => 50 ] + $extOpts ); $this->assertEquals( 'xxx1', $v, "Value returned" ); $this->assertEquals( false, $oldValReceived, "Callback got no stale value" ); $this->assertEquals( null, $oldAsOfReceived, "Callback got no stale value" ); - $timeyCache->setTime( $now + 40 ); - $v = $timeyCache->getWithSetCallback( + $cache->addTime( 40 ); + $v = $cache->getWithSetCallback( $key, 30, $checkFunc, [ 'staleTTL' => 50 ] + $extOpts ); $this->assertEquals( 'xxx2', $v, "Value still returned after expired" ); $this->assertEquals( 2, $wasSet, "Value recalculated while expired" ); $this->assertEquals( 'xxx1', $oldValReceived, "Callback got stale value" ); $this->assertNotEquals( null, $oldAsOfReceived, "Callback got stale value" ); - $timeyCache->setTime( $now + 300 ); - $v = $timeyCache->getWithSetCallback( + $cache->addTime( 260 ); + $v = $cache->getWithSetCallback( $key, 30, $checkFunc, [ 'staleTTL' => 50 ] + $extOpts ); $this->assertEquals( 'xxx3', $v, "Value still returned after expired" ); $this->assertEquals( 3, $wasSet, "Value recalculated while expired" ); @@ -312,39 +308,51 @@ class WANObjectCacheTest extends PHPUnit_Framework_TestCase { $wasSet = 0; $key = wfRandomString(); - $checkKey = $timeyCache->makeKey( 'template', 'X' ); - $timeyCache->setTime( $now - $timeyCache::HOLDOFF_TTL - 1 ); - $timeyCache->touchCheckKey( $checkKey ); // init check key - $timeyCache->setTime( $now ); - $v = $timeyCache->getWithSetCallback( + $checkKey = $cache->makeKey( 'template', 'X' ); + $cache->setTime( $now - $cache::HOLDOFF_TTL - 1 ); + $cache->touchCheckKey( $checkKey ); // init check key + $cache->setTime( $now ); + $v = $cache->getWithSetCallback( $key, - $timeyCache::TTL_WEEK, + $cache::TTL_INDEFINITE, $checkFunc, - [ 'graceTTL' => $timeyCache::TTL_DAY, 'checkKeys' => [ $checkKey ] ] + $extOpts + [ 'graceTTL' => $cache::TTL_WEEK, 'checkKeys' => [ $checkKey ] ] + $extOpts ); $this->assertEquals( 'xxx1', $v, "Value returned" ); $this->assertEquals( 1, $wasSet, "Value computed" ); $this->assertEquals( false, $oldValReceived, "Callback got no stale value" ); $this->assertEquals( null, $oldAsOfReceived, "Callback got no stale value" ); - $timeyCache->setTime( $now + 300 ); // some time passes - $timeyCache->touchCheckKey( $checkKey ); // make key stale - $timeyCache->setTime( $now + 3600 ); // ~23 hours left of grace - $v = $timeyCache->getWithSetCallback( + $cache->addTime( $cache::TTL_HOUR ); // some time passes + $v = $cache->getWithSetCallback( + $key, + $cache::TTL_INDEFINITE, + $checkFunc, + [ 'graceTTL' => $cache::TTL_WEEK, 'checkKeys' => [ $checkKey ] ] + $extOpts + ); + $this->assertEquals( 'xxx1', $v, "Cached value returned" ); + $this->assertEquals( 1, $wasSet, "Cached value returned" ); + + $cache->touchCheckKey( $checkKey ); // make key stale + $cache->addTime( 0.01 ); // ~1 week left of grace (barely stale to avoid refreshes) + + $v = $cache->getWithSetCallback( $key, - $timeyCache::TTL_WEEK, + $cache::TTL_INDEFINITE, $checkFunc, - [ 'graceTTL' => $timeyCache::TTL_DAY, 'checkKeys' => [ $checkKey ] ] + $extOpts + [ 'graceTTL' => $cache::TTL_WEEK, 'checkKeys' => [ $checkKey ] ] + $extOpts ); $this->assertEquals( 'xxx1', $v, "Value still returned after expired (in grace)" ); $this->assertEquals( 1, $wasSet, "Value still returned after expired (in grace)" ); - $timeyCache->setTime( $now + $timeyCache::TTL_DAY ); - $v = $timeyCache->getWithSetCallback( + // Change of refresh increase to unity as staleness approaches graceTTL + + $cache->addTime( $cache::TTL_WEEK ); // 8 days of being stale + $v = $cache->getWithSetCallback( $key, - $timeyCache::TTL_WEEK, + $cache::TTL_INDEFINITE, $checkFunc, - [ 'graceTTL' => $timeyCache::TTL_DAY, 'checkKeys' => [ $checkKey ] ] + $extOpts + [ 'graceTTL' => $cache::TTL_WEEK, 'checkKeys' => [ $checkKey ] ] + $extOpts ); $this->assertEquals( 'xxx2', $v, "Value was recomputed (past grace)" ); $this->assertEquals( 2, $wasSet, "Value was recomputed (past grace)" ); @@ -487,8 +495,11 @@ class WANObjectCacheTest extends PHPUnit_Framework_TestCase { $this->assertEquals( 1, $wasSet, "Value not regenerated" ); $this->assertEquals( 0, $cache->getWarmupKeyMisses(), "Keys warmed in process cache" ); - $priorTime = microtime( true ); - usleep( 1 ); + $priorTime = microtime( true ); // reference time + $cache->setTime( $priorTime ); + + $cache->addTime( 1 ); + $wasSet = 0; $keyedIds = new ArrayIterator( [ $keyB => 'efef' ] ); $v = $cache->getMultiWithSetCallback( @@ -503,7 +514,7 @@ class WANObjectCacheTest extends PHPUnit_Framework_TestCase { $t2 = $cache->getCheckKeyTime( $cKey2 ); $this->assertGreaterThanOrEqual( $priorTime, $t2, 'Check keys generated on miss' ); - $priorTime = microtime( true ); + $priorTime = $cache->addTime( 0.01 ); $value = "@43636$"; $wasSet = 0; $keyedIds = new ArrayIterator( [ $keyC => 43636 ] ); @@ -653,8 +664,11 @@ class WANObjectCacheTest extends PHPUnit_Framework_TestCase { $this->assertEquals( 1, $wasSet, "Value not regenerated" ); $this->assertEquals( 0, $cache->getWarmupKeyMisses(), "Keys warmed in process cache" ); - $priorTime = microtime( true ); - usleep( 1 ); + $priorTime = microtime( true ); // reference time + $cache->setTime( $priorTime ); + + $cache->addTime( 1 ); + $wasSet = 0; $keyedIds = new ArrayIterator( [ $keyB => 'efef' ] ); $v = $cache->getMultiWithUnionSetCallback( @@ -667,7 +681,7 @@ class WANObjectCacheTest extends PHPUnit_Framework_TestCase { $t2 = $cache->getCheckKeyTime( $cKey2 ); $this->assertGreaterThanOrEqual( $priorTime, $t2, 'Check keys generated on miss' ); - $priorTime = microtime( true ); + $priorTime = $cache->addTime( 0.01 ); $value = "@43636$"; $wasSet = 0; $keyedIds = new ArrayIterator( [ $keyC => 43636 ] ); @@ -904,8 +918,11 @@ class WANObjectCacheTest extends PHPUnit_Framework_TestCase { $cKey1 = wfRandomString(); $cKey2 = wfRandomString(); - $priorTime = microtime( true ); - usleep( 1 ); + $priorTime = microtime( true ); // reference time + $cache->setTime( $priorTime ); + + $cache->addTime( 1 ); + $curTTLs = []; $this->assertEquals( [ $key1 => $value1, $key2 => $value2 ], @@ -920,7 +937,8 @@ class WANObjectCacheTest extends PHPUnit_Framework_TestCase { $this->assertLessThanOrEqual( 0, $curTTLs[$key1], 'Key 1 has current TTL <= 0' ); $this->assertLessThanOrEqual( 0, $curTTLs[$key2], 'Key 2 has current TTL <= 0' ); - usleep( 1 ); + $cache->addTime( 1 ); + $curTTLs = []; $this->assertEquals( [ $key1 => $value1, $key2 => $value2 ], @@ -946,12 +964,15 @@ class WANObjectCacheTest extends PHPUnit_Framework_TestCase { $value1 = wfRandomString(); $value2 = wfRandomString(); + $cache->setTime( microtime( true ) ); + // 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 ( [ $checkAll, $check1, $check2 ] as $checkKey ) { $cache->touchCheckKey( $checkKey, WANObjectCache::HOLDOFF_NONE ); } - usleep( 100 ); + + $cache->addTime( 0.100 ); $cache->set( 'key1', $value1, 10 ); $cache->set( 'key2', $value2, 10 ); @@ -973,6 +994,7 @@ class WANObjectCacheTest extends PHPUnit_Framework_TestCase { $this->assertGreaterThanOrEqual( 9.5, $curTTLs['key2'], 'Initial ttls' ); $this->assertLessThanOrEqual( 10.5, $curTTLs['key2'], 'Initial ttls' ); + $cache->addTime( 0.100 ); $cache->touchCheckKey( $check1 ); $curTTLs = []; @@ -1157,36 +1179,39 @@ class WANObjectCacheTest extends PHPUnit_Framework_TestCase { * @covers WANObjectCache::parsePurgeValue */ public function testTouchKeys() { + $cache = $this->cache; $key = wfRandomString(); - $priorTime = microtime( true ); - usleep( 100 ); - $t0 = $this->cache->getCheckKeyTime( $key ); + $priorTime = microtime( true ); // reference time + $cache->setTime( $priorTime ); + + $newTime = $cache->addTime( 0.100 ); + $t0 = $cache->getCheckKeyTime( $key ); $this->assertGreaterThanOrEqual( $priorTime, $t0, 'Check key auto-created' ); - $priorTime = microtime( true ); - usleep( 100 ); - $this->cache->touchCheckKey( $key ); - $t1 = $this->cache->getCheckKeyTime( $key ); + $priorTime = $newTime; + $newTime = $cache->addTime( 0.100 ); + $cache->touchCheckKey( $key ); + $t1 = $cache->getCheckKeyTime( $key ); $this->assertGreaterThanOrEqual( $priorTime, $t1, 'Check key created' ); - $t2 = $this->cache->getCheckKeyTime( $key ); + $t2 = $cache->getCheckKeyTime( $key ); $this->assertEquals( $t1, $t2, 'Check key time did not change' ); - usleep( 100 ); - $this->cache->touchCheckKey( $key ); - $t3 = $this->cache->getCheckKeyTime( $key ); + $cache->addTime( 0.100 ); + $cache->touchCheckKey( $key ); + $t3 = $cache->getCheckKeyTime( $key ); $this->assertGreaterThan( $t2, $t3, 'Check key time increased' ); - $t4 = $this->cache->getCheckKeyTime( $key ); + $t4 = $cache->getCheckKeyTime( $key ); $this->assertEquals( $t3, $t4, 'Check key time did not change' ); - usleep( 100 ); - $this->cache->resetCheckKey( $key ); - $t5 = $this->cache->getCheckKeyTime( $key ); + $cache->addTime( 0.100 ); + $cache->resetCheckKey( $key ); + $t5 = $cache->getCheckKeyTime( $key ); $this->assertGreaterThan( $t4, $t5, 'Check key time increased' ); - $t6 = $this->cache->getCheckKeyTime( $key ); + $t6 = $cache->getCheckKeyTime( $key ); $this->assertEquals( $t5, $t6, 'Check key time did not change' ); } @@ -1513,6 +1538,15 @@ class TimeAdjustableWANObjectCache extends WANObjectCache { } } + public function addTime( $time ) { + $this->timeOverride += $time; + if ( $this->cache instanceof TimeAdjustableHashBagOStuff ) { + $this->cache->setTime( $this->timeOverride ); + } + + return $this->timeOverride; + } + protected function getCurrentTime() { return $this->timeOverride ?: parent::getCurrentTime(); } @@ -1522,7 +1556,7 @@ class NearExpiringWANObjectCache extends TimeAdjustableWANObjectCache { const CLOCK_SKEW = 1; protected function worthRefreshExpiring( $curTTL, $lowTTL ) { - return ( ( $curTTL + self::CLOCK_SKEW ) < $lowTTL ); + return ( $curTTL > 0 && ( $curTTL + self::CLOCK_SKEW ) < $lowTTL ); } } -- 2.20.1