ILoadBalancer/ILBFactory: Rename commitMasterChanges() to commitPrimaryChanges()

Bug: T282894
Change-Id: I0d80be56e683924254c4e38d05e1109ea74eeab5
This commit is contained in:
James D. Forrester 2021-05-14 15:22:15 -07:00
parent 577e1c3710
commit 5ad7ca7eba
18 changed files with 76 additions and 47 deletions

View file

@ -461,6 +461,8 @@ because of Phabricator reports.
- ILoadBalancer::approveMasterChanges() -> ::approvePrimaryChanges()
- ILoadBalancer::beginMasterChanges() -> ::beginPrimaryChanges()
- ILBFactory::beginMasterChanges() -> ::beginPrimaryChanges()
- ILoadBalancer::commitMasterChanges() -> ::commitPrimaryChanges()
- ILBFactory::commitMasterChanges() -> ::commitPrimaryChanges()
* wfIncrStats(), deprecated in 1.36, now emits deprecation warnings.
* wfCanIPUseHTTPS() is now deprecated, and always returns true.
* The UserLoadFromDatabase hook has been deprecated. It had no known uses.

View file

@ -145,7 +145,7 @@ class AjaxDispatcher {
// Make sure DB commit succeeds before sending a response
$lbFactory = MediaWikiServices::getInstance()->getDBLoadBalancerFactory();
$lbFactory->commitMasterChanges( __METHOD__ );
$lbFactory->commitPrimaryChanges( __METHOD__ );
$result->sendHeaders();
$result->printText();

View file

@ -678,7 +678,7 @@ class MediaWiki {
ignore_user_abort( true );
// Commit all RDBMs changes from the main transaction round
$lbFactory->commitMasterChanges(
$lbFactory->commitPrimaryChanges(
__METHOD__,
// Abort if any transaction was too big
[ 'maxWriteDuration' => $config->get( 'MaxUserDBWriteDuration' ) ]
@ -1121,7 +1121,7 @@ class MediaWiki {
$lbFactory = MediaWikiServices::getInstance()->getDBLoadBalancerFactory();
// Assure deferred updates are not in the main transaction
$lbFactory->commitMasterChanges( __METHOD__ );
$lbFactory->commitPrimaryChanges( __METHOD__ );
// Loosen DB query expectations since the HTTP client is unblocked
$trxProfiler = Profiler::instance()->getTransactionProfiler();
@ -1139,7 +1139,7 @@ class MediaWiki {
wfLogProfilingData();
// Commit and close up!
$lbFactory->commitMasterChanges( __METHOD__ );
$lbFactory->commitPrimaryChanges( __METHOD__ );
$lbFactory->shutdown( $lbFactory::SHUTDOWN_NO_CHRONPROT );
wfDebug( "Request ended normally" );

View file

@ -509,12 +509,12 @@ class DeferredUpdates {
if ( $useExplicitTrxRound ) {
$lbFactory->beginPrimaryChanges( $fnameTrxOwner ); // new explicit round
} else {
$lbFactory->commitMasterChanges( $fnameTrxOwner ); // new implicit round
$lbFactory->commitPrimaryChanges( $fnameTrxOwner ); // new implicit round
}
// Run the update after any stale master view snapshots have been flushed
$update->doUpdate();
// Commit any pending changes from the explicit or implicit transaction round
$lbFactory->commitMasterChanges( $fnameTrxOwner );
$lbFactory->commitPrimaryChanges( $fnameTrxOwner );
}
/**

View file

@ -93,7 +93,7 @@ class RefreshSecondaryDataUpdate extends DataUpdate
// T221577, T248003: flush any transaction; each update needs outer transaction scope
// and the code above may have implicitly started one.
$this->lbFactory->commitMasterChanges( __METHOD__ );
$this->lbFactory->commitPrimaryChanges( __METHOD__ );
$e = null;
foreach ( $updates as $update ) {

View file

@ -364,7 +364,7 @@ class JobRunner implements LoggerAwareInterface {
$fnameTrxOwner = get_class( $job ) . '::run'; // give run() outer scope
// Flush any pending changes left over from an implicit transaction round
if ( $job->hasExecutionFlag( $job::JOB_NO_EXPLICIT_TRX_ROUND ) ) {
$this->lbFactory->commitMasterChanges( $fnameTrxOwner ); // new implicit round
$this->lbFactory->commitPrimaryChanges( $fnameTrxOwner ); // new implicit round
} else {
$this->lbFactory->beginPrimaryChanges( $fnameTrxOwner ); // new explicit round
}
@ -373,7 +373,7 @@ class JobRunner implements LoggerAwareInterface {
$status = $job->run();
$error = $job->getLastError();
// Commit all pending changes from this job
$this->commitMasterChanges( $job, $fnameTrxOwner );
$this->commitPrimaryChanges( $job, $fnameTrxOwner );
// Run any deferred update tasks; doUpdates() manages transactions itself
DeferredUpdates::doUpdates();
} catch ( Throwable $e ) {
@ -623,7 +623,7 @@ class JobRunner implements LoggerAwareInterface {
* @param string $fnameTrxOwner
* @throws DBError
*/
private function commitMasterChanges( RunnableJob $job, $fnameTrxOwner ) {
private function commitPrimaryChanges( RunnableJob $job, $fnameTrxOwner ) {
$syncThreshold = $this->options->get( 'JobSerialCommitThreshold' );
$time = false;
@ -646,7 +646,7 @@ class JobRunner implements LoggerAwareInterface {
}
if ( !$dbwSerial ) {
$this->lbFactory->commitMasterChanges(
$this->lbFactory->commitPrimaryChanges(
$fnameTrxOwner,
// Abort if any transaction was too big
[ 'maxWriteDuration' => $this->options->get( 'MaxJobDBWriteDuration' ) ]
@ -682,7 +682,7 @@ class JobRunner implements LoggerAwareInterface {
}
// Actually commit the DB primary changes
$this->lbFactory->commitMasterChanges(
$this->lbFactory->commitPrimaryChanges(
$fnameTrxOwner,
// Abort if any transaction was too big
[ 'maxWriteDuration' => $this->options->get( 'MaxJobDBWriteDuration' ) ]

View file

@ -89,7 +89,7 @@ class ClearUserWatchlistJob extends Job implements GenericParameterJob {
// Commit changes and remove lock before inserting next job.
$lbf = MediaWikiServices::getInstance()->getDBLoadBalancerFactory();
$lbf->commitMasterChanges( __METHOD__ );
$lbf->commitPrimaryChanges( __METHOD__ );
unset( $scopedLock );
if ( count( $watchlistIds ) === (int)$batchSize ) {

View file

@ -197,7 +197,7 @@ class RefreshLinksJob extends Job {
$lbFactory->beginPrimaryChanges( __METHOD__ );
$output = $this->getParserOutput( $renderer, $parserCache, $page, $stats );
$options = $this->getDataUpdateOptions();
$lbFactory->commitMasterChanges( __METHOD__ );
$lbFactory->commitPrimaryChanges( __METHOD__ );
if ( !$output ) {
return false; // raced out?

View file

@ -234,7 +234,7 @@ interface ILBFactory {
* Flush any master transaction snapshots and set DBO_TRX (if DBO_DEFAULT is set)
*
* The DBO_TRX setting will be reverted to the default in each of these methods:
* - commitMasterChanges()
* - commitPrimaryChanges()
* - rollbackMasterChanges()
* - commitAll()
*
@ -264,6 +264,15 @@ interface ILBFactory {
* - maxWriteDuration: abort if more than this much time was spent in write queries
* @throws DBTransactionError
*/
public function commitPrimaryChanges( $fname = __METHOD__, array $options = [] );
/**
* @deprecated since 1.37; please use commitPrimaryChanges() instead.
* @param string $fname Caller name
* @param array $options Options map:
* - maxWriteDuration: abort if more than this much time was spent in write queries
* @throws DBTransactionError
*/
public function commitMasterChanges( $fname = __METHOD__, array $options = [] );
/**
@ -367,12 +376,12 @@ interface ILBFactory {
public function getEmptyTransactionTicket( $fname );
/**
* Call commitMasterChanges() and waitForReplication() if $ticket indicates it is safe
* Call commitPrimaryChanges() and waitForReplication() if $ticket indicates it is safe
*
* The ticket is used to check that the caller owns the transaction round or can act on
* behalf of the caller that owns the transaction round.
*
* @see commitMasterChanges()
* @see commitPrimaryChanges()
* @see waitForReplication()
*
* @param string $fname Caller name (e.g. __METHOD__)

View file

@ -232,7 +232,7 @@ abstract class LBFactory implements ILBFactory {
}
$cpClientId = $chronProt->getClientId();
$this->commitMasterChanges( __METHOD__ ); // sanity
$this->commitPrimaryChanges( __METHOD__ ); // sanity
$this->replLogger->debug( 'LBFactory shutdown completed' );
}
@ -263,7 +263,7 @@ abstract class LBFactory implements ILBFactory {
}
final public function commitAll( $fname = __METHOD__, array $options = [] ) {
$this->commitMasterChanges( $fname, $options );
$this->commitPrimaryChanges( $fname, $options );
$this->forEachLBCallMethod( 'flushMasterSnapshots', [ $fname, $this->id ] );
$this->forEachLBCallMethod( 'flushReplicaSnapshots', [ $fname, $this->id ] );
}
@ -291,7 +291,7 @@ abstract class LBFactory implements ILBFactory {
$this->beginPrimaryChanges( $fname );
}
final public function commitMasterChanges( $fname = __METHOD__, array $options = [] ) {
final public function commitPrimaryChanges( $fname = __METHOD__, array $options = [] ) {
$this->assertTransactionRoundStage( self::ROUND_CURSORY );
/** @noinspection PhpUnusedLocalVariableInspection */
$scope = ScopedCallback::newScopedIgnoreUserAbort();
@ -316,7 +316,7 @@ abstract class LBFactory implements ILBFactory {
// Log the DBs and methods involved in multi-DB transactions
$this->logIfMultiDbTransaction();
// Actually perform the commit on all primary DB connections and revert DBO_TRX
$this->forEachLBCallMethod( 'commitMasterChanges', [ $fname, $this->id ] );
$this->forEachLBCallMethod( 'commitPrimaryChanges', [ $fname, $this->id ] );
// Run all post-commit callbacks in a separate step
$this->trxRoundStage = self::ROUND_COMMIT_CALLBACKS;
$e = $this->executePostTransactionCallbacks();
@ -327,6 +327,11 @@ abstract class LBFactory implements ILBFactory {
}
}
final public function commitMasterChanges( $fname = __METHOD__, array $options = [] ) {
// wfDeprecated( __METHOD__, '1.37' );
$this->commitPrimaryChanges( $fname, $options );
}
final public function rollbackMasterChanges( $fname = __METHOD__ ) {
/** @noinspection PhpUnusedLocalVariableInspection */
$scope = ScopedCallback::newScopedIgnoreUserAbort();
@ -531,7 +536,7 @@ abstract class LBFactory implements ILBFactory {
$fnameEffective = $fname;
}
$this->commitMasterChanges( $fnameEffective );
$this->commitPrimaryChanges( $fnameEffective );
$waitSucceeded = $this->waitForReplication( $opts );
// If a nested caller committed on behalf of $fname, start another empty $fname
// transaction, leaving the caller with the same empty transaction state as before.

View file

@ -41,10 +41,10 @@ use LogicException;
* - In CLI mode, the flag has no effect with regards to LoadBalancer.
* - In non-CLI mode, the flag causes implicit transactions to be used; the first query on
* a database starts a transaction on that database. The transactions are meant to remain
* pending until either commitMasterChanges() or rollbackMasterChanges() is called. The
* application must have some point where it calls commitMasterChanges() near the end of
* pending until either commitPrimaryChanges() or rollbackMasterChanges() is called. The
* application must have some point where it calls commitPrimaryChanges() near the end of
* the PHP request.
* Every iteration of beginPrimaryChanges()/commitMasterChanges() is called a "transaction round".
* Every iteration of beginPrimaryChanges()/commitPrimaryChanges() is called a "transaction round".
* Rounds are useful on the primary DB connections because they make single-DB (and by and large
* multi-DB) updates in web requests all-or-nothing. Also, transactions on replica DBs are useful
* when REPEATABLE-READ or SERIALIZABLE isolation is used because all foriegn keys and constraints
@ -626,7 +626,7 @@ interface ILoadBalancer {
* Flush any primary transaction snapshots and set DBO_TRX (if DBO_DEFAULT is set)
*
* The DBO_TRX setting will be reverted to the default in each of these methods:
* - commitMasterChanges()
* - commitPrimaryChanges()
* - rollbackMasterChanges()
* - commitAll()
* This allows for custom transaction rounds from any outer transaction scope.
@ -651,6 +651,14 @@ interface ILoadBalancer {
* @param int|null $owner ID of the calling instance (e.g. the LBFactory ID)
* @throws DBExpectedError
*/
public function commitPrimaryChanges( $fname = __METHOD__, $owner = null );
/**
* @deprecated since 1.37; please use commitPrimaryChanges() instead.
* @param string $fname Caller name
* @param int|null $owner ID of the calling instance (e.g. the LBFactory ID)
* @throws DBExpectedError
*/
public function commitMasterChanges( $fname = __METHOD__, $owner = null );
/**

View file

@ -1701,7 +1701,7 @@ class LoadBalancer implements ILoadBalancer {
}
public function commitAll( $fname = __METHOD__, $owner = null ) {
$this->commitMasterChanges( $fname, $owner );
$this->commitPrimaryChanges( $fname, $owner );
$this->flushMasterSnapshots( $fname, $owner );
$this->flushReplicaSnapshots( $fname, $owner );
}
@ -1822,7 +1822,7 @@ class LoadBalancer implements ILoadBalancer {
$this->beginPrimaryChanges( $fname, $owner );
}
public function commitMasterChanges( $fname = __METHOD__, $owner = null ) {
public function commitPrimaryChanges( $fname = __METHOD__, $owner = null ) {
$this->assertOwnership( $fname, $owner );
$this->assertTransactionRoundStage( self::ROUND_APPROVED );
if ( $this->ownerId === null ) {
@ -1862,6 +1862,11 @@ class LoadBalancer implements ILoadBalancer {
$this->trxRoundStage = self::ROUND_COMMIT_CALLBACKS;
}
public function commitMasterChanges( $fname = __METHOD__, $owner = null ) {
// wfDeprecated( __METHOD__, '1.37' );
$this->commitPrimaryChanges( $fname, $owner );
}
public function runMasterTransactionIdleCallbacks( $fname = __METHOD__, $owner = null ) {
$this->assertOwnership( $fname, $owner );
if ( $this->trxRoundStage === self::ROUND_COMMIT_CALLBACKS ) {

View file

@ -1239,7 +1239,7 @@ abstract class Maintenance {
!MediaWikiServices::getInstance()->isServiceDisabled( 'DBLoadBalancerFactory' )
) {
$lbFactory = MediaWikiServices::getInstance()->getDBLoadBalancerFactory();
$lbFactory->commitMasterChanges( get_class( $this ) );
$lbFactory->commitPrimaryChanges( get_class( $this ) );
// @TODO: make less assumptions about deferred updates being coupled to the DB
DeferredUpdates::doUpdates();
}
@ -1247,7 +1247,7 @@ abstract class Maintenance {
wfLogProfilingData();
if ( $lbFactory ) {
$lbFactory->commitMasterChanges( 'doMaintenance' );
$lbFactory->commitPrimaryChanges( 'doMaintenance' );
$lbFactory->shutdown( $lbFactory::SHUTDOWN_NO_CHRONPROT );
}
}

View file

@ -172,7 +172,7 @@ class LBFactoryTest extends MediaWikiIntegrationTestCase {
// but this fools getMainLB() at least.
$factory->getMainLB( 's1wiki' )->getConnection( DB_PRIMARY );
} );
$factory->commitMasterChanges( __METHOD__ );
$factory->commitPrimaryChanges( __METHOD__ );
$this->assertSame( 1, $called );
$this->assertEquals( 2, $countLBsFunc( $factory ) );
$factory->shutdown();
@ -195,7 +195,7 @@ class LBFactoryTest extends MediaWikiIntegrationTestCase {
// but this fools getMainLB() at least.
$factory->getMainLB( 's1wiki' )->getConnection( DB_PRIMARY );
} );
$factory->commitMasterChanges( __METHOD__ );
$factory->commitPrimaryChanges( __METHOD__ );
$this->assertSame( 1, $called );
$this->assertEquals( 2, $countLBsFunc( $factory ) );
$factory->shutdown();

View file

@ -498,7 +498,7 @@ class LoadBalancerTest extends MediaWikiIntegrationTestCase {
* @covers \Wikimedia\Rdbms\LoadBalancer::beginPrimaryChanges()
* @covers \Wikimedia\Rdbms\LoadBalancer::finalizePrimaryChanges()
* @covers \Wikimedia\Rdbms\LoadBalancer::approveMasterChanges()
* @covers \Wikimedia\Rdbms\LoadBalancer::commitMasterChanges()
* @covers \Wikimedia\Rdbms\LoadBalancer::commitPrimaryChanges()
* @covers \Wikimedia\Rdbms\LoadBalancer::runMasterTransactionIdleCallbacks()
* @covers \Wikimedia\Rdbms\LoadBalancer::runMasterTransactionListenerCallbacks()
*/
@ -554,7 +554,7 @@ class LoadBalancerTest extends MediaWikiIntegrationTestCase {
} );
$lb->finalizePrimaryChanges();
$lb->approveMasterChanges( [] );
$lb->commitMasterChanges( __METHOD__ );
$lb->commitPrimaryChanges( __METHOD__ );
$lb->runMasterTransactionIdleCallbacks();
$lb->runMasterTransactionListenerCallbacks();
@ -578,7 +578,7 @@ class LoadBalancerTest extends MediaWikiIntegrationTestCase {
} );
$lb->finalizePrimaryChanges();
$lb->approveMasterChanges( [] );
$lb->commitMasterChanges( __METHOD__ );
$lb->commitPrimaryChanges( __METHOD__ );
$lb->runMasterTransactionIdleCallbacks();
$lb->runMasterTransactionListenerCallbacks();

View file

@ -211,7 +211,7 @@ class DeferredUpdatesTest extends MediaWikiIntegrationTestCase {
// clear anything
$lbFactory = MediaWikiServices::getInstance()->getDBLoadBalancerFactory();
$lbFactory->commitMasterChanges( __METHOD__ );
$lbFactory->commitPrimaryChanges( __METHOD__ );
DeferredUpdates::addCallableUpdate(
static function () use ( $updates ) {
@ -277,7 +277,7 @@ class DeferredUpdatesTest extends MediaWikiIntegrationTestCase {
$y = false;
// clear anything
$lbFactory = MediaWikiServices::getInstance()->getDBLoadBalancerFactory();
$lbFactory->commitMasterChanges( __METHOD__ );
$lbFactory->commitPrimaryChanges( __METHOD__ );
DeferredUpdates::addCallableUpdate(
static function () use ( &$x, &$y ) {
@ -373,7 +373,7 @@ class DeferredUpdatesTest extends MediaWikiIntegrationTestCase {
$this->assertSame( 1, $dbw->trxLevel() );
$this->assertEquals( [], $calls );
$lbFactory->commitMasterChanges( __METHOD__ );
$lbFactory->commitPrimaryChanges( __METHOD__ );
$this->assertEquals( [ 'oti' ], $calls );
@ -393,7 +393,7 @@ class DeferredUpdatesTest extends MediaWikiIntegrationTestCase {
new MWCallableUpdate(
static function () use ( $lbFactory, $fname, &$called ) {
$lbFactory->flushReplicaSnapshots( $fname );
$lbFactory->commitMasterChanges( $fname );
$lbFactory->commitPrimaryChanges( $fname );
$called = true;
},
$fname

View file

@ -24,7 +24,7 @@ class RefreshSecondaryDataUpdateTest extends MediaWikiIntegrationTestCase {
$goodUpdate->method( 'doUpdate' )
->willReturnCallback( static function () use ( &$goodCalls, $lbFactory, $goodTrxFname ) {
// Update can commit since it owns the transaction
$lbFactory->commitMasterChanges( $goodTrxFname );
$lbFactory->commitPrimaryChanges( $goodTrxFname );
++$goodCalls;
} );
$goodUpdate->expects( $this->once() )
@ -87,7 +87,7 @@ class RefreshSecondaryDataUpdateTest extends MediaWikiIntegrationTestCase {
$goodUpdate->method( 'doUpdate' )
->willReturnCallback( static function () use ( &$goodCalls, $lbFactory, $goodTrxFname ) {
// Update can commit since it owns the transaction
$lbFactory->commitMasterChanges( $goodTrxFname );
$lbFactory->commitPrimaryChanges( $goodTrxFname );
++$goodCalls;
} );
$goodUpdate->expects( $this->once() )
@ -103,7 +103,7 @@ class RefreshSecondaryDataUpdateTest extends MediaWikiIntegrationTestCase {
$badUpdate->method( 'doUpdate' )
->willReturnCallback( static function () use ( &$badCalls, $lbFactory, $badTrxFname ) {
// Update can commit since it owns the transaction
$lbFactory->commitMasterChanges( $badTrxFname );
$lbFactory->commitPrimaryChanges( $badTrxFname );
++$badCalls;
throw new LogicException( 'We have a problem' );
} );

View file

@ -261,7 +261,7 @@ class DatabaseTest extends PHPUnit\Framework\TestCase {
$db->onTransactionCommitOrIdle( $callback, __METHOD__ );
$this->assertFalse( $called, 'Not called when lb-transaction is active' );
$lbFactory->commitMasterChanges( __METHOD__ );
$lbFactory->commitPrimaryChanges( __METHOD__ );
$this->assertTrue( $called, 'Called when lb-transaction is committed' );
$called = false;
@ -272,7 +272,7 @@ class DatabaseTest extends PHPUnit\Framework\TestCase {
$lbFactory->rollbackMasterChanges( __METHOD__ );
$this->assertFalse( $called, 'Not called when lb-transaction is rolled back' );
$lbFactory->commitMasterChanges( __METHOD__ );
$lbFactory->commitPrimaryChanges( __METHOD__ );
$this->assertFalse( $called, 'Not called in next round commit' );
$db->setFlag( DBO_TRX );
@ -346,14 +346,14 @@ class DatabaseTest extends PHPUnit\Framework\TestCase {
$db->onTransactionPreCommitOrIdle( $callback, __METHOD__ );
$this->assertTrue( $called, 'Called when idle if DBO_TRX is set' );
$called = false;
$lbFactory->commitMasterChanges();
$lbFactory->commitPrimaryChanges();
$this->assertFalse( $called );
$called = false;
$lbFactory->beginPrimaryChanges( __METHOD__ );
$db->onTransactionPreCommitOrIdle( $callback, __METHOD__ );
$this->assertFalse( $called, 'Not called when lb-transaction is active' );
$lbFactory->commitMasterChanges( __METHOD__ );
$lbFactory->commitPrimaryChanges( __METHOD__ );
$this->assertTrue( $called, 'Called when lb-transaction is committed' );
$called = false;
@ -364,7 +364,7 @@ class DatabaseTest extends PHPUnit\Framework\TestCase {
$lbFactory->rollbackMasterChanges( __METHOD__ );
$this->assertFalse( $called, 'Not called when lb-transaction is rolled back' );
$lbFactory->commitMasterChanges( __METHOD__ );
$lbFactory->commitPrimaryChanges( __METHOD__ );
$this->assertFalse( $called, 'Not called in next round commit' );
}