From a4d737f22942d71252ad78e18d0221a581ed455e Mon Sep 17 00:00:00 2001
From: Aaron Schulz <aschulz@wikimedia.org>
Date: Tue, 26 Jul 2016 18:44:41 -0700
Subject: [PATCH] Remove web-request usages of deadlockLoop()

Add comment discouraging use of the method. There are no problems
with deadlocks/timeouts in these code paths according to WMF logs.

Change-Id: I5b21cc423df584efa881361063000e01932cc2ea
---
 includes/db/Database.php         | 5 +++++
 includes/import/WikiImporter.php | 9 +++------
 includes/import/WikiRevision.php | 6 ++++--
 3 files changed, 12 insertions(+), 8 deletions(-)

diff --git a/includes/db/Database.php b/includes/db/Database.php
index 7d04cacd40..2e3e22552b 100644
--- a/includes/db/Database.php
+++ b/includes/db/Database.php
@@ -2388,14 +2388,19 @@ abstract class DatabaseBase implements IDatabase {
 	 * queries. If a deadlock occurs during the processing, the transaction
 	 * will be rolled back and the callback function will be called again.
 	 *
+	 * Avoid using this method outside of Job or Maintenance classes.
+	 *
 	 * Usage:
 	 *   $dbw->deadlockLoop( callback, ... );
 	 *
 	 * Extra arguments are passed through to the specified callback function.
+	 * This method requires that no transactions are already active to avoid
+	 * causing premature commits or exceptions.
 	 *
 	 * Returns whatever the callback function returned on its successful,
 	 * iteration, or false on error, for example if the retry limit was
 	 * reached.
+	 *
 	 * @return mixed
 	 * @throws DBUnexpectedError
 	 * @throws Exception
diff --git a/includes/import/WikiImporter.php b/includes/import/WikiImporter.php
index 826bb59caf..406667e14d 100644
--- a/includes/import/WikiImporter.php
+++ b/includes/import/WikiImporter.php
@@ -332,8 +332,7 @@ class WikiImporter {
 		}
 
 		try {
-			$dbw = wfGetDB( DB_MASTER );
-			return $dbw->deadlockLoop( [ $revision, 'importOldRevision' ] );
+			return $revision->importOldRevision();
 		} catch ( MWContentSerializationException $ex ) {
 			$this->notice( 'import-error-unserialize',
 				$revision->getTitle()->getPrefixedText(),
@@ -351,8 +350,7 @@ class WikiImporter {
 	 * @return bool
 	 */
 	public function importLogItem( $revision ) {
-		$dbw = wfGetDB( DB_MASTER );
-		return $dbw->deadlockLoop( [ $revision, 'importLogItem' ] );
+		return $revision->importLogItem();
 	}
 
 	/**
@@ -361,8 +359,7 @@ class WikiImporter {
 	 * @return bool
 	 */
 	public function importUpload( $revision ) {
-		$dbw = wfGetDB( DB_MASTER );
-		return $dbw->deadlockLoop( [ $revision, 'importUpload' ] );
+		return $revision->importUpload();
 	}
 
 	/**
diff --git a/includes/import/WikiRevision.php b/includes/import/WikiRevision.php
index 356a79f82d..d78d61acd8 100644
--- a/includes/import/WikiRevision.php
+++ b/includes/import/WikiRevision.php
@@ -574,7 +574,7 @@ class WikiRevision {
 		if ( !$this->getTitle() ) {
 			wfDebug( __METHOD__ . ": skipping invalid {$this->type}/{$this->action} log time, timestamp " .
 				$this->timestamp . "\n" );
-			return;
+			return false;
 		}
 		# Check if it exists already
 		// @todo FIXME: Use original log ID (better for backups)
@@ -594,7 +594,7 @@ class WikiRevision {
 			wfDebug( __METHOD__
 				. ": skipping existing item for Log:{$this->type}/{$this->action}, timestamp "
 				. $this->timestamp . "\n" );
-			return;
+			return false;
 		}
 		$log_id = $dbw->nextSequenceValue( 'logging_log_id_seq' );
 		$data = [
@@ -610,6 +610,8 @@ class WikiRevision {
 			'log_params' => $this->params
 		];
 		$dbw->insert( 'logging', $data, __METHOD__ );
+
+		return true;
 	}
 
 	/**
-- 
2.20.1