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 933bea6..5ece4c1 100644 (file)
--- 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 efc6148..b320544 100644 (file)
--- 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 d08172a..c6d535a 100644 (file)
--- 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;
        }
 
        /**