From eff682981cf0c317e16588fa62644e84d9af1c26 Mon Sep 17 00:00:00 2001
From: Aaron Schulz <aschulz@wikimedia.org>
Date: Fri, 4 May 2018 15:33:34 -0700
Subject: [PATCH] rdbms: consolidate LBFactory code for running callbacks

This makes sure rollbackMasterChanges() has the same looping logic
for consistency.

Also clarify some code comments.

Change-Id: Id34ba8ff7377c9f3842e2db675c8050a6e68407e
---
 includes/libs/rdbms/lbfactory/LBFactory.php   | 37 ++++++++++++-------
 .../libs/rdbms/loadbalancer/ILoadBalancer.php |  2 +-
 2 files changed, 25 insertions(+), 14 deletions(-)

diff --git a/includes/libs/rdbms/lbfactory/LBFactory.php b/includes/libs/rdbms/lbfactory/LBFactory.php
index ccaebd3cd4..fbc413e6d4 100644
--- a/includes/libs/rdbms/lbfactory/LBFactory.php
+++ b/includes/libs/rdbms/lbfactory/LBFactory.php
@@ -254,6 +254,29 @@ abstract class LBFactory implements ILBFactory {
 		$this->logIfMultiDbTransaction();
 		// Actually perform the commit on all master DB connections and revert DBO_TRX
 		$this->forEachLBCallMethod( 'commitMasterChanges', [ $fname ] );
+		// Run all post-commit callbacks in a separate step
+		$e = $this->executePostTransactionCallbacks();
+		$this->trxRoundStage = self::ROUND_CURSORY;
+		// Throw any last post-commit callback error
+		if ( $e instanceof Exception ) {
+			throw $e;
+		}
+	}
+
+	final public function rollbackMasterChanges( $fname = __METHOD__ ) {
+		$this->trxRoundStage = self::ROUND_ROLLING_BACK;
+		$this->trxRoundId = false;
+		// Actually perform the rollback on all master DB connections and revert DBO_TRX
+		$this->forEachLBCallMethod( 'rollbackMasterChanges', [ $fname ] );
+		// Run all post-commit callbacks in a separate step
+		$this->executePostTransactionCallbacks();
+		$this->trxRoundStage = self::ROUND_CURSORY;
+	}
+
+	/**
+	 * @return Exception|null
+	 */
+	private function executePostTransactionCallbacks() {
 		// Run all post-commit callbacks until new ones stop getting added
 		$e = null; // first callback exception
 		do {
@@ -267,20 +290,8 @@ abstract class LBFactory implements ILBFactory {
 			$ex = $lb->runMasterTransactionListenerCallbacks();
 			$e = $e ?: $ex;
 		} );
-		$this->trxRoundStage = self::ROUND_CURSORY;
-		// Throw any last post-commit callback error
-		if ( $e instanceof Exception ) {
-			throw $e;
-		}
-	}
 
-	final public function rollbackMasterChanges( $fname = __METHOD__ ) {
-		$this->trxRoundStage = self::ROUND_ROLLING_BACK;
-		$this->trxRoundId = false;
-		$this->forEachLBCallMethod( 'rollbackMasterChanges', [ $fname ] );
-		$this->forEachLBCallMethod( 'runMasterTransactionIdleCallbacks' );
-		$this->forEachLBCallMethod( 'runMasterTransactionListenerCallbacks' );
-		$this->trxRoundStage = self::ROUND_CURSORY;
+		return $e;
 	}
 
 	public function hasTransactionRound() {
diff --git a/includes/libs/rdbms/loadbalancer/ILoadBalancer.php b/includes/libs/rdbms/loadbalancer/ILoadBalancer.php
index dd257e5ffa..e69ec26574 100644
--- a/includes/libs/rdbms/loadbalancer/ILoadBalancer.php
+++ b/includes/libs/rdbms/loadbalancer/ILoadBalancer.php
@@ -416,7 +416,7 @@ interface ILoadBalancer {
 	public function commitMasterChanges( $fname = __METHOD__ );
 
 	/**
-	 * Consume and run all pending post-COMMIT/ROLLBACK callbacks
+	 * Consume and run all pending post-COMMIT/ROLLBACK callbacks and commit dangling transactions
 	 *
 	 * @return Exception|null The first exception or null if there were none
 	 */
-- 
2.20.1