From: Aaron Schulz <aschulz@wikimedia.org>
Date: Thu, 17 Sep 2015 18:00:23 +0000 (-0700)
Subject: Small cleanups to WANObjectCache
X-Git-Tag: 1.31.0-rc.0~9981^2
X-Git-Url: https://git.cyclocoop.org/%7B%24www_url%7Dadmin/compta/comptes/ajouter.php?a=commitdiff_plain;h=73b27a3d32e6694df4e95536214ed40aab93c2a8;p=lhc%2Fweb%2Fwiklou.git

Small cleanups to WANObjectCache

* Added a few comments
* Renamed $locked => $lockAcquired for clarity

Change-Id: I45710974971731205d072a1f4b0f9cb37e2cb2a2
---

diff --git a/includes/libs/objectcache/WANObjectCache.php b/includes/libs/objectcache/WANObjectCache.php
index 56c544fd79..130caeba29 100644
--- a/includes/libs/objectcache/WANObjectCache.php
+++ b/includes/libs/objectcache/WANObjectCache.php
@@ -260,9 +260,10 @@ class WANObjectCache {
 	/**
 	 * Purge a key from all clusters
 	 *
-	 * This deletes the key and instantiates a hold-off period where the key
-	 * cannot be written to for the next few seconds (HOLDOFF_TTL). This is to
-	 * avoid the following race condition:
+	 * This should only be called when the underlying data (being cached)
+	 * changes in a significant way. This deletes the key and starts a hold-off
+	 * period where the key cannot be written to for a few seconds (HOLDOFF_TTL).
+	 * This is done to avoid the following race condition:
 	 *   a) Some DB data changes and delete() is called on a corresponding key
 	 *   b) A request refills the key with a stale value from a lagged DB
 	 *   c) The stale value is stuck there until the key is expired/evicted
@@ -274,9 +275,10 @@ class WANObjectCache {
 	 *   a) Replication lag is bounded to being less than HOLDOFF_TTL; or
 	 *   b) If lag is higher, the DB will have gone into read-only mode already
 	 *
-	 * This should only be called when the underlying data (being cached)
-	 * changes in a significant way. If called twice on the same key, then
-	 * the last TTL takes precedence.
+	 * If called twice on the same key, then the last hold-off TTL takes
+	 * precedence. For idempotence, the $ttl should not vary for different
+	 * delete() calls on the same key. Also note that lowering $ttl reduces
+	 * the effective range of the 'lockTSE' parameter to getWithSetCallback().
 	 *
 	 * @param string $key Cache key
 	 * @param integer $ttl How long to block writes to the key [seconds]
@@ -469,6 +471,7 @@ class WANObjectCache {
 	 *               Other threads will try to use stale values if possible.
 	 *               If, on miss, the time since expiration is low, the assumption
 	 *               is that the key is hot and that a stampede is worth avoiding.
+	 *               Setting this above WANObjectCache::HOLDOFF_TTL makes no difference.
 	 *   - tempTTL : TTL of the temp key used to cache values while a key is tombstoned.
 	 *               This avoids excessive regeneration of hot keys on delete() but may
 	 *               result in stale values.
@@ -491,23 +494,24 @@ class WANObjectCache {
 			return $value;
 		}
 
+		// A deleted key with a negative TTL left must be tombstoned
 		$isTombstone = ( $curTTL !== null && $value === false );
 		// Assume a key is hot if requested soon after invalidation
 		$isHot = ( $curTTL !== null && $curTTL <= 0 && abs( $curTTL ) <= $lockTSE );
 
-		$locked = false;
+		$lockAcquired = false;
 		if ( $isHot ) {
 			// Acquire a cluster-local non-blocking lock
 			if ( $this->cache->lock( $key, 0, self::LOCK_TTL ) ) {
 				// Lock acquired; this thread should update the key
-				$locked = true;
+				$lockAcquired = true;
 			} elseif ( $value !== false ) {
 				// If it cannot be acquired; then the stale value can be used
 				return $value;
 			}
 		}
 
-		if ( !$locked && ( $isTombstone || $isHot ) ) {
+		if ( !$lockAcquired && ( $isTombstone || $isHot ) ) {
 			// Use the stash value for tombstoned keys to reduce regeneration load.
 			// For hot keys, either another thread has the lock or the lock failed;
 			// use the stash value from the last thread that regenerated it.
@@ -529,7 +533,7 @@ class WANObjectCache {
 			$this->cache->set( self::STASH_KEY_PREFIX . $key, $value, $tempTTL );
 		}
 
-		if ( $locked ) {
+		if ( $lockAcquired ) {
 			$this->cache->unlock( $key );
 		}