From: Aaron Schulz <aschulz@wikimedia.org>
Date: Fri, 19 Aug 2016 22:25:08 +0000 (-0700)
Subject: Give all idle transaction callbacks a chance to run
X-Git-Tag: 1.31.0-rc.0~5981^2
X-Git-Url: http://git.cyclocoop.org/%24image?a=commitdiff_plain;h=979f550b6fbb16191aeef0f1c1bbbd6ff30eb26e;p=lhc%2Fweb%2Fwiklou.git

Give all idle transaction callbacks a chance to run

Catch exceptions from other DB handle callback runs.
Also use the first exception instead of the last for
callback runs, as the it is more likely to be meaningfull.

Change-Id: Ib180d684b090ae26ad6ec0854322d5cb4286cc81
---

diff --git a/includes/db/Database.php b/includes/db/Database.php
index 933bea60a9..5ece4c1e2d 100644
--- a/includes/db/Database.php
+++ b/includes/db/Database.php
@@ -2559,7 +2559,7 @@ abstract class DatabaseBase implements IDatabase {
 
 		$autoTrx = $this->getFlag( DBO_TRX ); // automatic begin() enabled?
 		/** @var Exception $e */
-		$e = $ePrior = null; // last exception
+		$e = null; // first exception
 		do { // callbacks may add callbacks :)
 			$callbacks = array_merge(
 				$this->mTrxIdleCallbacks,
@@ -2577,11 +2577,9 @@ abstract class DatabaseBase implements IDatabase {
 					} else {
 						$this->clearFlag( DBO_TRX ); // restore auto-commit
 					}
-				} catch ( Exception $e ) {
-					if ( $ePrior ) {
-						MWExceptionHandler::logException( $ePrior );
-					}
-					$ePrior = $e;
+				} catch ( Exception $ex ) {
+					MWExceptionHandler::logException( $ex );
+					$e = $e ?: $ex;
 					// Some callbacks may use startAtomic/endAtomic, so make sure
 					// their transactions are ended so other callbacks don't fail
 					if ( $this->trxLevel() ) {
@@ -2592,7 +2590,7 @@ abstract class DatabaseBase implements IDatabase {
 		} while ( count( $this->mTrxIdleCallbacks ) );
 
 		if ( $e instanceof Exception ) {
-			throw $e; // re-throw any last exception
+			throw $e; // re-throw any first exception
 		}
 	}
 
@@ -2602,9 +2600,10 @@ abstract class DatabaseBase implements IDatabase {
 	 * This method should not be used outside of Database/LoadBalancer
 	 *
 	 * @since 1.22
+	 * @throws Exception
 	 */
 	public function runOnTransactionPreCommitCallbacks() {
-		$e = $ePrior = null; // last exception
+		$e = null; // first exception
 		do { // callbacks may add callbacks :)
 			$callbacks = $this->mTrxPreCommitCallbacks;
 			$this->mTrxPreCommitCallbacks = []; // consumed (and recursion guard)
@@ -2612,17 +2611,15 @@ abstract class DatabaseBase implements IDatabase {
 				try {
 					list( $phpCallback ) = $callback;
 					call_user_func( $phpCallback );
-				} catch ( Exception $e ) {
-					if ( $ePrior ) {
-						MWExceptionHandler::logException( $ePrior );
-					}
-					$ePrior = $e;
+				} catch ( Exception $ex ) {
+					MWExceptionHandler::logException( $ex );
+					$e = $e ?: $ex;
 				}
 			}
 		} while ( count( $this->mTrxPreCommitCallbacks ) );
 
 		if ( $e instanceof Exception ) {
-			throw $e; // re-throw any last exception
+			throw $e; // re-throw any first exception
 		}
 	}
 
diff --git a/includes/db/loadbalancer/LBFactory.php b/includes/db/loadbalancer/LBFactory.php
index efc6148fee..b320544e01 100644
--- a/includes/db/loadbalancer/LBFactory.php
+++ b/includes/db/loadbalancer/LBFactory.php
@@ -234,6 +234,7 @@ abstract class LBFactory implements DestructibleService {
 	 * @param string $fname Caller name
 	 * @param array $options Options map:
 	 *   - maxWriteDuration: abort if more than this much time was spent in write queries
+	 * @throws Exception
 	 */
 	public function commitMasterChanges( $fname = __METHOD__, array $options = [] ) {
 		// Perform all pre-commit callbacks, aborting on failure
@@ -245,9 +246,18 @@ abstract class LBFactory implements DestructibleService {
 		// Actually perform the commit on all master DB connections
 		$this->forEachLBCallMethod( 'commitMasterChanges', [ $fname ] );
 		// Run all post-commit callbacks
-		$this->forEachLBCallMethod( 'runMasterPostCommitCallbacks' );
+		/** @var Exception $e */
+		$e = null; // first callback exception
+		$this->forEachLB( function ( LoadBalancer $lb ) use ( &$e ) {
+			$ex = $lb->runMasterPostCommitCallbacks();
+			$e = $e ?: $ex;
+		} );
 		// Commit any dangling DBO_TRX transactions from callbacks on one DB to another DB
 		$this->forEachLBCallMethod( 'commitMasterChanges', [ $fname ] );
+		// Throw any last post-commit callback error
+		if ( $e instanceof Exception ) {
+			throw $e;
+		}
 	}
 
 	/**
diff --git a/includes/db/loadbalancer/LoadBalancer.php b/includes/db/loadbalancer/LoadBalancer.php
index d08172a5e8..c6d535a5b8 100644
--- a/includes/db/loadbalancer/LoadBalancer.php
+++ b/includes/db/loadbalancer/LoadBalancer.php
@@ -1116,13 +1116,21 @@ class LoadBalancer {
 
 	/**
 	 * Issue all pending post-commit callbacks
+	 * @return Exception|null The first exception or null if there were none
 	 * @since 1.28
 	 */
 	public function runMasterPostCommitCallbacks() {
-		$this->forEachOpenMasterConnection( function ( DatabaseBase $db ) {
+		$e = null; // first exception
+		$this->forEachOpenMasterConnection( function ( DatabaseBase $db ) use ( &$e ) {
 			$db->setPostCommitCallbackSupression( false );
-			$db->runOnTransactionIdleCallbacks( IDatabase::TRIGGER_COMMIT );
+			try {
+				$db->runOnTransactionIdleCallbacks( IDatabase::TRIGGER_COMMIT );
+			} catch ( Exception $ex ) {
+				$e = $e ?: $ex;
+			}
 		} );
+
+		return $e;
 	}
 
 	/**