Add AutoCommitUpdate class and replace some onTransactionIdle callers

* This puts the complex logic here after the commit step for
  all DBs, making the main multi-DB transaction more likely
  to be atomic.
* Made some cleanups to AtomicSectionUpdate and made it cancel
  if the transaction is rolled back as it should.
* Also cleaned up some closures for PHP 5.4.

Change-Id: If2f7bb6b1ba6daf1cfdc934f27c32b0b10431a3d

diff --git a/autoload.php b/autoload.php
index ecbc9b3..76a329f 100644 (file)
--- a/autoload.php
+++ b/autoload.php
@@ -157,6 +157,7 @@ $wgAutoloadLocalClasses = [
        'AuthManagerSpecialPage' => __DIR__ . '/includes/specialpage/AuthManagerSpecialPage.php',
        'AuthPlugin' => __DIR__ . '/includes/AuthPlugin.php',
        'AuthPluginUser' => __DIR__ . '/includes/AuthPlugin.php',
+       'AutoCommitUpdate' => __DIR__ . '/includes/deferred/AutoCommitUpdate.php',
        'AutoLoader' => __DIR__ . '/includes/AutoLoader.php',
        'AutoloadGenerator' => __DIR__ . '/includes/utils/AutoloadGenerator.php',
        'Autopromote' => __DIR__ . '/includes/Autopromote.php',

diff --git a/includes/Title.php b/includes/Title.php
index 62f4060..8aa8cb7 100644 (file)
--- a/includes/Title.php
+++ b/includes/Title.php
@@ -4367,18 +4367,23 @@ class Title implements LinkTarget {
                        return true; // avoid gap locking if we know it's not there
                }
 
-               $method = __METHOD__;
-               $dbw = wfGetDB( DB_MASTER );
                $conds = $this->pageCond();
-               $dbw->onTransactionIdle( function () use ( $dbw, $conds, $method, $purgeTime ) {
-                       $dbTimestamp = $dbw->timestamp( $purgeTime ?: time() );
-                       $dbw->update(
-                               'page',
-                               [ 'page_touched' => $dbTimestamp ],
-                               $conds + [ 'page_touched < ' . $dbw->addQuotes( $dbTimestamp ) ],
-                               $method
-                       );
-               } );
+               DeferredUpdates::addUpdate(
+                       new AutoCommitUpdate(
+                               wfGetDB( DB_MASTER ),
+                               __METHOD__,
+                               function ( IDatabase $dbw, $fname ) use ( $conds, $purgeTime ) {
+                                       $dbTimestamp = $dbw->timestamp( $purgeTime ?: time() );
+                                       $dbw->update(
+                                               'page',
+                                               [ 'page_touched' => $dbTimestamp ],
+                                               $conds + [ 'page_touched < ' . $dbw->addQuotes( $dbTimestamp ) ],
+                                               $fname
+                                       );
+                               }
+                       ),
+                       DeferredUpdates::PRESEND
+               );
 
                return true;
        }

