From 3cc06f6d8c06ddddea43ceac17eebd6ef2368a08 Mon Sep 17 00:00:00 2001
From: Aaron Schulz <aschulz@wikimedia.org>
Date: Mon, 31 Mar 2014 13:36:14 -0700
Subject: [PATCH] Speed up LocalFile locking behavior

* Lowered the lock() timeout
* Wrap the destination DB write with memcached locks for move
  ops since it is easier to control that lock timeout than the
  DB lock timeouts.
* Cleanup locks in the destructor (e.g. on exceptions). Other
  process can pile up more if locks have to expire.

bug: 63058
Change-Id: I38b28d81ec96daa80ece2354db284a614289ba0b
---
 includes/filerepo/file/LocalFile.php | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/includes/filerepo/file/LocalFile.php b/includes/filerepo/file/LocalFile.php
index cfa26b0fed..7a9e6171cc 100644
--- a/includes/filerepo/file/LocalFile.php
+++ b/includes/filerepo/file/LocalFile.php
@@ -1830,7 +1830,7 @@ class LocalFile extends File {
 			// SELECT FOR UPDATE only locks records not the gaps where there are none.
 			$cache = wfGetMainCache();
 			$key = $this->getCacheKey();
-			if ( !$cache->lock( $key, 60 ) ) {
+			if ( !$cache->lock( $key, 5 ) ) {
 				throw new MWException( "Could not acquire lock for '{$this->getName()}.'" );
 			}
 			$dbw->onTransactionIdle( function () use ( $cache, $key ) {
@@ -1874,6 +1874,13 @@ class LocalFile extends File {
 		return $this->getRepo()->newFatal( 'filereadonlyerror', $this->getName(),
 			$this->getRepo()->getName(), $this->getRepo()->getReadOnlyReason() );
 	}
+
+	/**
+	 * Clean up any dangling locks
+	 */
+	function __destruct() {
+		$this->unlock();
+	}
 } // LocalFile class
 
 # ------------------------------------------------------------------------------
@@ -2714,8 +2721,10 @@ class LocalFileMoveBatch {
 
 		$triplets = $this->getMoveTriplets();
 		$triplets = $this->removeNonexistentFiles( $triplets );
+		$destFile = wfLocalFile( $this->target );
 
 		$this->file->lock(); // begin
+		$destFile->lock(); // quickly fail if destination is not available
 		// Rename the file versions metadata in the DB.
 		// This implicitly locks the destination file, which avoids race conditions.
 		// If we moved the files from A -> C before DB updates, another process could
@@ -2746,6 +2755,7 @@ class LocalFileMoveBatch {
 
 			return $statusMove;
 		}
+		$destFile->unlock();
 		$this->file->unlock(); // done
 
 		// Everything went ok, remove the source files
-- 
2.20.1