jenkins-bot has submitted this change and it was merged.
Change subject: Avoid lock error exceptions during upgradeRow() contention
......................................................................
Avoid lock error exceptions during upgradeRow() contention
Bug: T132921
Change-Id: I229031c3d4ae5b700fcc4d4dd3f5208a853823dc
---
M autoload.php
M includes/filerepo/file/LocalFile.php
2 files changed, 26 insertions(+), 11 deletions(-)
Approvals:
Ori.livneh: Looks good to me, approved
jenkins-bot: Verified
diff --git a/autoload.php b/autoload.php
index f802ddd..3b17215 100644
--- a/autoload.php
+++ b/autoload.php
@@ -711,6 +711,7 @@
'LoadMonitorNull' => __DIR__ .
'/includes/db/loadbalancer/LoadMonitor.php',
'LocalFile' => __DIR__ . '/includes/filerepo/file/LocalFile.php',
'LocalFileDeleteBatch' => __DIR__ .
'/includes/filerepo/file/LocalFile.php',
+ 'LocalFileLockError' => __DIR__ .
'/includes/filerepo/file/LocalFile.php',
'LocalFileMoveBatch' => __DIR__ .
'/includes/filerepo/file/LocalFile.php',
'LocalFileRestoreBatch' => __DIR__ .
'/includes/filerepo/file/LocalFile.php',
'LocalIdLookup' => __DIR__ . '/includes/user/LocalIdLookup.php',
diff --git a/includes/filerepo/file/LocalFile.php
b/includes/filerepo/file/LocalFile.php
index aa278aa..f7275fc 100644
--- a/includes/filerepo/file/LocalFile.php
+++ b/includes/filerepo/file/LocalFile.php
@@ -559,11 +559,11 @@
return;
}
+ $upgrade = false;
if ( is_null( $this->media_type ) ||
$this->mime == 'image/svg'
) {
- $this->upgradeRow();
- $this->upgraded = true;
+ $upgrade = true;
} else {
$handler = $this->getHandler();
if ( $handler ) {
@@ -571,10 +571,18 @@
if ( $validity === MediaHandler::METADATA_BAD
|| ( $validity ===
MediaHandler::METADATA_COMPATIBLE && $wgUpdateCompatibleMetadata )
) {
- $this->upgradeRow();
- $this->upgraded = true;
+ $upgrade = true;
}
}
+ }
+
+ if ( $upgrade ) {
+ try {
+ $this->upgradeRow();
+ } catch ( LocalFileLockError $e ) {
+ // let the other process handle it (or do it
next time)
+ }
+ $this->upgraded = true; // avoid rework/retries
}
}
@@ -586,7 +594,6 @@
* Fix assorted version-related problems with the image row by
reloading it from the file
*/
function upgradeRow() {
-
$this->lock(); // begin
$this->loadFromFile();
@@ -1898,18 +1905,16 @@
/**
* 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
+ * @throws LocalFileLockError Throws an error if the lock was not
acquired
* @return bool Whether the file lock owns/spawned the DB transaction
*/
function lock() {
- $dbw = $this->repo->getMasterDB();
-
if ( !$this->locked ) {
+ $dbw = $this->repo->getMasterDB();
if ( !$dbw->trxLevel() ) {
$dbw->begin( __METHOD__ );
$this->lockedOwnTrx = true;
}
- $this->locked++;
// Bug 54736: use simple lock to handle when the file
does not exist.
// SELECT FOR UPDATE prevents changes, not other
SELECTs with FOR UPDATE.
// Also, that would cause contention on INSERT of
similarly named rows.
@@ -1917,10 +1922,15 @@
$lockPaths = [ $this->getPath() ]; // represents all
versions of the file
$status = $backend->lockFiles( $lockPaths,
LockManager::LOCK_EX, 5 );
if ( !$status->isGood() ) {
- throw new MWException( "Could not acquire lock
for '{$this->getName()}.'" );
+ if ( $this->lockedOwnTrx ) {
+ $dbw->rollback( __METHOD__ );
+ }
+ throw new LocalFileLockError( "Could not
acquire lock for '{$this->getName()}.'" );
}
+ // Release the lock *after* commit to avoid row-level
contention
+ $this->locked++;
$dbw->onTransactionIdle( function () use ( $backend,
$lockPaths ) {
- $backend->unlockFiles( $lockPaths,
LockManager::LOCK_EX ); // release on commit
+ $backend->unlockFiles( $lockPaths,
LockManager::LOCK_EX );
} );
}
@@ -3031,3 +3041,7 @@
$this->file->repo->cleanupBatch( $files );
}
}
+
+class LocalFileLockError extends Exception {
+
+}
--
To view, visit https://gerrit.wikimedia.org/r/284213
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I229031c3d4ae5b700fcc4d4dd3f5208a853823dc
Gerrit-PatchSet: 4
Gerrit-Project: mediawiki/core
Gerrit-Branch: master
Gerrit-Owner: Aaron Schulz <[email protected]>
Gerrit-Reviewer: Gilles <[email protected]>
Gerrit-Reviewer: MaxSem <[email protected]>
Gerrit-Reviewer: Ori.livneh <[email protected]>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
