Make pre-commit DB callbacks more atomic with multi-DB updates

Bug: T119736
Change-Id: I141903145aae0f6874f4e631624e52b6b9d8a4c8

diff --git a/includes/db/Database.php b/includes/db/Database.php
index cfdf382..147bf27 100644 (file)
--- a/includes/db/Database.php
+++ b/includes/db/Database.php
@@ -2520,9 +2520,11 @@ abstract class DatabaseBase implements IDatabase {
        /**
         * Actually any "on transaction pre-commit" callbacks.
         *
+        * This method should not be used outside of Database/LoadBalancer
+        *
         * @since 1.22
         */
-       protected function runOnTransactionPreCommitCallbacks() {
+       public function runOnTransactionPreCommitCallbacks() {
                $e = $ePrior = null; // last exception
                do { // callbacks may add callbacks :)
                        $callbacks = $this->mTrxPreCommitCallbacks;

diff --git a/includes/db/loadbalancer/LBFactory.php b/includes/db/loadbalancer/LBFactory.php
index 5b048b5..9a1d679 100644 (file)
--- a/includes/db/loadbalancer/LBFactory.php
+++ b/includes/db/loadbalancer/LBFactory.php
@@ -224,6 +224,10 @@ abstract class LBFactory implements DestructibleService {
        public function commitMasterChanges( $fname = __METHOD__, array $options = [] ) {
                $limit = isset( $options['maxWriteDuration'] ) ? $options['maxWriteDuration'] : 0;
 
+               // Run pre-commit callbacks to keep them out of the COMMIT step. If one errors out here
+               // then all DB transactions can be rolled back before anything was committed yet.
+               $this->forEachLBCallMethod( 'runPreCommitCallbacks' );
+
                $this->logMultiDbTransaction();
                $this->forEachLB( function ( LoadBalancer $lb ) use ( $limit ) {
                        $lb->forEachOpenConnection( function ( IDatabase $db ) use ( $limit ) {

diff --git a/includes/db/loadbalancer/LoadBalancer.php b/includes/db/loadbalancer/LoadBalancer.php
index d96c665..d9a7381 100644 (file)
--- a/includes/db/loadbalancer/LoadBalancer.php
+++ b/includes/db/loadbalancer/LoadBalancer.php
@@ -1115,6 +1115,28 @@ class LoadBalancer {
                }
        }
 
+       /**
+        * Call runOnTransactionPreCommitCallbacks() on all DB handles
+        *
+        * This method should not be used outside of LBFactory/LoadBalancer
+        *
+        * @since 1.28
+        */
+       public function runPreCommitCallbacks() {
+               $masterIndex = $this->getWriterIndex();
+               foreach ( $this->mConns as $conns2 ) {
+                       if ( empty( $conns2[$masterIndex] ) ) {
+                               continue;
+                       }
+                       /** @var DatabaseBase $conn */
+                       foreach ( $conns2[$masterIndex] as $conn ) {
+                               if ( $conn->trxLevel() && $conn->writesOrCallbacksPending() ) {
+                                       $conn->runOnTransactionPreCommitCallbacks();
+                               }
+                       }
+               }
+       }
+
        /**
         * @return bool Whether a master connection is already open
         * @since 1.24