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

diff --git a/includes/LinksUpdate.php b/includes/LinksUpdate.php
index 1d0bcf7..87db4d6 100644 (file)
--- 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 aeb9ba4..52c9be0 100644 (file)
--- 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;
                }
        }