Clean up DeferredUpdates transaction handling

Bail out in attemptUpdate() if the transaction state is dirty rather
that failing at some later point. Also, flush implicit transaction
rounds before calling DeferrableUpdate::doUpdate() for fresher data.

Bug: T225103
Change-Id: I4f5d2f9814a562069619f05e003663fcedbd3f64

diff --git a/includes/deferred/DeferredUpdates.php b/includes/deferred/DeferredUpdates.php
index c754cff..d43ffbc 100644 (file)
--- a/includes/deferred/DeferredUpdates.php
+++ b/includes/deferred/DeferredUpdates.php
@@ -28,6 +28,7 @@ use MediaWiki\MediaWikiServices;
 use Wikimedia\Rdbms\LBFactory;
 use Wikimedia\Rdbms\ILBFactory;
 use Wikimedia\Rdbms\LoadBalancer;
+use Wikimedia\Rdbms\DBTransactionError;
 
 /**
  * Class for managing the deferred updates
@@ -352,28 +353,30 @@ class DeferredUpdates {
         * @since 1.34
         */
        public static function attemptUpdate( DeferrableUpdate $update, ILBFactory $lbFactory ) {
+               $ticket = $lbFactory->getEmptyTransactionTicket( __METHOD__ );
+               if ( !$ticket || $lbFactory->hasTransactionRound() ) {
+                       throw new DBTransactionError( null, "A database transaction round is pending." );
+               }
+
                if ( $update instanceof DataUpdate ) {
-                       $update->setTransactionTicket( $lbFactory->getEmptyTransactionTicket( __METHOD__ ) );
+                       $update->setTransactionTicket( $ticket );
                }
 
-               if (
+               $fnameTrxOwner = get_class( $update ) . '::doUpdate';
+               $useExplicitTrxRound = !(
                        $update instanceof TransactionRoundAwareUpdate &&
                        $update->getTransactionRoundRequirement() == $update::TRX_ROUND_ABSENT
-               ) {
-                       $fnameTrxOwner = null;
+               );
+               // Flush any pending changes left over from an implicit transaction round
+               if ( $useExplicitTrxRound ) {
+                       $lbFactory->beginMasterChanges( $fnameTrxOwner ); // new explicit round
                } else {
-                       $fnameTrxOwner = get_class( $update ) . '::doUpdate';
+                       $lbFactory->commitMasterChanges( $fnameTrxOwner ); // new implicit round
                }
-
-               if ( $fnameTrxOwner !== null ) {
-                       $lbFactory->beginMasterChanges( $fnameTrxOwner );
-               }
-
+               // Run the update after any stale master view snapshots have been flushed
                $update->doUpdate();
-
-               if ( $fnameTrxOwner !== null ) {
-                       $lbFactory->commitMasterChanges( $fnameTrxOwner );
-               }
+               // Commit any pending changes from the explicit or implicit transaction round
+               $lbFactory->commitMasterChanges( $fnameTrxOwner );
        }
 
        /**