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

diff --git a/includes/libs/objectcache/WANObjectCache.php b/includes/libs/objectcache/WANObjectCache.php
index b8d90d9..fc3a727 100644 (file)
--- 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 d94c546..a518cee 100644 (file)
--- 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 );
        }
 }