[FileRepo] Locking and transaction fixes.
authorAaron Schulz <aschulz@wikimedia.org>
Mon, 7 May 2012 06:28:03 +0000 (23:28 -0700)
committerAaron Schulz <aschulz@wikimedia.org>
Mon, 7 May 2012 06:28:07 +0000 (23:28 -0700)
* Make sure locks for file moves cover the whole operation.
* Pushed purging out of the move/delete/restore transactions.

Change-Id: I398c5627808fa79739f0dee632c4edf7416507c1

includes/filerepo/file/LocalFile.php

index 701a1ec..68288e5 100644 (file)
@@ -1231,19 +1231,18 @@ class LocalFile extends File {
                }
 
                wfDebugLog( 'imagemove', "Got request to move {$this->name} to " . $target->getText() );
-               $this->lock(); // begin
-
                $batch = new LocalFileMoveBatch( $this, $target );
+
+               $this->lock(); // begin
                $batch->addCurrent();
                $batch->addOlds();
-
                $status = $batch->execute();
+               $this->unlock(); // done
+
                wfDebugLog( 'imagemove', "Finished moving {$this->name}" );
 
                $this->purgeEverything();
-               $this->unlock(); // done
-
-               if ( $status->isOk() ) {
+               if ( $status->isOK() ) {
                        // Now switch the object
                        $this->title = $target;
                        // Force regeneration of the name and hashpath
@@ -1273,31 +1272,33 @@ class LocalFile extends File {
                        return $this->readOnlyFatalStatus();
                }
 
-               $this->lock(); // begin
-
+               $dbw = $this->repo->getMasterDB();
                $batch = new LocalFileDeleteBatch( $this, $reason, $suppress );
-               $batch->addCurrent();
+               $archiveNames = array();
 
+               $this->lock(); // begin
+               $batch->addCurrent();
                # Get old version relative paths
-               $dbw = $this->repo->getMasterDB();
                $result = $dbw->select( 'oldimage',
                        array( 'oi_archive_name' ),
                        array( 'oi_name' => $this->getName() ) );
                foreach ( $result as $row ) {
                        $batch->addOld( $row->oi_archive_name );
-                       $this->purgeOldThumbnails( $row->oi_archive_name );
+                       $archiveNames[] = $row->oi_archive_name;
                }
                $status = $batch->execute();
-
                if ( $status->isOK() ) {
                        // Update site_stats
                        $site_stats = $dbw->tableName( 'site_stats' );
                        $dbw->query( "UPDATE $site_stats SET ss_images=ss_images-1", __METHOD__ );
-                       $this->purgeEverything();
                }
-
                $this->unlock(); // done
 
+               $this->purgeEverything();
+               foreach ( $archiveNames as $archiveName ) {
+                       $this->purgeOldThumbnails( $archiveName );
+               }
+
                return $status;
        }
 
@@ -1353,25 +1354,21 @@ class LocalFile extends File {
                        return $this->readOnlyFatalStatus();
                }
 
-               $this->lock(); // begin
-
                $batch = new LocalFileRestoreBatch( $this, $unsuppress );
 
+               $this->lock(); // begin
                if ( !$versions ) {
                        $batch->addAll();
                } else {
                        $batch->addIds( $versions );
                }
-
                $status = $batch->execute();
-
                if ( $status->isGood() ) {
                        $cleanupStatus = $batch->cleanup();
                        $cleanupStatus->successCount = 0;
                        $cleanupStatus->failCount = 0;
                        $status->merge( $cleanupStatus );
                }
-
                $this->unlock(); // done
 
                return $status;
@@ -2171,7 +2168,7 @@ class LocalFileRestoreBatch {
 class LocalFileMoveBatch {
 
        /**
-        * @var File
+        * @var LocalFile
         */
        var $file;
 
@@ -2266,17 +2263,17 @@ class LocalFileMoveBatch {
                        return $statusMove;
                }
 
-               $this->db->begin( __METHOD__ );
+               $this->file->lock(); // begin
                $statusDb = $this->doDBUpdates();
                wfDebugLog( 'imagemove', "Renamed {$this->file->getName()} in database: {$statusDb->successCount} successes, {$statusDb->failCount} failures" );
                if ( !$statusDb->isGood() ) {
-                       $this->db->rollback( __METHOD__ );
+                       $this->file->unlockAndRollback();
                        // Something went wrong with the DB updates, so remove the target files
                        $this->cleanupTarget( $triplets );
                        $statusDb->ok = false;
                        return $statusDb;
                }
-               $this->db->commit( __METHOD__ );
+               $this->file->unlock(); // done
 
                // Everything went ok, remove the source files
                $this->cleanupSource( $triplets );