Merge "Updated key WANObjectCache::delete() callers to avoid races"

includes/User.php

@@ -2290,10 +2290,14 @@ class User implements IDBAccessObject {
 	 */
 	public function clearSharedCache() {
 		$id = $this->getId();
-		if ( $id ) {
-			$key = wfMemcKey( 'user', 'id', $id );
-			ObjectCache::getMainWANInstance()->delete( $key );
+		if ( !$id ) {
+			return;
 		}
+
+		$key = wfMemcKey( 'user', 'id', $id );
+		wfGetDB( DB_MASTER )->onTransactionPreCommitOrIdle( function() use ( $key ) {
+			ObjectCache::getMainWANInstance()->delete( $key );
+		} );
 	}
 
 	/**

includes/UserRightsProxy.php

@@ -278,8 +278,9 @@ class UserRightsProxy {
 			array( 'user_id' => $this->id ),
 			__METHOD__ );
 
-		$cache = ObjectCache::getMainWANInstance();
 		$key = wfForeignMemcKey( $this->database, false, 'user', 'id', $this->id );
-		$cache->delete( $key );
+		$this->db->onTransactionPreCommitOrIdle( function() use ( $key ) {
+			ObjectCache::getMainWANInstance()->delete( $key );
+		} );
 	}
 }

includes/filerepo/LocalRepo.php

@@ -521,15 +521,11 @@ class LocalRepo extends FileRepo {
 	 * @return void
 	 */
 	function invalidateImageRedirect( Title $title ) {
-		$cache = ObjectCache::getMainWANInstance();
-
-		$memcKey = $this->getSharedCacheKey( 'image_redirect', md5( $title->getDBkey() ) );
-		if ( $memcKey ) {
-			// Set a temporary value for the cache key, to ensure
-			// that this value stays purged long enough so that
-			// it isn't refreshed with a stale value due to a
-			// lagged slave.
-			$cache->delete( $memcKey, 12 );
+		$key = $this->getSharedCacheKey( 'image_redirect', md5( $title->getDBkey() ) );
+		if ( $key ) {
+			$this->getMasterDB()->onTransactionPreCommitOrIdle( function() use ( $key ) {
+				ObjectCache::getMainWANInstance()->delete( $key );
+			} );
 		}
 	}
 

includes/filerepo/file/LocalFile.php

@@ -321,7 +321,9 @@ class LocalFile extends File {
 			return;
 		}
 
-		ObjectCache::getMainWANInstance()->delete( $key );
+		$this->repo->getMasterDB()->onTransactionPreCommitOrIdle( function() use ( $key ) {
+			ObjectCache::getMainWANInstance()->delete( $key );
+		} );
 	}
 
 	/**

includes/libs/objectcache/WANObjectCache.php

@@ -286,6 +286,31 @@ 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
 	 *
+	 * When using potentially long-running ACID transactions, a good pattern is
+	 * to use a pre-commit hook to issue the delete. This means that immediately
+	 * after commit, callers will see the tombstone in cache in the local datacenter
+	 * and in the others upon relay. It also avoids the following race condition:
+	 *   - a) T1 begins, changes a row, and calls delete()
+	 *   - b) The HOLDOFF_TTL passes, expiring the delete() tombstone
+	 *   - c) T2 starts, reads the row and calls set() due to a cache miss
+	 *   - d) T1 finally commits
+	 *   - e) Stale value is stuck in cache
+	 *
+	 * Example usage:
+	 * @code
+	 *     $dbw->begin(); // start of request
+	 *     ... <execute some stuff> ...
+	 *     // Update the row in the DB
+	 *     $dbw->update( ... );
+	 *     $key = wfMemcKey( 'homes', $homeId );
+	 *     // Purge the corresponding cache entry just before committing
+	 *     $dbw->onTransactionPreCommitOrIdle( function() use ( $cache, $key ) {
+	 *         $cache->delete( $key );
+	 *     } );
+	 *     ... <execute some stuff> ...
+	 *     $dbw->commit(); // end of request
+	 * @endcode
+	 *
 	 * 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