objectcache: fix cache warmup bug in getMultiWithSetCallback()

The warmup cache was not properly prefixed and was also using the entity
IDs instead of the cache keys. Thus, it effectively just wasted a
getMulti() query and resulted in the usual separate GETs anyway.

Added some unit tests for this.

Change-Id: I75b7a31214b515511856f9d95db32e8881d80ccc

diff --git a/includes/libs/objectcache/WANObjectCache.php b/includes/libs/objectcache/WANObjectCache.php
index bc6047f..cb1be95 100644 (file)
--- a/includes/libs/objectcache/WANObjectCache.php
+++ b/includes/libs/objectcache/WANObjectCache.php
@@ -97,6 +97,8 @@ class WANObjectCache implements IExpiringStore, LoggerAwareInterface {
        private $callbackDepth = 0;
        /** @var mixed[] Temporary warm-up cache */
        private $warmupCache = [];
+       /** @var integer Key fetched */
+       private $warmupKeyMisses = 0;
 
        /** Max time expected to pass between delete() and DB commit finishing */
        const MAX_COMMIT_DELAY = 3;
@@ -298,10 +300,13 @@ class WANObjectCache implements IExpiringStore, LoggerAwareInterface {
                if ( $this->warmupCache ) {
                        $wrappedValues = array_intersect_key( $this->warmupCache, array_flip( $keysGet ) );
                        $keysGet = array_diff( $keysGet, array_keys( $wrappedValues ) ); // keys left to fetch
+                       $this->warmupKeyMisses += count( $keysGet );
                } else {
                        $wrappedValues = [];
                }
-               $wrappedValues += $this->cache->getMulti( $keysGet );
+               if ( $keysGet ) {
+                       $wrappedValues += $this->cache->getMulti( $keysGet );
+               }
                // Time used to compare/init "check" keys (derived after getMulti() to be pessimistic)
                $now = microtime( true );
 
@@ -1103,18 +1108,30 @@ class WANObjectCache implements IExpiringStore, LoggerAwareInterface {
        final public function getMultiWithSetCallback(
                ArrayIterator $keyedIds, $ttl, callable $callback, array $opts = []
        ) {
-               $keysWarmUp = iterator_to_array( $keyedIds, true );
                $checkKeys = isset( $opts['checkKeys'] ) ? $opts['checkKeys'] : [];
+
+               $keysWarmUp = [];
+               // Get all the value keys to fetch...
+               foreach ( $keyedIds as $key => $id ) {
+                       $keysWarmUp[] = self::VALUE_KEY_PREFIX . $key;
+               }
+               // Get all the check keys to fetch...
                foreach ( $checkKeys as $i => $checkKeyOrKeys ) {
                        if ( is_int( $i ) ) {
-                               $keysWarmUp[] = $checkKeyOrKeys;
+                               // Single check key that applies to all value keys
+                               $keysWarmUp[] = self::TIME_KEY_PREFIX . $checkKeyOrKeys;
                        } else {
-                               $keysWarmUp = array_merge( $keysWarmUp, $checkKeyOrKeys );
+                               // List of check keys that apply to value key $i
+                               $keysWarmUp = array_merge(
+                                       $keysWarmUp,
+                                       self::prefixCacheKeys( $checkKeyOrKeys, self::TIME_KEY_PREFIX )
+                               );
                        }
                }
 
                $this->warmupCache = $this->cache->getMulti( $keysWarmUp );
                $this->warmupCache += array_fill_keys( $keysWarmUp, false );
+               $this->warmupKeyMisses = 0;
 
                // Wrap $callback to match the getWithSetCallback() format while passing $id to $callback
                $id = null;
@@ -1316,6 +1333,14 @@ class WANObjectCache implements IExpiringStore, LoggerAwareInterface {
                return (int)min( $maxTTL, max( $minTTL, $factor * $age ) );
        }
 
+       /**
+        * @return integer Number of warmup key cache misses last round
+        * @since 1.30
+        */
+       public function getWarmupKeyMisses() {
+               return $this->warmupKeyMisses;
+       }
+
        /**
         * Do the actual async bus purge of a key
         *

diff --git a/tests/phpunit/includes/libs/objectcache/WANObjectCacheTest.php b/tests/phpunit/includes/libs/objectcache/WANObjectCacheTest.php
index 72effd7..dcb0986 100644 (file)
--- a/tests/phpunit/includes/libs/objectcache/WANObjectCacheTest.php
+++ b/tests/phpunit/includes/libs/objectcache/WANObjectCacheTest.php
@@ -310,10 +310,12 @@ class WANObjectCacheTest extends PHPUnit_Framework_TestCase  {
                        $keyedIds, 30, $genFunc, [ 'lowTTL' => 0, 'lockTSE' => 5, ] + $extOpts );
                $this->assertEquals( $value, $v[$keyB], "Value returned" );
                $this->assertEquals( 1, $wasSet, "Value regenerated" );
+               $this->assertEquals( 0, $cache->getWarmupKeyMisses(), "Keys warmed yet in process cache" );
                $v = $cache->getMultiWithSetCallback(
                        $keyedIds, 30, $genFunc, [ 'lowTTL' => 0, 'lockTSE' => 5, ] + $extOpts );
                $this->assertEquals( $value, $v[$keyB], "Value returned" );
                $this->assertEquals( 1, $wasSet, "Value not regenerated" );
+               $this->assertEquals( 0, $cache->getWarmupKeyMisses(), "Keys warmed in process cache" );
 
                $priorTime = microtime( true );
                usleep( 1 );