From: Aaron Schulz <aschulz@wikimedia.org>
Date: Thu, 19 Oct 2017 04:00:29 +0000 (-0700)
Subject: objectcache: Split off some code in WANObjectCache::getWithSetCallback
X-Git-Tag: 1.31.0-rc.0~1708^2
X-Git-Url: https://git.heureux-cyclage.org/?p=lhc%2Fweb%2Fwiklou.git;a=commitdiff_plain;h=0fe50ad75986fe31d6d75bbad3813836ffd7f831

objectcache: Split off some code in WANObjectCache::getWithSetCallback

This makes it a bit easier to follow

Change-Id: I67968814ab046473eb8eca4086a8600c77417b82
---

diff --git a/includes/libs/objectcache/WANObjectCache.php b/includes/libs/objectcache/WANObjectCache.php
index cab5782f37..a0652c6cdc 100644
--- a/includes/libs/objectcache/WANObjectCache.php
+++ b/includes/libs/objectcache/WANObjectCache.php
@@ -987,11 +987,8 @@ class WANObjectCache implements IExpiringStore, LoggerAwareInterface {
 				// Use the INTERIM value for tombstoned keys to reduce regeneration load.
 				// For hot keys, either another thread has the lock or the lock failed;
 				// use the INTERIM value from the last thread that regenerated it.
-				$wrapped = $this->cache->get( self::INTERIM_KEY_PREFIX . $key );
-				list( $value ) = $this->unwrap( $wrapped, microtime( true ) );
-				if ( $value !== false && $this->isValid( $value, $versioned, $asOf, $minTime ) ) {
-					$asOf = $wrapped[self::FLD_TIME];
-
+				$value = $this->getInterimValue( $key, $versioned, $minTime, $asOf );
+				if ( $value !== false ) {
 					return $value;
 				}
 				// Use the busy fallback value if nothing else
@@ -1013,24 +1010,19 @@ class WANObjectCache implements IExpiringStore, LoggerAwareInterface {
 		} finally {
 			--$this->callbackDepth;
 		}
+		$valueIsCacheable = ( $value !== false && $ttl >= 0 );
+
 		// 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 ) {
+		if ( ( $isTombstone && $lockTSE > 0 ) && $valueIsCacheable ) {
 			$tempTTL = max( 1, (int)$lockTSE ); // set() expects seconds
 			$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,
-				function () use ( $wrapped ) {
-					return $wrapped;
-				},
-				$tempTTL,
-				1
-			);
+			$this->setInterimValue( $key, $wrapped, $tempTTL );
 		}
 
-		if ( $value !== false && $ttl >= 0 ) {
+		if ( $valueIsCacheable ) {
 			$setOpts['lockTSE'] = $lockTSE;
 			// Use best known "since" timestamp if not provided
 			$setOpts += [ 'since' => $preCallbackTime ];
@@ -1046,6 +1038,41 @@ class WANObjectCache implements IExpiringStore, LoggerAwareInterface {
 		return $value;
 	}
 
+	/**
+	 * @param string $key
+	 * @param bool $versioned
+	 * @param float $minTime
+	 * @param mixed $asOf
+	 * @return mixed
+	 */
+	protected function getInterimValue( $key, $versioned, $minTime, &$asOf ) {
+		$wrapped = $this->cache->get( self::INTERIM_KEY_PREFIX . $key );
+		list( $value ) = $this->unwrap( $wrapped, microtime( true ) );
+		if ( $value !== false && $this->isValid( $value, $versioned, $asOf, $minTime ) ) {
+			$asOf = $wrapped[self::FLD_TIME];
+
+			return $value;
+		}
+
+		return false;
+	}
+
+	/**
+	 * @param string $key
+	 * @param array $wrapped
+	 * @param int $tempTTL
+	 */
+	protected function setInterimValue( $key, $wrapped, $tempTTL ) {
+		$this->cache->merge(
+			self::INTERIM_KEY_PREFIX . $key,
+			function () use ( $wrapped ) {
+				return $wrapped;
+			},
+			$tempTTL,
+			1
+		);
+	}
+
 	/**
 	 * Method to fetch multiple cache keys at once with regeneration
 	 *
diff --git a/tests/phpunit/includes/libs/objectcache/WANObjectCacheTest.php b/tests/phpunit/includes/libs/objectcache/WANObjectCacheTest.php
index 0b2df619e3..c5a1759ff9 100644
--- a/tests/phpunit/includes/libs/objectcache/WANObjectCacheTest.php
+++ b/tests/phpunit/includes/libs/objectcache/WANObjectCacheTest.php
@@ -13,6 +13,8 @@ use Wikimedia\TestingAccessWrapper;
  * @covers WANObjectCache::getProcessCache
  * @covers WANObjectCache::getNonProcessCachedKeys
  * @covers WANObjectCache::getRawKeysForWarmup
+ * @covers WANObjectCache::getInterimValue
+ * @covers WANObjectCache::setInterimValue
  */
 class WANObjectCacheTest extends PHPUnit_Framework_TestCase {
 	/** @var WANObjectCache */