diff --git a/includes/deferred/AtomicSectionUpdate.php b/includes/deferred/AtomicSectionUpdate.php
index a9921b3..ccbd6b0 100644 (file)
--- a/includes/deferred/AtomicSectionUpdate.php
+++ b/includes/deferred/AtomicSectionUpdate.php
@@ -9,26 +9,34 @@ class AtomicSectionUpdate implements DeferrableUpdate {
        private $dbw;
        /** @var string */
        private $fname;
-       /** @var Closure|callable */
+       /** @var callable */
        private $callback;
 
        /**
         * @param IDatabase $dbw
         * @param string $fname Caller name (usually __METHOD__)
         * @param callable $callback
-        * @throws InvalidArgumentException
         * @see IDatabase::doAtomicSection()
         */
-       public function __construct( IDatabase $dbw, $fname, $callback ) {
+       public function __construct( IDatabase $dbw, $fname, callable $callback ) {
                $this->dbw = $dbw;
                $this->fname = $fname;
-               if ( !is_callable( $callback ) ) {
-                       throw new InvalidArgumentException( 'Not a valid callback/closure!' );
-               }
                $this->callback = $callback;
+
+               if ( $this->dbw->trxLevel() ) {
+                       $this->dbw->onTransactionResolution( [ $this, 'cancelOnRollback' ] );
+               }
        }
 
        public function doUpdate() {
-               $this->dbw->doAtomicSection( $this->fname, $this->callback );
+               if ( $this->callback ) {
+                       $this->dbw->doAtomicSection( $this->fname, $this->callback );
+               }
+       }
+
+       public function cancelOnRollback( $trigger ) {
+               if ( $trigger === IDatabase::TRIGGER_ROLLBACK ) {
+                       $this->callback = null;
+               }
        }
 }

diff --git a/includes/deferred/AutoCommitUpdate.php b/includes/deferred/AutoCommitUpdate.php
new file mode 100644 (file)
index 0000000..ddf2bb8
--- /dev/null
+++ b/includes/deferred/AutoCommitUpdate.php
@@ -0,0 +1,56 @@
+<?php
+
+/**
+ * Deferrable Update for closure/callback updates that should use auto-commit mode
+ * @since 1.28
+ */
+class AutoCommitUpdate implements DeferrableUpdate {
+       /** @var IDatabase */
+       private $dbw;
+       /** @var string */
+       private $fname;
+       /** @var callable */
+       private $callback;
+
+       /**
+        * @param IDatabase $dbw
+        * @param string $fname Caller name (usually __METHOD__)
+        * @param callable $callback Callback that takes (IDatabase, method name string)
+        */
+       public function __construct( IDatabase $dbw, $fname, callable $callback ) {
+               $this->dbw = $dbw;
+               $this->fname = $fname;
+               $this->callback = $callback;
+
+               if ( $this->dbw->trxLevel() ) {
+                       $this->dbw->onTransactionResolution( [ $this, 'cancelOnRollback' ] );
+               }
+       }
+
+       public function doUpdate() {
+               if ( !$this->callback ) {
+                       return;
+               }
+
+               $autoTrx = $this->dbw->getFlag( DBO_TRX );
+               $this->dbw->clearFlag( DBO_TRX );
+               try {
+                       /** @var Exception $e */
+                       $e = null;
+                       call_user_func_array( $this->callback, [ $this->dbw, $this->fname ] );
+               } catch ( Exception $e ) {
+               }
+               if ( $autoTrx ) {
+                       $this->dbw->setFlag( DBO_TRX );
+               }
+               if ( $e ) {
+                       throw $e;
+               }
+       }
+
+       public function cancelOnRollback( $trigger ) {
+               if ( $trigger === IDatabase::TRIGGER_ROLLBACK ) {
+                       $this->callback = null;
+               }
+       }
+}

diff --git a/includes/filerepo/file/LocalFile.php b/includes/filerepo/file/LocalFile.php
index cab9316..234dbac 100644 (file)
--- a/includes/filerepo/file/LocalFile.php
+++ b/includes/filerepo/file/LocalFile.php
@@ -1637,16 +1637,20 @@ class LocalFile extends File {
                // Purge the source and target files...
                $oldTitleFile = wfLocalFile( $this->title );
                $newTitleFile = wfLocalFile( $target );
-               // Hack: the lock()/unlock() pair is nested in a transaction so the locking is not
-               // tied to BEGIN/COMMIT. To avoid slow purges in the transaction, move them outside.
-               $this->getRepo()->getMasterDB()->onTransactionIdle(
-                       function () use ( $oldTitleFile, $newTitleFile, $archiveNames ) {
-                               $oldTitleFile->purgeEverything();
-                               foreach ( $archiveNames as $archiveName ) {
-                                       $oldTitleFile->purgeOldThumbnails( $archiveName );
+               // To avoid slow purges in the transaction, move them outside...
+               DeferredUpdates::addUpdate(
+                       new AutoCommitUpdate(
+                               $this->getRepo()->getMasterDB(),
+                               __METHOD__,
+                               function () use ( $oldTitleFile, $newTitleFile, $archiveNames ) {
+                                       $oldTitleFile->purgeEverything();
+                                       foreach ( $archiveNames as $archiveName ) {
+                                               $oldTitleFile->purgeOldThumbnails( $archiveName );
+                                       }
+                                       $newTitleFile->purgeEverything();
                                }
-                               $newTitleFile->purgeEverything();
-                       }
+                       ),
+                       DeferredUpdates::PRESEND
                );
 
                if ( $status->isOK() ) {
@@ -1682,7 +1686,7 @@ class LocalFile extends File {
 
                $this->lock(); // begin
                $batch->addCurrent();
-               # Get old version relative paths
+               // Get old version relative paths
                $archiveNames = $batch->addOlds();
                $status = $batch->execute();
                $this->unlock(); // done
@@ -1691,16 +1695,19 @@ class LocalFile extends File {
                        DeferredUpdates::addUpdate( SiteStatsUpdate::factory( [ 'images' => -1 ] ) );
                }
 
-               // Hack: the lock()/unlock() pair is nested in a transaction so the locking is not
-               // tied to BEGIN/COMMIT. To avoid slow purges in the transaction, move them outside.
-               $that = $this;
-               $this->getRepo()->getMasterDB()->onTransactionIdle(
-                       function () use ( $that, $archiveNames ) {
-                               $that->purgeEverything();
-                               foreach ( $archiveNames as $archiveName ) {
-                                       $that->purgeOldThumbnails( $archiveName );
+               // To avoid slow purges in the transaction, move them outside...
+               DeferredUpdates::addUpdate(
+                       new AutoCommitUpdate(
+                               $this->getRepo()->getMasterDB(),
+                               __METHOD__,
+                               function () use ( $archiveNames ) {
+                                       $this->purgeEverything();
+                                       foreach ( $archiveNames as $archiveName ) {
+                                               $this->purgeOldThumbnails( $archiveName );
+                                       }
                                }
-                       }
+                       ),
+                       DeferredUpdates::PRESEND
                );
 
                // Purge the CDN