From a2790b1b80c5c686951a7167e14d267a2f044bb9 Mon Sep 17 00:00:00 2001 From: Aaron Schulz Date: Thu, 15 Sep 2016 14:40:00 -0700 Subject: [PATCH] Remove wfGetCaller() dependency from DatabaseBase Change-Id: I3e240b2eb5c1f6a21f1bc974c3d28f5755c7451a --- ...icationSecondaryAuthenticationProvider.php | 21 +- ...yPasswordPrimaryAuthenticationProvider.php | 20 +- includes/changes/RecentChange.php | 27 +-- includes/db/Database.php | 30 +-- includes/deferred/AtomicSectionUpdate.php | 2 +- includes/deferred/AutoCommitUpdate.php | 2 +- includes/deferred/LinksUpdate.php | 9 +- includes/deferred/MWCallableUpdate.php | 2 +- includes/filerepo/LocalRepo.php | 9 +- includes/filerepo/file/LocalFile.php | 24 ++- includes/jobqueue/JobQueueDB.php | 22 +- .../jobqueue/jobs/RecentChangesUpdateJob.php | 190 +++++++++--------- includes/jobqueue/utils/PurgeJobUtils.php | 67 +++--- includes/libs/rdbms/database/DBConnRef.php | 6 +- includes/libs/rdbms/database/IDatabase.php | 9 +- includes/page/WikiPage.php | 14 +- .../resourceloader/ResourceLoaderModule.php | 9 +- includes/revisiondelete/RevDelList.php | 11 +- includes/user/User.php | 12 +- includes/user/UserRightsProxy.php | 15 +- tests/phpunit/includes/db/DatabaseTest.php | 38 ++-- 21 files changed, 304 insertions(+), 235 deletions(-) diff --git a/includes/auth/EmailNotificationSecondaryAuthenticationProvider.php b/includes/auth/EmailNotificationSecondaryAuthenticationProvider.php index a82f018d48..a4855318b5 100644 --- a/includes/auth/EmailNotificationSecondaryAuthenticationProvider.php +++ b/includes/auth/EmailNotificationSecondaryAuthenticationProvider.php @@ -51,15 +51,18 @@ class EmailNotificationSecondaryAuthenticationProvider && !$this->manager->getAuthenticationSessionData( 'no-email' ) ) { // TODO show 'confirmemail_oncreate'/'confirmemail_sendfailed' message - wfGetDB( DB_MASTER )->onTransactionIdle( function () use ( $user ) { - $user = $user->getInstanceForUpdate(); - $status = $user->sendConfirmationMail(); - $user->saveSettings(); - if ( !$status->isGood() ) { - $this->logger->warning( 'Could not send confirmation email: ' . - $status->getWikiText( false, false, 'en' ) ); - } - } ); + wfGetDB( DB_MASTER )->onTransactionIdle( + function () use ( $user ) { + $user = $user->getInstanceForUpdate(); + $status = $user->sendConfirmationMail(); + $user->saveSettings(); + if ( !$status->isGood() ) { + $this->logger->warning( 'Could not send confirmation email: ' . + $status->getWikiText( false, false, 'en' ) ); + } + }, + __METHOD__ + ); } return AuthenticationResponse::newPass(); diff --git a/includes/auth/TemporaryPasswordPrimaryAuthenticationProvider.php b/includes/auth/TemporaryPasswordPrimaryAuthenticationProvider.php index f5571c7309..f16423dd2f 100644 --- a/includes/auth/TemporaryPasswordPrimaryAuthenticationProvider.php +++ b/includes/auth/TemporaryPasswordPrimaryAuthenticationProvider.php @@ -304,10 +304,13 @@ class TemporaryPasswordPrimaryAuthenticationProvider if ( $sendMail ) { // Send email after DB commit - $dbw->onTransactionIdle( function () use ( $req ) { - /** @var TemporaryPasswordAuthenticationRequest $req */ - $this->sendPasswordResetEmail( $req ); - } ); + $dbw->onTransactionIdle( + function () use ( $req ) { + /** @var TemporaryPasswordAuthenticationRequest $req */ + $this->sendPasswordResetEmail( $req ); + }, + __METHOD__ + ); } } @@ -375,9 +378,12 @@ class TemporaryPasswordPrimaryAuthenticationProvider if ( $mailpassword ) { // Send email after DB commit - wfGetDB( DB_MASTER )->onTransactionIdle( function () use ( $user, $creator, $req ) { - $this->sendNewAccountEmail( $user, $creator, $req->password ); - } ); + wfGetDB( DB_MASTER )->onTransactionIdle( + function () use ( $user, $creator, $req ) { + $this->sendNewAccountEmail( $user, $creator, $req->password ); + }, + __METHOD__ + ); } return $mailpassword ? 'byemail' : null; diff --git a/includes/changes/RecentChange.php b/includes/changes/RecentChange.php index 8e74674845..590fd3704f 100644 --- a/includes/changes/RecentChange.php +++ b/includes/changes/RecentChange.php @@ -342,18 +342,21 @@ class RecentChange { ) { // @FIXME: This would be better as an extension hook // Send emails or email jobs once this row is safely committed - $dbw->onTransactionIdle( function () use ( $editor, $title ) { - $enotif = new EmailNotification(); - $enotif->notifyOnPageChange( - $editor, - $title, - $this->mAttribs['rc_timestamp'], - $this->mAttribs['rc_comment'], - $this->mAttribs['rc_minor'], - $this->mAttribs['rc_last_oldid'], - $this->mExtra['pageStatus'] - ); - } ); + $dbw->onTransactionIdle( + function () use ( $editor, $title ) { + $enotif = new EmailNotification(); + $enotif->notifyOnPageChange( + $editor, + $title, + $this->mAttribs['rc_timestamp'], + $this->mAttribs['rc_comment'], + $this->mAttribs['rc_minor'], + $this->mAttribs['rc_last_oldid'], + $this->mExtra['pageStatus'] + ); + }, + __METHOD__ + ); } } diff --git a/includes/db/Database.php b/includes/db/Database.php index 3fa1335e2c..6cfff0e6d1 100644 --- a/includes/db/Database.php +++ b/includes/db/Database.php @@ -85,7 +85,7 @@ abstract class DatabaseBase implements IDatabase, LoggerAwareInterface { protected $mTrxPreCommitCallbacks = []; /** @var array[] List of (callable, method name) */ protected $mTrxEndCallbacks = []; - /** @var array[] Map of (name => (callable, method name)) */ + /** @var callable[] Map of (name => callable) */ protected $mTrxRecurringCallbacks = []; /** @var bool Whether to suppress triggering of transaction end callbacks */ protected $mTrxEndCallbacksSuppressed = false; @@ -2671,23 +2671,23 @@ abstract class DatabaseBase implements IDatabase, LoggerAwareInterface { return false; } - final public function onTransactionResolution( callable $callback ) { + final public function onTransactionResolution( callable $callback, $fname = __METHOD__ ) { if ( !$this->mTrxLevel ) { throw new DBUnexpectedError( $this, "No transaction is active." ); } - $this->mTrxEndCallbacks[] = [ $callback, wfGetCaller() ]; + $this->mTrxEndCallbacks[] = [ $callback, $fname ]; } - final public function onTransactionIdle( callable $callback ) { - $this->mTrxIdleCallbacks[] = [ $callback, wfGetCaller() ]; + final public function onTransactionIdle( callable $callback, $fname = __METHOD__ ) { + $this->mTrxIdleCallbacks[] = [ $callback, $fname ]; if ( !$this->mTrxLevel ) { $this->runOnTransactionIdleCallbacks( self::TRIGGER_IDLE ); } } - final public function onTransactionPreCommitOrIdle( callable $callback ) { + final public function onTransactionPreCommitOrIdle( callable $callback, $fname = __METHOD__ ) { if ( $this->mTrxLevel ) { - $this->mTrxPreCommitCallbacks[] = [ $callback, wfGetCaller() ]; + $this->mTrxPreCommitCallbacks[] = [ $callback, $fname ]; } else { // If no transaction is active, then make one for this callback $this->startAtomic( __METHOD__ ); @@ -2703,7 +2703,7 @@ abstract class DatabaseBase implements IDatabase, LoggerAwareInterface { final public function setTransactionListener( $name, callable $callback = null ) { if ( $callback ) { - $this->mTrxRecurringCallbacks[$name] = [ $callback, wfGetCaller() ]; + $this->mTrxRecurringCallbacks[$name] = $callback; } else { unset( $this->mTrxRecurringCallbacks[$name] ); } @@ -2818,9 +2818,8 @@ abstract class DatabaseBase implements IDatabase, LoggerAwareInterface { /** @var Exception $e */ $e = null; // first exception - foreach ( $this->mTrxRecurringCallbacks as $callback ) { + foreach ( $this->mTrxRecurringCallbacks as $phpCallback ) { try { - list( $phpCallback ) = $callback; $phpCallback( $trigger, $this ); } catch ( Exception $ex ) { call_user_func( $this->errorLogger, $ex ); @@ -3527,15 +3526,18 @@ abstract class DatabaseBase implements IDatabase, LoggerAwareInterface { // There is a good chance an exception was thrown, causing any early return // from the caller. Let any error handler get a chance to issue rollback(). // If there isn't one, let the error bubble up and trigger server-side rollback. - $this->onTransactionResolution( function () use ( $lockKey, $fname ) { - $this->unlock( $lockKey, $fname ); - } ); + $this->onTransactionResolution( + function () use ( $lockKey, $fname ) { + $this->unlock( $lockKey, $fname ); + }, + $fname + ); } else { $this->unlock( $lockKey, $fname ); } } ); - $this->commit( __METHOD__, self::FLUSHING_INTERNAL ); + $this->commit( $fname, self::FLUSHING_INTERNAL ); return $unlocker; } diff --git a/includes/deferred/AtomicSectionUpdate.php b/includes/deferred/AtomicSectionUpdate.php index a348719211..6585575dc3 100644 --- a/includes/deferred/AtomicSectionUpdate.php +++ b/includes/deferred/AtomicSectionUpdate.php @@ -24,7 +24,7 @@ class AtomicSectionUpdate implements DeferrableUpdate, DeferrableCallback { $this->callback = $callback; if ( $this->dbw->trxLevel() ) { - $this->dbw->onTransactionResolution( [ $this, 'cancelOnRollback' ] ); + $this->dbw->onTransactionResolution( [ $this, 'cancelOnRollback' ], $fname ); } } diff --git a/includes/deferred/AutoCommitUpdate.php b/includes/deferred/AutoCommitUpdate.php index d26cf9d1c6..d61dec2cb2 100644 --- a/includes/deferred/AutoCommitUpdate.php +++ b/includes/deferred/AutoCommitUpdate.php @@ -23,7 +23,7 @@ class AutoCommitUpdate implements DeferrableUpdate, DeferrableCallback { $this->callback = $callback; if ( $this->dbw->trxLevel() ) { - $this->dbw->onTransactionResolution( [ $this, 'cancelOnRollback' ] ); + $this->dbw->onTransactionResolution( [ $this, 'cancelOnRollback' ], $fname ); } } diff --git a/includes/deferred/LinksUpdate.php b/includes/deferred/LinksUpdate.php index e24a9c0737..d18349b1db 100644 --- a/includes/deferred/LinksUpdate.php +++ b/includes/deferred/LinksUpdate.php @@ -174,9 +174,12 @@ class LinksUpdate extends DataUpdate implements EnqueueableDataUpdate { // Commit and release the lock (if set) ScopedCallback::consume( $scopedLock ); // Run post-commit hooks without DBO_TRX - $this->getDB()->onTransactionIdle( function() { - Hooks::run( 'LinksUpdateComplete', [ &$this ] ); - } ); + $this->getDB()->onTransactionIdle( + function () { + Hooks::run( 'LinksUpdateComplete', [ &$this ] ); + }, + __METHOD__ + ); } /** diff --git a/includes/deferred/MWCallableUpdate.php b/includes/deferred/MWCallableUpdate.php index 47b162c270..5247e97cf1 100644 --- a/includes/deferred/MWCallableUpdate.php +++ b/includes/deferred/MWCallableUpdate.php @@ -19,7 +19,7 @@ class MWCallableUpdate implements DeferrableUpdate, DeferrableCallback { $this->fname = $fname; if ( $dbw && $dbw->trxLevel() ) { - $dbw->onTransactionResolution( [ $this, 'cancelOnRollback' ] ); + $dbw->onTransactionResolution( [ $this, 'cancelOnRollback' ], $fname ); } } diff --git a/includes/filerepo/LocalRepo.php b/includes/filerepo/LocalRepo.php index fccb755eef..7b40a7ba81 100644 --- a/includes/filerepo/LocalRepo.php +++ b/includes/filerepo/LocalRepo.php @@ -500,9 +500,12 @@ class LocalRepo extends FileRepo { function invalidateImageRedirect( Title $title ) { $key = $this->getSharedCacheKey( 'image_redirect', md5( $title->getDBkey() ) ); if ( $key ) { - $this->getMasterDB()->onTransactionPreCommitOrIdle( function() use ( $key ) { - ObjectCache::getMainWANInstance()->delete( $key ); - } ); + $this->getMasterDB()->onTransactionPreCommitOrIdle( + function () use ( $key ) { + ObjectCache::getMainWANInstance()->delete( $key ); + }, + __METHOD__ + ); } } diff --git a/includes/filerepo/file/LocalFile.php b/includes/filerepo/file/LocalFile.php index d63a91b6a2..618272c0c9 100644 --- a/includes/filerepo/file/LocalFile.php +++ b/includes/filerepo/file/LocalFile.php @@ -313,9 +313,12 @@ class LocalFile extends File { return; } - $this->repo->getMasterDB()->onTransactionPreCommitOrIdle( function() use ( $key ) { - ObjectCache::getMainWANInstance()->delete( $key ); - } ); + $this->repo->getMasterDB()->onTransactionPreCommitOrIdle( + function () use ( $key ) { + ObjectCache::getMainWANInstance()->delete( $key ); + }, + __METHOD__ + ); } /** @@ -2002,12 +2005,15 @@ class LocalFile extends File { } // Release the lock *after* commit to avoid row-level contention. // Make sure it triggers on rollback() as well as commit() (T132921). - $dbw->onTransactionResolution( function () use ( $logger ) { - $status = $this->releaseFileLock(); - if ( !$status->isGood() ) { - $logger->error( "Failed to unlock '{file}'", [ 'file' => $this->name ] ); - } - } ); + $dbw->onTransactionResolution( + function () use ( $logger ) { + $status = $this->releaseFileLock(); + if ( !$status->isGood() ) { + $logger->error( "Failed to unlock '{file}'", [ 'file' => $this->name ] ); + } + }, + __METHOD__ + ); // Callers might care if the SELECT snapshot is safely fresh $this->lockedOwnTrx = $makesTransaction; } diff --git a/includes/jobqueue/JobQueueDB.php b/includes/jobqueue/JobQueueDB.php index 50727a2cc9..856cdfd61e 100644 --- a/includes/jobqueue/JobQueueDB.php +++ b/includes/jobqueue/JobQueueDB.php @@ -186,7 +186,8 @@ class JobQueueDB extends JobQueue { $dbw->onTransactionIdle( function () use ( $dbw, $jobs, $flags, $method ) { $this->doBatchPushInternal( $dbw, $jobs, $flags, $method ); - } + }, + __METHOD__ ); } @@ -494,15 +495,18 @@ class JobQueueDB extends JobQueue { // jobs to become no-ops without any actual jobs that made them redundant. $dbw = $this->getMasterDB(); $cache = $this->dupCache; - $dbw->onTransactionIdle( function () use ( $cache, $params, $key, $dbw ) { - $timestamp = $cache->get( $key ); // current last timestamp of this job - if ( $timestamp && $timestamp >= $params['rootJobTimestamp'] ) { - return true; // a newer version of this root job was enqueued - } + $dbw->onTransactionIdle( + function () use ( $cache, $params, $key, $dbw ) { + $timestamp = $cache->get( $key ); // current last timestamp of this job + if ( $timestamp && $timestamp >= $params['rootJobTimestamp'] ) { + return true; // a newer version of this root job was enqueued + } - // Update the timestamp of the last root job started at the location... - return $cache->set( $key, $params['rootJobTimestamp'], JobQueueDB::ROOTJOB_TTL ); - } ); + // Update the timestamp of the last root job started at the location... + return $cache->set( $key, $params['rootJobTimestamp'], JobQueueDB::ROOTJOB_TTL ); + }, + __METHOD__ + ); return true; } diff --git a/includes/jobqueue/jobs/RecentChangesUpdateJob.php b/includes/jobqueue/jobs/RecentChangesUpdateJob.php index 809fb637f9..0e90674a2f 100644 --- a/includes/jobqueue/jobs/RecentChangesUpdateJob.php +++ b/includes/jobqueue/jobs/RecentChangesUpdateJob.php @@ -19,6 +19,7 @@ * @author Aaron Schulz * @ingroup JobQueue */ +use MediaWiki\MediaWikiServices; /** * Job for pruning recent changes @@ -81,7 +82,7 @@ class RecentChangesUpdateJob extends Job { return; // already in progress } - $factory = wfGetLBFactory(); + $factory = MediaWikiServices::getInstance()->getDBLoadBalancerFactory(); $ticket = $factory->getEmptyTransactionTicket( __METHOD__ ); $cutoff = $dbw->timestamp( time() - $wgRCMaxAge ); do { @@ -119,109 +120,112 @@ class RecentChangesUpdateJob extends Job { $dbw = wfGetDB( DB_MASTER ); // JobRunner uses DBO_TRX, but doesn't call begin/commit itself; // onTransactionIdle() will run immediately since there is no trx. - $dbw->onTransactionIdle( function() use ( $dbw, $days, $window ) { - $factory = wfGetLBFactory(); - $ticket = $factory->getEmptyTransactionTicket( __METHOD__ ); - // Avoid disconnect/ping() cycle that makes locks fall off - $dbw->setSessionOptions( [ 'connTimeout' => 900 ] ); - - $lockKey = wfWikiID() . '-activeusers'; - if ( !$dbw->lock( $lockKey, __METHOD__, 1 ) ) { - return; // exclusive update (avoids duplicate entries) - } - - $nowUnix = time(); - // Get the last-updated timestamp for the cache - $cTime = $dbw->selectField( 'querycache_info', - 'qci_timestamp', - [ 'qci_type' => 'activeusers' ] - ); - $cTimeUnix = $cTime ? wfTimestamp( TS_UNIX, $cTime ) : 1; - - // Pick the date range to fetch from. This is normally from the last - // update to till the present time, but has a limited window for sanity. - // If the window is limited, multiple runs are need to fully populate it. - $sTimestamp = max( $cTimeUnix, $nowUnix - $days * 86400 ); - $eTimestamp = min( $sTimestamp + $window, $nowUnix ); - - // Get all the users active since the last update - $res = $dbw->select( - [ 'recentchanges' ], - [ 'rc_user_text', 'lastedittime' => 'MAX(rc_timestamp)' ], - [ - 'rc_user > 0', // actual accounts - 'rc_type != ' . $dbw->addQuotes( RC_EXTERNAL ), // no wikidata - 'rc_log_type IS NULL OR rc_log_type != ' . $dbw->addQuotes( 'newusers' ), - 'rc_timestamp >= ' . $dbw->addQuotes( $dbw->timestamp( $sTimestamp ) ), - 'rc_timestamp <= ' . $dbw->addQuotes( $dbw->timestamp( $eTimestamp ) ) - ], - __METHOD__, - [ - 'GROUP BY' => [ 'rc_user_text' ], - 'ORDER BY' => 'NULL' // avoid filesort - ] - ); - $names = []; - foreach ( $res as $row ) { - $names[$row->rc_user_text] = $row->lastedittime; - } - - // Rotate out users that have not edited in too long (according to old data set) - $dbw->delete( 'querycachetwo', - [ - 'qcc_type' => 'activeusers', - 'qcc_value < ' . $dbw->addQuotes( $nowUnix - $days * 86400 ) // TS_UNIX - ], - __METHOD__ - ); + $dbw->onTransactionIdle( + function () use ( $dbw, $days, $window ) { + $factory = MediaWikiServices::getInstance()->getDBLoadBalancerFactory(); + $ticket = $factory->getEmptyTransactionTicket( __METHOD__ ); + // Avoid disconnect/ping() cycle that makes locks fall off + $dbw->setSessionOptions( [ 'connTimeout' => 900 ] ); + + $lockKey = wfWikiID() . '-activeusers'; + if ( !$dbw->lock( $lockKey, __METHOD__, 1 ) ) { + return; // exclusive update (avoids duplicate entries) + } - // Find which of the recently active users are already accounted for - if ( count( $names ) ) { - $res = $dbw->select( 'querycachetwo', - [ 'user_name' => 'qcc_title' ], + $nowUnix = time(); + // Get the last-updated timestamp for the cache + $cTime = $dbw->selectField( 'querycache_info', + 'qci_timestamp', + [ 'qci_type' => 'activeusers' ] + ); + $cTimeUnix = $cTime ? wfTimestamp( TS_UNIX, $cTime ) : 1; + + // Pick the date range to fetch from. This is normally from the last + // update to till the present time, but has a limited window for sanity. + // If the window is limited, multiple runs are need to fully populate it. + $sTimestamp = max( $cTimeUnix, $nowUnix - $days * 86400 ); + $eTimestamp = min( $sTimestamp + $window, $nowUnix ); + + // Get all the users active since the last update + $res = $dbw->select( + [ 'recentchanges' ], + [ 'rc_user_text', 'lastedittime' => 'MAX(rc_timestamp)' ], [ - 'qcc_type' => 'activeusers', - 'qcc_namespace' => NS_USER, - 'qcc_title' => array_keys( $names ) ], - __METHOD__ + 'rc_user > 0', // actual accounts + 'rc_type != ' . $dbw->addQuotes( RC_EXTERNAL ), // no wikidata + 'rc_log_type IS NULL OR rc_log_type != ' . $dbw->addQuotes( 'newusers' ), + 'rc_timestamp >= ' . $dbw->addQuotes( $dbw->timestamp( $sTimestamp ) ), + 'rc_timestamp <= ' . $dbw->addQuotes( $dbw->timestamp( $eTimestamp ) ) + ], + __METHOD__, + [ + 'GROUP BY' => [ 'rc_user_text' ], + 'ORDER BY' => 'NULL' // avoid filesort + ] ); + $names = []; foreach ( $res as $row ) { - unset( $names[$row->user_name] ); + $names[$row->rc_user_text] = $row->lastedittime; } - } - // Insert the users that need to be added to the list - if ( count( $names ) ) { - $newRows = []; - foreach ( $names as $name => $lastEditTime ) { - $newRows[] = [ + // Rotate out users that have not edited in too long (according to old data set) + $dbw->delete( 'querycachetwo', + [ 'qcc_type' => 'activeusers', - 'qcc_namespace' => NS_USER, - 'qcc_title' => $name, - 'qcc_value' => wfTimestamp( TS_UNIX, $lastEditTime ), - 'qcc_namespacetwo' => 0, // unused - 'qcc_titletwo' => '' // unused - ]; + 'qcc_value < ' . $dbw->addQuotes( $nowUnix - $days * 86400 ) // TS_UNIX + ], + __METHOD__ + ); + + // Find which of the recently active users are already accounted for + if ( count( $names ) ) { + $res = $dbw->select( 'querycachetwo', + [ 'user_name' => 'qcc_title' ], + [ + 'qcc_type' => 'activeusers', + 'qcc_namespace' => NS_USER, + 'qcc_title' => array_keys( $names ) ], + __METHOD__ + ); + foreach ( $res as $row ) { + unset( $names[$row->user_name] ); + } } - foreach ( array_chunk( $newRows, 500 ) as $rowBatch ) { - $dbw->insert( 'querycachetwo', $rowBatch, __METHOD__ ); - $factory->commitAndWaitForReplication( __METHOD__, $ticket ); + + // Insert the users that need to be added to the list + if ( count( $names ) ) { + $newRows = []; + foreach ( $names as $name => $lastEditTime ) { + $newRows[] = [ + 'qcc_type' => 'activeusers', + 'qcc_namespace' => NS_USER, + 'qcc_title' => $name, + 'qcc_value' => wfTimestamp( TS_UNIX, $lastEditTime ), + 'qcc_namespacetwo' => 0, // unused + 'qcc_titletwo' => '' // unused + ]; + } + foreach ( array_chunk( $newRows, 500 ) as $rowBatch ) { + $dbw->insert( 'querycachetwo', $rowBatch, __METHOD__ ); + $factory->commitAndWaitForReplication( __METHOD__, $ticket ); + } } - } - // If a transaction was already started, it might have an old - // snapshot, so kludge the timestamp range back as needed. - $asOfTimestamp = min( $eTimestamp, (int)$dbw->trxTimestamp() ); + // If a transaction was already started, it might have an old + // snapshot, so kludge the timestamp range back as needed. + $asOfTimestamp = min( $eTimestamp, (int)$dbw->trxTimestamp() ); - // Touch the data freshness timestamp - $dbw->replace( 'querycache_info', - [ 'qci_type' ], - [ 'qci_type' => 'activeusers', - 'qci_timestamp' => $dbw->timestamp( $asOfTimestamp ) ], // not always $now - __METHOD__ - ); + // Touch the data freshness timestamp + $dbw->replace( 'querycache_info', + [ 'qci_type' ], + [ 'qci_type' => 'activeusers', + 'qci_timestamp' => $dbw->timestamp( $asOfTimestamp ) ], // not always $now + __METHOD__ + ); - $dbw->unlock( $lockKey, __METHOD__ ); - } ); + $dbw->unlock( $lockKey, __METHOD__ ); + }, + __METHOD__ + ); } } diff --git a/includes/jobqueue/utils/PurgeJobUtils.php b/includes/jobqueue/utils/PurgeJobUtils.php index 5eafcb384d..d76d8661b4 100644 --- a/includes/jobqueue/utils/PurgeJobUtils.php +++ b/includes/jobqueue/utils/PurgeJobUtils.php @@ -36,42 +36,45 @@ class PurgeJobUtils { return; } - $dbw->onTransactionIdle( function() use ( $dbw, $namespace, $dbkeys ) { - $services = MediaWikiServices::getInstance(); - $lbFactory = $services->getDBLoadBalancerFactory(); - // Determine which pages need to be updated. - // This is necessary to prevent the job queue from smashing the DB with - // large numbers of concurrent invalidations of the same page. - $now = $dbw->timestamp(); - $ids = $dbw->selectFieldValues( - 'page', - 'page_id', - [ - 'page_namespace' => $namespace, - 'page_title' => $dbkeys, - 'page_touched < ' . $dbw->addQuotes( $now ) - ], - __METHOD__ - ); - - if ( !$ids ) { - return; - } - - $batchSize = $services->getMainConfig()->get( 'UpdateRowsPerQuery' ); - $ticket = $lbFactory->getEmptyTransactionTicket( __METHOD__ ); - foreach ( array_chunk( $ids, $batchSize ) as $idBatch ) { - $dbw->update( + $dbw->onTransactionIdle( + function () use ( $dbw, $namespace, $dbkeys ) { + $services = MediaWikiServices::getInstance(); + $lbFactory = $services->getDBLoadBalancerFactory(); + // Determine which pages need to be updated. + // This is necessary to prevent the job queue from smashing the DB with + // large numbers of concurrent invalidations of the same page. + $now = $dbw->timestamp(); + $ids = $dbw->selectFieldValues( 'page', - [ 'page_touched' => $now ], + 'page_id', [ - 'page_id' => $idBatch, - 'page_touched < ' . $dbw->addQuotes( $now ) // handle races + 'page_namespace' => $namespace, + 'page_title' => $dbkeys, + 'page_touched < ' . $dbw->addQuotes( $now ) ], __METHOD__ ); - $lbFactory->commitAndWaitForReplication( __METHOD__, $ticket ); - } - } ); + + if ( !$ids ) { + return; + } + + $batchSize = $services->getMainConfig()->get( 'UpdateRowsPerQuery' ); + $ticket = $lbFactory->getEmptyTransactionTicket( __METHOD__ ); + foreach ( array_chunk( $ids, $batchSize ) as $idBatch ) { + $dbw->update( + 'page', + [ 'page_touched' => $now ], + [ + 'page_id' => $idBatch, + 'page_touched < ' . $dbw->addQuotes( $now ) // handle races + ], + __METHOD__ + ); + $lbFactory->commitAndWaitForReplication( __METHOD__, $ticket ); + } + }, + __METHOD__ + ); } } diff --git a/includes/libs/rdbms/database/DBConnRef.php b/includes/libs/rdbms/database/DBConnRef.php index 3a18fc0b78..f9405801e2 100644 --- a/includes/libs/rdbms/database/DBConnRef.php +++ b/includes/libs/rdbms/database/DBConnRef.php @@ -444,15 +444,15 @@ class DBConnRef implements IDatabase { return $this->__call( __FUNCTION__, func_get_args() ); } - public function onTransactionResolution( callable $callback ) { + public function onTransactionResolution( callable $callback, $fname = __METHOD__ ) { return $this->__call( __FUNCTION__, func_get_args() ); } - public function onTransactionIdle( callable $callback ) { + public function onTransactionIdle( callable $callback, $fname = __METHOD__ ) { return $this->__call( __FUNCTION__, func_get_args() ); } - public function onTransactionPreCommitOrIdle( callable $callback ) { + public function onTransactionPreCommitOrIdle( callable $callback, $fname = __METHOD__ ) { return $this->__call( __FUNCTION__, func_get_args() ); } diff --git a/includes/libs/rdbms/database/IDatabase.php b/includes/libs/rdbms/database/IDatabase.php index 026cbc566c..9a0ffd5956 100644 --- a/includes/libs/rdbms/database/IDatabase.php +++ b/includes/libs/rdbms/database/IDatabase.php @@ -1284,10 +1284,11 @@ interface IDatabase { * - How the transaction ended (IDatabase::TRIGGER_COMMIT or IDatabase::TRIGGER_ROLLBACK) * * @param callable $callback + * @param string $fname Caller name * @return mixed * @since 1.28 */ - public function onTransactionResolution( callable $callback ); + public function onTransactionResolution( callable $callback, $fname = __METHOD__ ); /** * Run a callback as soon as there is no transaction pending. @@ -1306,9 +1307,10 @@ interface IDatabase { * - How the transaction ended (IDatabase::TRIGGER_COMMIT or IDatabase::TRIGGER_IDLE) * * @param callable $callback + * @param string $fname Caller name * @since 1.20 */ - public function onTransactionIdle( callable $callback ); + public function onTransactionIdle( callable $callback, $fname = __METHOD__ ); /** * Run a callback before the current transaction commits or now if there is none. @@ -1322,9 +1324,10 @@ interface IDatabase { * Updates will execute in the order they were enqueued. * * @param callable $callback + * @param string $fname Caller name * @since 1.22 */ - public function onTransactionPreCommitOrIdle( callable $callback ); + public function onTransactionPreCommitOrIdle( callable $callback, $fname = __METHOD__ ); /** * Run a callback each time any transaction commits or rolls back diff --git a/includes/page/WikiPage.php b/includes/page/WikiPage.php index 6c0c4a8c5a..faac26dc06 100644 --- a/includes/page/WikiPage.php +++ b/includes/page/WikiPage.php @@ -3031,10 +3031,13 @@ class WikiPage implements Page, IDBAccessObject { $logEntry->setComment( $reason ); $logid = $logEntry->insert(); - $dbw->onTransactionPreCommitOrIdle( function () use ( $dbw, $logEntry, $logid ) { - // Bug 56776: avoid deadlocks (especially from FileDeleteForm) - $logEntry->publish( $logid ); - } ); + $dbw->onTransactionPreCommitOrIdle( + function () use ( $dbw, $logEntry, $logid ) { + // Bug 56776: avoid deadlocks (especially from FileDeleteForm) + $logEntry->publish( $logid ); + }, + __METHOD__ + ); $dbw->endAtomic( __METHOD__ ); @@ -3672,7 +3675,8 @@ class WikiPage implements Page, IDBAccessObject { $cat->refreshCounts(); } } - } + }, + __METHOD__ ); } diff --git a/includes/resourceloader/ResourceLoaderModule.php b/includes/resourceloader/ResourceLoaderModule.php index 2351efdaea..3e94460529 100644 --- a/includes/resourceloader/ResourceLoaderModule.php +++ b/includes/resourceloader/ResourceLoaderModule.php @@ -487,9 +487,12 @@ abstract class ResourceLoaderModule implements LoggerAwareInterface { ); if ( $dbw->trxLevel() ) { - $dbw->onTransactionResolution( function () use ( &$scopeLock ) { - ScopedCallback::consume( $scopeLock ); // release after commit - } ); + $dbw->onTransactionResolution( + function () use ( &$scopeLock ) { + ScopedCallback::consume( $scopeLock ); // release after commit + }, + __METHOD__ + ); } } } catch ( Exception $e ) { diff --git a/includes/revisiondelete/RevDelList.php b/includes/revisiondelete/RevDelList.php index 48604e1a01..674846db05 100644 --- a/includes/revisiondelete/RevDelList.php +++ b/includes/revisiondelete/RevDelList.php @@ -120,10 +120,13 @@ abstract class RevDelList extends RevisionListBase { } $dbw->startAtomic( __METHOD__ ); - $dbw->onTransactionResolution( function () { - // Release locks on commit or error - $this->releaseItemLocks(); - } ); + $dbw->onTransactionResolution( + function () { + // Release locks on commit or error + $this->releaseItemLocks(); + }, + __METHOD__ + ); $missing = array_flip( $this->ids ); $this->clearFileOps(); diff --git a/includes/user/User.php b/includes/user/User.php index 2af0324134..0d06c7bac4 100644 --- a/includes/user/User.php +++ b/includes/user/User.php @@ -2358,7 +2358,8 @@ class User implements IDBAccessObject { wfGetDB( DB_MASTER )->onTransactionPreCommitOrIdle( function() use ( $cache, $key ) { $cache->delete( $key ); - } + }, + __METHOD__ ); } } @@ -4927,9 +4928,12 @@ class User implements IDBAccessObject { * Deferred version of incEditCountImmediate() */ public function incEditCount() { - wfGetDB( DB_MASTER )->onTransactionPreCommitOrIdle( function() { - $this->incEditCountImmediate(); - } ); + wfGetDB( DB_MASTER )->onTransactionPreCommitOrIdle( + function () { + $this->incEditCountImmediate(); + }, + __METHOD__ + ); } /** diff --git a/includes/user/UserRightsProxy.php b/includes/user/UserRightsProxy.php index 09244a46b8..69bc503b11 100644 --- a/includes/user/UserRightsProxy.php +++ b/includes/user/UserRightsProxy.php @@ -273,15 +273,20 @@ class UserRightsProxy { * Replaces User::touchUser() */ function invalidateCache() { - $this->db->update( 'user', + $this->db->update( + 'user', [ 'user_touched' => $this->db->timestamp() ], [ 'user_id' => $this->id ], - __METHOD__ ); + __METHOD__ + ); $wikiId = $this->db->getWikiID(); $userId = $this->id; - $this->db->onTransactionPreCommitOrIdle( function() use ( $wikiId, $userId ) { - User::purge( $wikiId, $userId ); - } ); + $this->db->onTransactionPreCommitOrIdle( + function () use ( $wikiId, $userId ) { + User::purge( $wikiId, $userId ); + }, + __METHOD__ + ); } } diff --git a/tests/phpunit/includes/db/DatabaseTest.php b/tests/phpunit/includes/db/DatabaseTest.php index e884640fbc..d4be6e4fa0 100644 --- a/tests/phpunit/includes/db/DatabaseTest.php +++ b/tests/phpunit/includes/db/DatabaseTest.php @@ -25,6 +25,7 @@ class DatabaseTest extends MediaWikiTestCase { } $this->db->restoreFlags( IDatabase::RESTORE_INITIAL ); } + /** * @covers DatabaseBase::dropTable */ @@ -231,7 +232,7 @@ class DatabaseTest extends MediaWikiTestCase { private function dropFunctions() { $this->db->query( 'DROP FUNCTION IF EXISTS mw_test_function' - . ( $this->db->getType() == 'postgres' ? '()' : '' ) + . ( $this->db->getType() == 'postgres' ? '()' : '' ) ); } @@ -247,26 +248,35 @@ class DatabaseTest extends MediaWikiTestCase { $db->setFlag( DBO_TRX ); $called = false; $flagSet = null; - $db->onTransactionIdle( function() use ( $db, &$flagSet, &$called ) { - $called = true; - $flagSet = $db->getFlag( DBO_TRX ); - } ); + $db->onTransactionIdle( + function () use ( $db, &$flagSet, &$called ) { + $called = true; + $flagSet = $db->getFlag( DBO_TRX ); + }, + __METHOD__ + ); $this->assertFalse( $flagSet, 'DBO_TRX off in callback' ); $this->assertTrue( $db->getFlag( DBO_TRX ), 'DBO_TRX restored to default' ); $this->assertTrue( $called, 'Callback reached' ); $db->clearFlag( DBO_TRX ); $flagSet = null; - $db->onTransactionIdle( function() use ( $db, &$flagSet ) { - $flagSet = $db->getFlag( DBO_TRX ); - } ); + $db->onTransactionIdle( + function () use ( $db, &$flagSet ) { + $flagSet = $db->getFlag( DBO_TRX ); + }, + __METHOD__ + ); $this->assertFalse( $flagSet, 'DBO_TRX off in callback' ); $this->assertFalse( $db->getFlag( DBO_TRX ), 'DBO_TRX restored to default' ); $db->clearFlag( DBO_TRX ); - $db->onTransactionIdle( function() use ( $db ) { - $db->setFlag( DBO_TRX ); - } ); + $db->onTransactionIdle( + function () use ( $db ) { + $db->setFlag( DBO_TRX ); + }, + __METHOD__ + ); $this->assertFalse( $db->getFlag( DBO_TRX ), 'DBO_TRX restored to default' ); } @@ -276,7 +286,7 @@ class DatabaseTest extends MediaWikiTestCase { $db->clearFlag( DBO_TRX ); $db->begin( __METHOD__ ); $called = false; - $db->onTransactionResolution( function() use ( $db, &$called ) { + $db->onTransactionResolution( function () use ( $db, &$called ) { $called = true; $db->setFlag( DBO_TRX ); } ); @@ -287,7 +297,7 @@ class DatabaseTest extends MediaWikiTestCase { $db->clearFlag( DBO_TRX ); $db->begin( __METHOD__ ); $called = false; - $db->onTransactionResolution( function() use ( $db, &$called ) { + $db->onTransactionResolution( function () use ( $db, &$called ) { $called = true; $db->setFlag( DBO_TRX ); } ); @@ -302,7 +312,7 @@ class DatabaseTest extends MediaWikiTestCase { public function testTransactionListener() { $db = $this->db; - $db->setTransactionListener( 'ping', function() use ( $db, &$called ) { + $db->setTransactionListener( 'ping', function () use ( $db, &$called ) { $called = true; } ); -- 2.20.1