From 745637c3da4b0157ca2b2fe7f93296721ebbb856 Mon Sep 17 00:00:00 2001 From: Aaron Schulz Date: Tue, 7 Oct 2014 14:08:34 -0700 Subject: [PATCH] Reduced LocalFileRestoreBatch::execute deadlocks when doing batch restores * Often users restore a batch of many files with similar names. This fails due to gap locking on non-existing rows which tangles up unrelated actions. Avoid that locking for the common case. * This changes the return value of LocalFile::lock() from a useless one (due to the exception) to one that conveys some useful information. The method is fairly internal in any case. Change-Id: Idb86367cf45b731d4b2a67b1813a660bc0e7a84f --- includes/filerepo/file/LocalFile.php | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/includes/filerepo/file/LocalFile.php b/includes/filerepo/file/LocalFile.php index 6ca19fe1bf..08c9afb18e 100644 --- a/includes/filerepo/file/LocalFile.php +++ b/includes/filerepo/file/LocalFile.php @@ -1850,7 +1850,7 @@ class LocalFile extends File { * Start a transaction and lock the image for update * Increments a reference counter if the lock is already held * @throws MWException Throws an error if the lock was not acquired - * @return bool Success + * @return bool Whether the file lock owns/spawned the DB transaction */ function lock() { $dbw = $this->repo->getMasterDB(); @@ -1877,7 +1877,7 @@ class LocalFile extends File { $this->markVolatile(); // file may change soon - return true; + return $this->lockedOwnTrx; } /** @@ -2419,13 +2419,19 @@ class LocalFileRestoreBatch { return $this->file->repo->newGood(); } - $this->file->lock(); + $lockOwnsTrx = $this->file->lock(); $dbw = $this->file->repo->getMasterDB(); $status = $this->file->repo->newGood(); $exists = (bool)$dbw->selectField( 'image', '1', - array( 'img_name' => $this->file->getName() ), __METHOD__, array( 'FOR UPDATE' ) ); + array( 'img_name' => $this->file->getName() ), + __METHOD__, + // The lock() should already prevents changes, but this still may need + // to bypass any transaction snapshot. However, if lock() started the + // trx (which it probably did) then snapshot is post-lock and up-to-date. + $lockOwnsTrx ? array() : array( 'LOCK IN SHARE MODE' ) + ); // Fetch all or selected archived revisions for the file, // sorted from the most recent to the oldest. -- 2.20.1