objectcache: pass in the $oldValue as-of time in getWithSetCallback()
authorAaron Schulz <aschulz@wikimedia.org>
Thu, 8 Sep 2016 18:34:39 +0000 (11:34 -0700)
committerAaron Schulz <aschulz@wikimedia.org>
Thu, 8 Sep 2016 18:36:25 +0000 (18:36 +0000)
This lets callers use adaptive TTLs on the near-expiration
preemptive refreshes if the new and current values match,
using the as-of time as $mtime.

Change-Id: Ie541c35f890c9f789d1accf9f2a43506daaf31f0

includes/libs/objectcache/WANObjectCache.php
tests/phpunit/includes/libs/objectcache/WANObjectCacheTest.php

index ed3b5cc..9293631 100644 (file)
@@ -643,6 +643,7 @@ class WANObjectCache implements IExpiringStore, LoggerAwareInterface {
         *   - $oldValue : current cache value or false if not present
         *   - &$ttl : a reference to the TTL which can be altered
         *   - &$setOpts : a reference to options for set() which can be altered
+        *   - $oldAsOf : generation UNIX timestamp of $oldValue or null if not present (since 1.28)
         *
         * It is strongly recommended to set the 'lag' and 'since' fields to avoid race conditions
         * that can cause stale values to get stuck at keys. Usually, callbacks ignore the current
@@ -840,7 +841,8 @@ class WANObjectCache implements IExpiringStore, LoggerAwareInterface {
                                $cur = $this->doGetWithSetCallback(
                                        $key,
                                        $ttl,
-                                       function ( $oldValue, &$ttl, &$setOpts ) use ( $callback, $version ) {
+                                       function ( $oldValue, &$ttl, &$setOpts, $oldAsOf )
+                                       use ( $callback, $version ) {
                                                if ( is_array( $oldValue )
                                                        && array_key_exists( self::VFLD_DATA, $oldValue )
                                                ) {
@@ -851,7 +853,7 @@ class WANObjectCache implements IExpiringStore, LoggerAwareInterface {
                                                }
 
                                                return [
-                                                       self::VFLD_DATA => $callback( $oldData, $ttl, $setOpts ),
+                                                       self::VFLD_DATA => $callback( $oldData, $ttl, $setOpts, $oldAsOf ),
                                                        self::VFLD_VERSION => $version
                                                ];
                                        },
@@ -969,13 +971,13 @@ class WANObjectCache implements IExpiringStore, LoggerAwareInterface {
 
                // Generate the new value from the callback...
                $setOpts = [];
-               $value = call_user_func_array( $callback, [ $cValue, &$ttl, &$setOpts ] );
-               $asOf = microtime( true );
+               $value = call_user_func_array( $callback, [ $cValue, &$ttl, &$setOpts, $asOf ] );
                // When delete() is called, writes are write-holed by the tombstone,
                // so use a special INTERIM key to pass the new value around threads.
                if ( ( $isTombstone && $lockTSE > 0 ) && $value !== false && $ttl >= 0 ) {
                        $tempTTL = max( 1, (int)$lockTSE ); // set() expects seconds
-                       $wrapped = $this->wrap( $value, $tempTTL, $asOf );
+                       $newAsOf = microtime( true );
+                       $wrapped = $this->wrap( $value, $tempTTL, $newAsOf );
                        // Avoid using set() to avoid pointless mcrouter broadcasting
                        $this->cache->merge(
                                self::INTERIM_KEY_PREFIX . $key,
index cef31dc..99b959b 100644 (file)
@@ -161,9 +161,15 @@ class WANObjectCacheTest extends MediaWikiTestCase {
                $cKey1 = wfRandomString();
                $cKey2 = wfRandomString();
 
+               $priorValue = null;
+               $priorAsOf = null;
                $wasSet = 0;
-               $func = function( $old, &$ttl ) use ( &$wasSet, $value ) {
+               $func = function( $old, &$ttl, &$opts, $asOf )
+                       use ( &$wasSet, &$priorValue, &$priorAsOf, $value )
+               {
                        ++$wasSet;
+                       $priorValue = $old;
+                       $priorAsOf = $asOf;
                        $ttl = 20; // override with another value
                        return $value;
                };
@@ -172,6 +178,8 @@ class WANObjectCacheTest extends MediaWikiTestCase {
                $v = $cache->getWithSetCallback( $key, 30, $func, [ 'lockTSE' => 5 ] + $extOpts );
                $this->assertEquals( $value, $v, "Value returned" );
                $this->assertEquals( 1, $wasSet, "Value regenerated" );
+               $this->assertFalse( $priorValue, "No prior value" );
+               $this->assertNull( $priorAsOf, "No prior value" );
 
                $curTTL = null;
                $cache->get( $key, $curTTL );
@@ -194,6 +202,8 @@ class WANObjectCacheTest extends MediaWikiTestCase {
                );
                $this->assertEquals( $value, $v, "Value returned" );
                $this->assertEquals( 1, $wasSet, "Value regenerated due to check keys" );
+               $this->assertEquals( $value, $priorValue, "Has prior value" );
+               $this->assertType( 'float', $priorAsOf, "Has prior value" );
                $t1 = $cache->getCheckKeyTime( $cKey1 );
                $this->assertGreaterThanOrEqual( $priorTime, $t1, 'Check keys generated on miss' );
                $t2 = $cache->getCheckKeyTime( $cKey2 );