jenkins-bot has submitted this change and it was merged. (
https://gerrit.wikimedia.org/r/393630 )
Change subject: Use time forcing methods to avoid WANObjectCacheTest flakeiness
......................................................................
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
---
M includes/libs/objectcache/WANObjectCache.php
M tests/phpunit/includes/libs/objectcache/WANObjectCacheTest.php
2 files changed, 102 insertions(+), 65 deletions(-)
Approvals:
Krinkle: Looks good to me, approved
jenkins-bot: Verified
diff --git a/includes/libs/objectcache/WANObjectCache.php
b/includes/libs/objectcache/WANObjectCache.php
index b8d90d9..fc3a727 100644
--- a/includes/libs/objectcache/WANObjectCache.php
+++ b/includes/libs/objectcache/WANObjectCache.php
@@ -1662,6 +1662,9 @@
$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
--- a/tests/phpunit/includes/libs/objectcache/WANObjectCacheTest.php
+++ b/tests/phpunit/includes/libs/objectcache/WANObjectCacheTest.php
@@ -17,7 +17,7 @@
* @covers WANObjectCache::setInterimValue
*/
class WANObjectCacheTest extends PHPUnit_Framework_TestCase {
- /** @var WANObjectCache */
+ /** @var TimeAdjustableWANObjectCache */
private $cache;
/** @var BagOStuff */
private $internalCache;
@@ -25,8 +25,8 @@
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 @@
$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 @@
$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 @@
$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 @@
$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 @@
$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,
- $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, "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,
+ $cache::TTL_INDEFINITE,
+ $checkFunc,
+ [ '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 @@
$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 @@
$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 @@
$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 @@
$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 @@
$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 @@
$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 @@
$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 @@
$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 @@
* @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 @@
}
}
+ 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 @@
const CLOCK_SKEW = 1;
protected function worthRefreshExpiring( $curTTL, $lowTTL ) {
- return ( ( $curTTL + self::CLOCK_SKEW ) < $lowTTL );
+ return ( $curTTL > 0 && ( $curTTL + self::CLOCK_SKEW ) <
$lowTTL );
}
}
--
To view, visit https://gerrit.wikimedia.org/r/393630
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Ica2a47982495bc95b10ca507414972744ea9507e
Gerrit-PatchSet: 5
Gerrit-Project: mediawiki/core
Gerrit-Branch: master
Gerrit-Owner: Aaron Schulz <[email protected]>
Gerrit-Reviewer: Krinkle <[email protected]>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
