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

diff --git a/includes/filerepo/file/LocalFile.php b/includes/filerepo/file/LocalFile.php
index cfa26b0..7a9e617 100644 (file)
--- 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