Convert onTransactionIdle() callers to DeferredUpdate subclasses
authorAaron Schulz <aschulz@wikimedia.org>
Thu, 21 Jul 2016 19:43:26 +0000 (12:43 -0700)
committerAaron Schulz <aschulz@wikimedia.org>
Mon, 25 Jul 2016 23:49:15 +0000 (16:49 -0700)
* This puts the complex logic here after the commit step for
  all DBs, making the main multi-DB transaction more likely
  to be atomic. Previously, the idle callbacks could be hit
  bewteen DB commits.
* Enforce transactionality via AtomicSectionUpdate.
* Use $this instead of $that hacks for old PHP versions.

Change-Id: Idf7d54fdac6487f86907099680f5c1c4f5530b4e

includes/MovePage.php
includes/filerepo/file/LocalFile.php
includes/page/WikiPage.php
includes/revisiondelete/RevDelList.php

index 708dea1..70b6738 100644 (file)
@@ -392,11 +392,16 @@ class MovePage {
                        $reason,
                        $nullRevision
                ];
-               $dbw->onTransactionIdle( function () use ( $params, $dbw ) {
-                       // Keep each single hook handler atomic
-                       $dbw->setFlag( DBO_TRX ); // flag is automatically reset by DB layer
-                       Hooks::run( 'TitleMoveComplete', $params );
-               } );
+               // Keep each single hook handler atomic
+               DeferredUpdates::addUpdate(
+                       new AtomicSectionUpdate(
+                               $dbw,
+                               __METHOD__,
+                               function () use ( $params ) {
+                                       Hooks::run( 'TitleMoveComplete', $params );
+                               }
+                       )
+               );
 
                return Status::newGood();
        }
