Avoid redundant COMMIT calls on page views

These just clear empty transactions and do not do anything
otherwise. No need in doing a few extra RTTs for that.

This also avoids "could not COMMIT" errors when transaction
callbacks do silly things like add more callbacks to outside
DBs. In that case, they either ran their callbacks already or
calling clearSnapshot() will error out.

Change-Id: I4d52c4ef69b02a33adffd8892b98ad1bd5c7d6dc

diff --git a/includes/db/loadbalancer/LoadBalancer.php b/includes/db/loadbalancer/LoadBalancer.php
index 9ceae20..f66636f 100644 (file)
--- a/includes/db/loadbalancer/LoadBalancer.php
+++ b/includes/db/loadbalancer/LoadBalancer.php
@@ -1244,9 +1244,21 @@ class LoadBalancer {
        public function runMasterPostTrxCallbacks( $type ) {
                $e = null; // first exception
                $this->forEachOpenMasterConnection( function ( DatabaseBase $conn ) use ( $type, &$e ) {
-                       $conn->clearSnapshot( __METHOD__ ); // clear no-op transactions
-
                        $conn->setTrxEndCallbackSuppression( false );
+                       if ( $conn->writesOrCallbacksPending() ) {
+                               // This happens if onTransactionIdle() callbacks leave callbacks on *another* DB
+                               // (which finished its callbacks already). Warn and recover in this case. Let the
+                               // callbacks run in the final commitMasterChanges() in LBFactory::shutdown().
+                               wfWarn( __METHOD__ . ": did not expect writes/callbacks pending." );
+                               return;
+                       } elseif ( $conn->trxLevel() ) {
+                               // This happens for single-DB setups where DB_REPLICA uses the master DB,
+                               // thus leaving an implicit read-only transaction open at this point. It
+                               // also happens if onTransactionIdle() callbacks leave implicit transactions
+                               // open on *other* DBs (which is slightly improper). Let these COMMIT on the
+                               // next call to commitMasterChanges(), possibly in LBFactory::shutdown().
+                               return;
+                       }
                        try {
                                $conn->runOnTransactionIdleCallbacks( $type );
                        } catch ( Exception $ex ) {