Avoid the use of DB rollback() in LocalFileDeleteBatch
authorAaron Schulz <aschulz@wikimedia.org>
Thu, 21 Jul 2016 05:16:20 +0000 (22:16 -0700)
committerOri.livneh <ori@wikimedia.org>
Fri, 22 Jul 2016 05:03:54 +0000 (05:03 +0000)
Race conditions are already handled by LockManager now.

Change-Id: Idb9ac511d565db4920aff7faa6ff291e05079798

includes/filerepo/file/LocalFile.php

index 234dbac..e35af75 100644 (file)
@@ -2293,13 +2293,6 @@ class LocalFileDeleteBatch {
                        }
                }
 
-               // Lock the filearchive rows so that the files don't get deleted by a cleanup operation
-               // We acquire this lock by running the inserts now, before the file operations.
-               // This potentially has poor lock contention characteristics -- an alternative
-               // scheme would be to insert stub filearchive entries with no fa_name and commit
-               // them in a separate transaction, then run the file ops, then update the fa_name fields.
-               $this->doDBInserts();
-
                if ( !$repo->hasSha1Storage() ) {
                        // Removes non-existent file from the batch, so we don't get errors.
                        // This also handles files in the 'deleted' zone deleted via revision deletion.
@@ -2312,21 +2305,20 @@ class LocalFileDeleteBatch {
 
                        // Execute the file deletion batch
                        $status = $this->file->repo->deleteBatch( $this->deletionBatch );
-
                        if ( !$status->isGood() ) {
                                $this->status->merge( $status );
                        }
                }
 
                if ( !$this->status->isOK() ) {
-                       // Critical file deletion error
-                       // Roll back inserts, release lock and abort
-                       // TODO: delete the defunct filearchive rows if we are using a non-transactional DB
-                       $this->file->unlockAndRollback();
+                       // Critical file deletion error; abort
+                       $this->file->unlock();
 
                        return $this->status;
                }
 
+               // Copy the image/oldimage rows to filearchive
+               $this->doDBInserts();
                // Delete image/oldimage rows
                $this->doDBDeletes();