index f91026f..c4d421c 100644 (file)
@@ -1422,97 +1422,103 @@ class LocalFile extends File {
                # Do some cache purges after final commit so that:
                # a) Changes are more likely to be seen post-purge
                # b) They won't cause rollback of the log publish/update above
-               $that = $this;
-               $dbw->onTransactionIdle( function () use (
-                       $that, $reupload, $wikiPage, $newPageContent, $comment, $user, $logEntry, $logId, $descId, $tags
-               ) {
-                       # Update memcache after the commit
-                       $that->invalidateCache();
-
-                       $updateLogPage = false;
-                       if ( $newPageContent ) {
-                               # New file page; create the description page.
-                               # There's already a log entry, so don't make a second RC entry
-                               # CDN and file cache for the description page are purged by doEditContent.
-                               $status = $wikiPage->doEditContent(
-                                       $newPageContent,
-                                       $comment,
-                                       EDIT_NEW | EDIT_SUPPRESS_RC,
-                                       false,
-                                       $user
-                               );
-
-                               if ( isset( $status->value['revision'] ) ) {
-                                       // Associate new page revision id
-                                       $logEntry->setAssociatedRevId( $status->value['revision']->getId() );
-                               }
-                               // This relies on the resetArticleID() call in WikiPage::insertOn(),
-                               // which is triggered on $descTitle by doEditContent() above.
-                               if ( isset( $status->value['revision'] ) ) {
-                                       /** @var $rev Revision */
-                                       $rev = $status->value['revision'];
-                                       $updateLogPage = $rev->getPage();
-                               }
-                       } else {
-                               # Existing file page: invalidate description page cache
-                               $wikiPage->getTitle()->invalidateCache();
-                               $wikiPage->getTitle()->purgeSquid();
-                               # Allow the new file version to be patrolled from the page footer
-                               Article::purgePatrolFooterCache( $descId );
-                       }
-
-                       # Update associated rev id. This should be done by $logEntry->insert() earlier,
-                       # but setAssociatedRevId() wasn't called at that point yet...
-                       $logParams = $logEntry->getParameters();
-                       $logParams['associated_rev_id'] = $logEntry->getAssociatedRevId();
-                       $update = [ 'log_params' => LogEntryBase::makeParamBlob( $logParams ) ];
-                       if ( $updateLogPage ) {
-                               # Also log page, in case where we just created it above
-                               $update['log_page'] = $updateLogPage;
-                       }
-                       $that->getRepo()->getMasterDB()->update(
-                               'logging',
-                               $update,
-                               [ 'log_id' => $logId ],
-                               __METHOD__
-                       );
-                       $that->getRepo()->getMasterDB()->insert(
-                               'log_search',
-                               [
-                                       'ls_field' => 'associated_rev_id',
-                                       'ls_value' => $logEntry->getAssociatedRevId(),
-                                       'ls_log_id' => $logId,
-                               ],
-                               __METHOD__
-                       );
+               DeferredUpdates::addUpdate(
+                       new AutoCommitUpdate(
+                               $dbw,
+                               __METHOD__,
+                               function () use (
+                                       $reupload, $wikiPage, $newPageContent, $comment, $user,
+                                       $logEntry, $logId, $descId, $tags
+                               ) {
+                                       # Update memcache after the commit
+                                       $this->invalidateCache();
+
+                                       $updateLogPage = false;
+                                       if ( $newPageContent ) {
+                                               # New file page; create the description page.
+                                               # There's already a log entry, so don't make a second RC entry
+                                               # CDN and file cache for the description page are purged by doEditContent.
+                                               $status = $wikiPage->doEditContent(
+                                                       $newPageContent,
+                                                       $comment,
+                                                       EDIT_NEW | EDIT_SUPPRESS_RC,
+                                                       false,
+                                                       $user
+                                               );
+
+                                               if ( isset( $status->value['revision'] ) ) {
+                                                       // Associate new page revision id
+                                                       $logEntry->setAssociatedRevId( $status->value['revision']->getId() );
+                                               }
+                                               // This relies on the resetArticleID() call in WikiPage::insertOn(),
+                                               // which is triggered on $descTitle by doEditContent() above.
+                                               if ( isset( $status->value['revision'] ) ) {
+                                                       /** @var $rev Revision */
+                                                       $rev = $status->value['revision'];
+                                                       $updateLogPage = $rev->getPage();
+                                               }
+                                       } else {
+                                               # Existing file page: invalidate description page cache
+                                               $wikiPage->getTitle()->invalidateCache();
+                                               $wikiPage->getTitle()->purgeSquid();
+                                               # Allow the new file version to be patrolled from the page footer
+                                               Article::purgePatrolFooterCache( $descId );
+                                       }
 
-                       # Add change tags, if any
-                       if ( $tags ) {
-                               $logEntry->setTags( $tags );
-                       }
+                                       # Update associated rev id. This should be done by $logEntry->insert() earlier,
+                                       # but setAssociatedRevId() wasn't called at that point yet...
+                                       $logParams = $logEntry->getParameters();
+                                       $logParams['associated_rev_id'] = $logEntry->getAssociatedRevId();
+                                       $update = [ 'log_params' => LogEntryBase::makeParamBlob( $logParams ) ];
+                                       if ( $updateLogPage ) {
+                                               # Also log page, in case where we just created it above
+                                               $update['log_page'] = $updateLogPage;
+                                       }
+                                       $this->getRepo()->getMasterDB()->update(
+                                               'logging',
+                                               $update,
+                                               [ 'log_id' => $logId ],
+                                               __METHOD__
+                                       );
+                                       $this->getRepo()->getMasterDB()->insert(
+                                               'log_search',
+                                               [
+                                                       'ls_field' => 'associated_rev_id',
+                                                       'ls_value' => $logEntry->getAssociatedRevId(),
+                                                       'ls_log_id' => $logId,
+                                               ],
+                                               __METHOD__
+                                       );
+
+                                       # Add change tags, if any
+                                       if ( $tags ) {
+                                               $logEntry->setTags( $tags );
+                                       }
 
-                       # Uploads can be patrolled
-                       $logEntry->setIsPatrollable( true );
+                                       # Uploads can be patrolled
+                                       $logEntry->setIsPatrollable( true );
 
-                       # Now that the log entry is up-to-date, make an RC entry.
-                       $logEntry->publish( $logId );
+                                       # Now that the log entry is up-to-date, make an RC entry.
+                                       $logEntry->publish( $logId );
 
-                       # Run hook for other updates (typically more cache purging)
-                       Hooks::run( 'FileUpload', [ $that, $reupload, !$newPageContent ] );
+                                       # Run hook for other updates (typically more cache purging)
+                                       Hooks::run( 'FileUpload', [ $this, $reupload, !$newPageContent ] );
 
-                       if ( $reupload ) {
-                               # Delete old thumbnails
-                               $that->purgeThumbnails();
-                               # Remove the old file from the CDN cache
-                               DeferredUpdates::addUpdate(
-                                       new CdnCacheUpdate( [ $that->getUrl() ] ),
-                                       DeferredUpdates::PRESEND
-                               );
-                       } else {
-                               # Update backlink pages pointing to this title if created
-                               LinksUpdate::queueRecursiveJobsForTable( $that->getTitle(), 'imagelinks' );
-                       }
-               } );
+                                       if ( $reupload ) {
+                                               # Delete old thumbnails
+                                               $this->purgeThumbnails();
+                                               # Remove the old file from the CDN cache
+                                               DeferredUpdates::addUpdate(
+                                                       new CdnCacheUpdate( [ $this->getUrl() ] ),
+                                                       DeferredUpdates::PRESEND
+                                               );
+                                       } else {
+                                               # Update backlink pages pointing to this title if created
+                                               LinksUpdate::queueRecursiveJobsForTable( $this->getTitle(), 'imagelinks' );
+                                       }
+                               }
+                       )
+               );
 
                if ( !$reupload ) {
                        # This is a new file, so update the image count
index dbc27a9..e7352af 100644 (file)
@@ -1810,30 +1810,31 @@ class WikiPage implements Page, IDBAccessObject {
                }
 
                // Do secondary updates once the main changes have been committed...
-               $that = $this;
-               $dbw->onTransactionIdle(
-                       function () use (
-                               $dbw, &$that, $revision, &$user, $content, $summary, &$flags,
-                               $changed, $meta, &$status
-                       ) {
-                               // Do per-page updates in a transaction
-                               $dbw->setFlag( DBO_TRX );
-                               // Update links tables, site stats, etc.
-                               $that->doEditUpdates(
-                                       $revision,
-                                       $user,
-                                       [
-                                               'changed' => $changed,
-                                               'oldcountable' => $meta['oldCountable'],
-                                               'oldrevision' => $meta['oldRevision']
-                                       ]
-                               );
-                               // Trigger post-save hook
-                               $params = [ &$that, &$user, $content, $summary, $flags & EDIT_MINOR,
-                                       null, null, &$flags, $revision, &$status, $meta['baseRevId'] ];
-                               ContentHandler::runLegacyHooks( 'ArticleSaveComplete', $params );
-                               Hooks::run( 'PageContentSaveComplete', $params );
-                       }
+               DeferredUpdates::addUpdate(
+                       new AtomicSectionUpdate(
+                               $dbw,
+                               __METHOD__,
+                               function () use (
+                                       $revision, &$user, $content, $summary, &$flags,
+                                       $changed, $meta, &$status
+                               ) {
+                                       // Update links tables, site stats, etc.
+                                       $this->doEditUpdates(
+                                               $revision,
+                                               $user,
+                                               [
+                                                       'changed' => $changed,
+                                                       'oldcountable' => $meta['oldCountable'],
+                                                       'oldrevision' => $meta['oldRevision']
+                                               ]
+                                       );
+                                       // Trigger post-save hook
+                                       $params = [ &$this, &$user, $content, $summary, $flags & EDIT_MINOR,
+                                               null, null, &$flags, $revision, &$status, $meta['baseRevId'] ];
+                                       ContentHandler::runLegacyHooks( 'ArticleSaveComplete', $params );
+                                       Hooks::run( 'PageContentSaveComplete', $params );
+                               }
+                       )
                );
 
                return $status;
@@ -1938,26 +1939,27 @@ class WikiPage implements Page, IDBAccessObject {
                $status->value['revision'] = $revision;
 
                // Do secondary updates once the main changes have been committed...
-               $that = $this;
-               $dbw->onTransactionIdle(
-                       function () use (
-                               &$that, $dbw, $revision, &$user, $content, $summary, &$flags, $meta, &$status
-                       ) {
-                               // Do per-page updates in a transaction
-                               $dbw->setFlag( DBO_TRX );
-                               // Update links, etc.
-                               $that->doEditUpdates( $revision, $user, [ 'created' => true ] );
-                               // Trigger post-create hook
-                               $params = [ &$that, &$user, $content, $summary,
-                                       $flags & EDIT_MINOR, null, null, &$flags, $revision ];
-                               ContentHandler::runLegacyHooks( 'ArticleInsertComplete', $params );
-                               Hooks::run( 'PageContentInsertComplete', $params );
-                               // Trigger post-save hook
-                               $params = array_merge( $params, [ &$status, $meta['baseRevId'] ] );
-                               ContentHandler::runLegacyHooks( 'ArticleSaveComplete', $params );
-                               Hooks::run( 'PageContentSaveComplete', $params );
+               DeferredUpdates::addUpdate(
+                       new AtomicSectionUpdate(
+                               $dbw,
+                               __METHOD__,
+                               function () use (
+                                       $revision, &$user, $content, $summary, &$flags, $meta, &$status
+                               ) {
+                                       // Update links, etc.
+                                       $this->doEditUpdates( $revision, $user, [ 'created' => true ] );
+                                       // Trigger post-create hook
+                                       $params = [ &$this, &$user, $content, $summary,
+                                               $flags & EDIT_MINOR, null, null, &$flags, $revision ];
+                                       ContentHandler::runLegacyHooks( 'ArticleInsertComplete', $params );
+                                       Hooks::run( 'PageContentInsertComplete', $params );
+                                       // Trigger post-save hook
+                                       $params = array_merge( $params, [ &$status, $meta['baseRevId'] ] );
+                                       ContentHandler::runLegacyHooks( 'ArticleSaveComplete', $params );
+                                       Hooks::run( 'PageContentSaveComplete', $params );
 
-                       }
+                               }
+                       )
                );
 
                return $status;
index 3c81feb..fc04c69 100644 (file)
@@ -264,11 +264,13 @@ abstract class RevDelList extends RevisionListBase {
                        ]
                );
 
-               // Clear caches
-               $that = $this;
-               $dbw->onTransactionIdle( function() use ( $that, $visibilityChangeMap ) {
-                       $that->doPostCommitUpdates( $visibilityChangeMap );
-               } );
+               // Clear caches after commit
+               DeferredUpdates::addCallableUpdate(
+                       function () use ( $visibilityChangeMap ) {
+                               $this->doPostCommitUpdates( $visibilityChangeMap );
+                       },
+                       DeferredUpdates::PRESEND
+               );
 
                $dbw->endAtomic( __METHOD__ );