From 45daade27ceb91538b6d4b83ca154a3561b94e4d Mon Sep 17 00:00:00 2001
From: Aaron Schulz <aschulz@wikimedia.org>
Date: Sat, 20 Jul 2013 11:48:06 -0700
Subject: [PATCH] Avoid "Transaction already in progress" errors in
 SiteStatsUpdate::doUpdate

* Also fixed TRX callback code to match documentation (wrong since 3b7c4f6)

Change-Id: I8c4ea7412e9f96912939c441c68090eacc42b3d4
---
 includes/SiteStats.php   | 86 +++++++++++++++++++++-------------------
 includes/db/Database.php |  7 ++--
 2 files changed, 49 insertions(+), 44 deletions(-)

diff --git a/includes/SiteStats.php b/includes/SiteStats.php
index 199c64fa9b..6e2359ae54 100644
--- a/includes/SiteStats.php
+++ b/includes/SiteStats.php
@@ -297,50 +297,56 @@ class SiteStatsUpdate implements DeferrableUpdate {
 		if ( $rate && ( $rate < 0 || mt_rand( 0, $rate - 1 ) != 0 ) ) {
 			$this->doUpdatePendingDeltas();
 		} else {
-			$dbw = wfGetDB( DB_MASTER );
-
-			$lockKey = wfMemcKey( 'site_stats' ); // prepend wiki ID
-			if ( $rate ) {
-				// Lock the table so we don't have double DB/memcached updates
-				if ( !$dbw->lockIsFree( $lockKey, __METHOD__ )
-					|| !$dbw->lock( $lockKey, __METHOD__, 1 ) // 1 sec timeout
-				) {
-					$this->doUpdatePendingDeltas();
-					return;
-				}
-				$pd = $this->getPendingDeltas();
-				// Piggy-back the async deltas onto those of this stats update....
-				$this->views += ( $pd['ss_total_views']['+'] - $pd['ss_total_views']['-'] );
-				$this->edits += ( $pd['ss_total_edits']['+'] - $pd['ss_total_edits']['-'] );
-				$this->articles += ( $pd['ss_good_articles']['+'] - $pd['ss_good_articles']['-'] );
-				$this->pages += ( $pd['ss_total_pages']['+'] - $pd['ss_total_pages']['-'] );
-				$this->users += ( $pd['ss_users']['+'] - $pd['ss_users']['-'] );
-				$this->images += ( $pd['ss_images']['+'] - $pd['ss_images']['-'] );
-			}
-
 			// Need a separate transaction because this a global lock
-			$dbw->begin( __METHOD__ );
-
-			// Build up an SQL query of deltas and apply them...
-			$updates = '';
-			$this->appendUpdate( $updates, 'ss_total_views', $this->views );
-			$this->appendUpdate( $updates, 'ss_total_edits', $this->edits );
-			$this->appendUpdate( $updates, 'ss_good_articles', $this->articles );
-			$this->appendUpdate( $updates, 'ss_total_pages', $this->pages );
-			$this->appendUpdate( $updates, 'ss_users', $this->users );
-			$this->appendUpdate( $updates, 'ss_images', $this->images );
-			if ( $updates != '' ) {
-				$dbw->update( 'site_stats', array( $updates ), array(), __METHOD__ );
-			}
+			wfGetDB( DB_MASTER )->onTransactionIdle( array( $this, 'tryDBUpdateInternal' ) );
+		}
+	}
 
-			if ( $rate ) {
-				// Decrement the async deltas now that we applied them
-				$this->removePendingDeltas( $pd );
-				// Commit the updates and unlock the table
-				$dbw->unlock( $lockKey, __METHOD__ );
+	/**
+	 * Do not call this outside of SiteStatsUpdate
+	 *
+	 * @return void
+	 */
+	public function tryDBUpdateInternal() {
+		global $wgSiteStatsAsyncFactor;
+
+		$dbw = wfGetDB( DB_MASTER );
+		$lockKey = wfMemcKey( 'site_stats' ); // prepend wiki ID
+		if ( $wgSiteStatsAsyncFactor ) {
+			// Lock the table so we don't have double DB/memcached updates
+			if ( !$dbw->lockIsFree( $lockKey, __METHOD__ )
+				|| !$dbw->lock( $lockKey, __METHOD__, 1 ) // 1 sec timeout
+			) {
+				$this->doUpdatePendingDeltas();
+				return;
 			}
+			$pd = $this->getPendingDeltas();
+			// Piggy-back the async deltas onto those of this stats update....
+			$this->views += ( $pd['ss_total_views']['+'] - $pd['ss_total_views']['-'] );
+			$this->edits += ( $pd['ss_total_edits']['+'] - $pd['ss_total_edits']['-'] );
+			$this->articles += ( $pd['ss_good_articles']['+'] - $pd['ss_good_articles']['-'] );
+			$this->pages += ( $pd['ss_total_pages']['+'] - $pd['ss_total_pages']['-'] );
+			$this->users += ( $pd['ss_users']['+'] - $pd['ss_users']['-'] );
+			$this->images += ( $pd['ss_images']['+'] - $pd['ss_images']['-'] );
+		}
+
+		// Build up an SQL query of deltas and apply them...
+		$updates = '';
+		$this->appendUpdate( $updates, 'ss_total_views', $this->views );
+		$this->appendUpdate( $updates, 'ss_total_edits', $this->edits );
+		$this->appendUpdate( $updates, 'ss_good_articles', $this->articles );
+		$this->appendUpdate( $updates, 'ss_total_pages', $this->pages );
+		$this->appendUpdate( $updates, 'ss_users', $this->users );
+		$this->appendUpdate( $updates, 'ss_images', $this->images );
+		if ( $updates != '' ) {
+			$dbw->update( 'site_stats', array( $updates ), array(), __METHOD__ );
+		}
 
-			$dbw->commit( __METHOD__ );
+		if ( $wgSiteStatsAsyncFactor ) {
+			// Decrement the async deltas now that we applied them
+			$this->removePendingDeltas( $pd );
+			// Commit the updates and unlock the table
+			$dbw->unlock( $lockKey, __METHOD__ );
 		}
 	}
 
diff --git a/includes/db/Database.php b/includes/db/Database.php
index f9f4d5d9d3..c69ba526ab 100644
--- a/includes/db/Database.php
+++ b/includes/db/Database.php
@@ -3104,10 +3104,9 @@ abstract class DatabaseBase implements IDatabase, DatabaseType {
 			foreach ( $callbacks as $callback ) {
 				try {
 					$this->clearFlag( DBO_TRX ); // make each query its own transaction
-					$callback();
+					call_user_func( $callback );
 					$this->setFlag( $autoTrx ? DBO_TRX : 0 ); // restore automatic begin()
-				} catch ( Exception $e ) {
-				}
+				} catch ( Exception $e ) {}
 			}
 		} while ( count( $this->mTrxIdleCallbacks ) );
 
@@ -3128,7 +3127,7 @@ abstract class DatabaseBase implements IDatabase, DatabaseType {
 			$this->mTrxPreCommitCallbacks = array(); // recursion guard
 			foreach ( $callbacks as $callback ) {
 				try {
-					$callback();
+					call_user_func( $callback );
 				} catch ( Exception $e ) {}
 			}
 		} while ( count( $this->mTrxPreCommitCallbacks ) );
-- 
2.20.1