From 41f2132fe83ab31c77c8d02a7e944d4123720dd0 Mon Sep 17 00:00:00 2001
From: daniel <daniel.kinzler@wikimedia.de>
Date: Mon, 27 Aug 2012 14:32:06 +0200
Subject: [PATCH] Make LinksUpdate run without a db transaction.

This causes LinksUpdate not to be wrapped by a transaction, fixing
a problem caused by the introduction of SqlDataUpdate. LinksUpdate
never used a transaction wrapping the entire update, but only small
transactions for some database operations. SqlDataUpdate however introduced
an implicite transaction bracket for the entire update, causing the old,
inner transactions to be nested transactions, which is unsupported and
may cause database corruption (because starting a "nested" transaction
will commit the previous transaction prematurely).

Once we have support for nested transactions, LinksUpdate may again use a
transaction bracket for the entire update, but this is also subject to
performance considerations.

Change-Id: I80faf2ed79b56a3990a1724516e65621ca5bbece
---
 includes/LinksUpdate.php   |  4 ++--
 includes/SqlDataUpdate.php | 23 ++++++++++++++++++-----
 2 files changed, 20 insertions(+), 7 deletions(-)

diff --git a/includes/LinksUpdate.php b/includes/LinksUpdate.php
index 1d0bcf78d2..87db4d60da 100644
--- a/includes/LinksUpdate.php
+++ b/includes/LinksUpdate.php
@@ -51,7 +51,7 @@ class LinksUpdate extends SqlDataUpdate {
 	 * @param $recursive Boolean: queue jobs for recursive updates?
 	 */
 	function __construct( $title, $parserOutput, $recursive = true ) {
-		parent::__construct( );
+		parent::__construct( false ); // no implicit transaction
 
 		if ( !( $title instanceof Title ) ) {
 			throw new MWException( "The calling convention to LinksUpdate::LinksUpdate() has changed. " .
@@ -825,7 +825,7 @@ class LinksDeletionUpdate extends SqlDataUpdate {
 	 * @param $page WikiPage Page we are updating
 	 */
 	function __construct( WikiPage $page ) {
-		parent::__construct( );
+		parent::__construct( false ); // no implicit transaction
 
 		$this->mPage = $page;
 	}
diff --git a/includes/SqlDataUpdate.php b/includes/SqlDataUpdate.php
index aeb9ba4c7f..52c9be00d4 100644
--- a/includes/SqlDataUpdate.php
+++ b/includes/SqlDataUpdate.php
@@ -36,11 +36,16 @@ abstract class SqlDataUpdate extends DataUpdate {
 	protected $mOptions;       //!< SELECT options to be used (array)
 
 	private   $mHasTransaction; //!< bool whether a transaction is open on this object (internal use only!)
+	protected $mUseTransaction; //!< bool whether this update should be wrapped in a transaction
 
 	/**
 	 * Constructor
-	**/
-	public function __construct( ) {
+	 *
+	 * @param bool $withTransaction whether this update should be wrapped in a transaction (default: true).
+	 *             A transaction is only started if no transaction is already in progress,
+	 *             see beginTransaction() for details.
+	 **/
+	public function __construct( $withTransaction = true ) {
 		global $wgAntiLockFlags;
 
 		parent::__construct( );
@@ -53,16 +58,22 @@ abstract class SqlDataUpdate extends DataUpdate {
 
 		// @todo: get connection only when it's needed? make sure that doesn't break anything, especially transactions!
 		$this->mDb = wfGetDB( DB_MASTER );
+
+		$this->mWithTransaction = $withTransaction;
 		$this->mHasTransaction = false;
 	}
 
 	/**
-	 * Begin a database transaction.
+	 * Begin a database transaction, if $withTransaction was given as true in the constructor for this SqlDataUpdate.
 	 *
-	 * Because nested transactions are not supportred by the Database class, this implementation
-	 * checkes Database::trxLevel() and only opens a transaction if none is yet active.
+	 * Because nested transactions are not supported by the Database class, this implementation
+	 * checks Database::trxLevel() and only opens a transaction if none is already active.
 	 */
 	public function beginTransaction() {
+		if ( !$this->mWithTransaction ) {
+			return;
+		}
+
 		// NOTE: nested transactions are not supported, only start a transaction if none is open
 		if ( $this->mDb->trxLevel() === 0 ) {
 			$this->mDb->begin( get_class( $this ) . '::beginTransaction'  );
@@ -76,6 +87,7 @@ abstract class SqlDataUpdate extends DataUpdate {
 	public function commitTransaction() {
 		if ( $this->mHasTransaction ) {
 			$this->mDb->commit( get_class( $this ) . '::commitTransaction' );
+			$this->mHasTransaction = false;
 		}
 	}
 
@@ -85,6 +97,7 @@ abstract class SqlDataUpdate extends DataUpdate {
 	public function abortTransaction() {
 		if ( $this->mHasTransaction ) {
 			$this->mDb->rollback( get_class( $this ) . '::abortTransaction' );
+			$this->mHasTransaction = false;
 		}
 	}
 
-- 
2.20.1