Thijs/wiki.techinc.nl
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

rdbms: clean up DBO_TRX behavior for onTransaction* callbacks

Browse source 
* Make onTransactionIdle() wait until any transaction round
  is gone, even if there is no SQL transaction active. This
  is what onTransactionPreCommitOrIdle() already does.
* Decouple "transaction round mode" (DBO_TRX) from whether a
  round is active via a 'trxRoundId' LB info field. If rounds
  are enabled, but not is started, then the transaction state
  should be interpreted as "idle".
* Improve related documentation.
* Add more related unit tests.

Change-Id: I3ab18f577ec0375897fcb63f18f4ee2deeb436e9
This commit is contained in:
Aaron Schulz 2018-03-18 22:26:11 -07:00
parent b5fe7fbbed
commit d4c31cf841
5 changed files with 130 additions and 34 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

31
includes/libs/rdbms/database/Database.php
View file

@ -635,6 +635,20 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
);
}
/**
* @return string|null
*/
final protected function getTransactionRoundId() {
// If transaction round participation is enabled, see if one is active
if ( $this->getFlag( self::DBO_TRX ) ) {
$id = $this->getLBInfo( 'trxRoundId' );
return is_string( $id ) ? $id : null;
}
return null;
}
public function pendingWriteQueryDuration( $type = self::ESTIMATE_TOTAL ) {
if ( !$this->trxLevel ) {
return false;
@ -3073,6 +3087,12 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
}
final public function onTransactionIdle( callable $callback, $fname = __METHOD__ ) {
if ( !$this->trxLevel && $this->getTransactionRoundId() ) {
// Start an implicit transaction similar to how query() does
$this->begin( __METHOD__, self::TRANSACTION_INTERNAL );
$this->trxAutomatic = true;
}
$this->trxIdleCallbacks[] = [ $callback, $fname ];
if ( !$this->trxLevel ) {
$this->runOnTransactionIdleCallbacks( self::TRIGGER_IDLE );
@ -3080,10 +3100,13 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
}
final public function onTransactionPreCommitOrIdle( callable $callback, $fname = __METHOD__ ) {
if ( $this->trxLevel || $this->getFlag( self::DBO_TRX ) ) {
// As long as DBO_TRX is set, writes will accumulate until the load balancer issues
// an implicit commit of all peer databases. This is true even if a transaction has
// not yet been triggered by writes; make sure $callback runs *after* any such writes.
if ( !$this->trxLevel && $this->getTransactionRoundId() ) {
// Start an implicit transaction similar to how query() does
$this->begin( __METHOD__, self::TRANSACTION_INTERNAL );
$this->trxAutomatic = true;
}
if ( $this->trxLevel ) {
$this->trxPreCommitCallbacks[] = [ $callback, $fname ];
} else {
// No transaction is active nor will start implicitly, so make one for this callback

2
includes/libs/rdbms/database/DatabaseMysqlBase.php
View file

@ -317,7 +317,7 @@ abstract class DatabaseMysqlBase extends Database {
abstract protected function mysqlFetchObject( $res );
/**
* @param ResultWrapper|resource $res
* @param IResultWrapper|resource $res
* @return array|bool
* @throws DBUnexpectedError
*/

19
includes/libs/rdbms/database/IDatabase.php
View file

@ -90,7 +90,7 @@ interface IDatabase {
const DBO_NOBUFFER = 2;
/** @var int Ignore query errors (internal use only!) */
const DBO_IGNORE = 4;
/** @var int Autoatically start transaction on first query (work with ILoadBalancer rounds) */
/** @var int Automatically start a transaction before running a query if none is active */
const DBO_TRX = 8;
/** @var int Use DBO_TRX in non-CLI mode */
const DBO_DEFAULT = 16;
@ -253,7 +253,7 @@ interface IDatabase {
public function writesPending();
/**
* Returns true if there is a transaction open with possible write
* Returns true if there is a transaction/round open with possible write
* queries or transaction pre-commit/idle callbacks waiting on it to finish.
* This does *not* count recurring callbacks, e.g. from setTransactionListener().
*
@ -1490,13 +1490,19 @@ interface IDatabase {
/**
* Run a callback as soon as there is no transaction pending.
* If there is a transaction and it is rolled back, then the callback is cancelled.
*
* When transaction round mode (DBO_TRX) is set, the callback will run at the end
* of the round, just after all peer transactions COMMIT. If the transaction round
* is rolled back, then the callback is cancelled.
*
* Queries in the function will run in AUTO-COMMIT mode unless there are begin() calls.
* Callbacks must commit any transactions that they begin.
*
* This is useful for updates to different systems or when separate transactions are needed.
* For example, one might want to enqueue jobs into a system outside the database, but only
* after the database is updated so that the jobs will see the data when they actually run.
* It can also be used for updates that easily cause deadlocks if locks are held too long.
* It can also be used for updates that easily suffer from lock timeouts and deadlocks,
* but where atomicity is not essential.
*
* Updates will execute in the order they were enqueued.
*
@ -1512,10 +1518,15 @@ interface IDatabase {
/**
* Run a callback before the current transaction commits or now if there is none.
* If there is a transaction and it is rolled back, then the callback is cancelled.
*
* When transaction round mode (DBO_TRX) is set, the callback will run at the end
* of the round, just before all peer transactions COMMIT. If the transaction round
* is rolled back, then the callback is cancelled.
*
* Callbacks must not start nor commit any transactions. If no transaction is active,
* then a transaction will wrap the callback.
*
* This is useful for updates that easily cause deadlocks if locks are held too long
* This is useful for updates that easily suffer from lock timeouts and deadlocks,
* but where atomicity is strongly desired for these updates and some related updates.
*
* Updates will execute in the order they were enqueued.

17
includes/libs/rdbms/loadbalancer/LoadBalancer.php
View file

@ -1423,6 +1423,12 @@ class LoadBalancer implements ILoadBalancer {
}
/**
* Make all DB servers with DBO_DEFAULT/DBO_TRX set join the transaction round
*
* Some servers may have neither flag enabled, meaning that they opt out of such
* transaction rounds and remain in auto-commit mode. Such behavior might be desired
* when a DB server is used for something like simple key/value storage.
*
* @param IDatabase $conn
*/
private function applyTransactionRoundFlags( IDatabase $conn ) {
@ -1434,9 +1440,10 @@ class LoadBalancer implements ILoadBalancer {
// DBO_TRX is controlled entirely by CLI mode presence with DBO_DEFAULT.
// Force DBO_TRX even in CLI mode since a commit round is expected soon.
$conn->setFlag( $conn::DBO_TRX, $conn::REMEMBER_PRIOR );
// If config has explicitly requested DBO_TRX be either on or off by not
// setting DBO_DEFAULT, then respect that. Forcing no transactions is useful
// for things like blob stores (ExternalStore) which want auto-commit mode.
}
if ( $conn->getFlag( $conn::DBO_TRX ) ) {
$conn->setLBInfo( 'trxRoundId', $this->trxRoundId );
}
}
@ -1448,6 +1455,10 @@ class LoadBalancer implements ILoadBalancer {
return; // transaction rounds do not apply to these connections
}
if ( $conn->getFlag( $conn::DBO_TRX ) ) {
$conn->setLBInfo( 'trxRoundId', false );
}
if ( $conn->getFlag( $conn::DBO_DEFAULT ) ) {
$conn->restoreFlags( $conn::RESTORE_PRIOR );
}

95
tests/phpunit/includes/libs/rdbms/database/DatabaseTest.php
View file

@ -177,28 +177,27 @@ class DatabaseTest extends PHPUnit\Framework\TestCase {
public function testTransactionIdle() {
$db = $this->db;
$db->setFlag( DBO_TRX );
$db->clearFlag( DBO_TRX );
$called = false;
$flagSet = null;
$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' );
$callback = function () use ( $db, &$flagSet, &$called ) {
$called = true;
$flagSet = $db->getFlag( DBO_TRX );
};
$db->onTransactionIdle( $callback, __METHOD__ );
$this->assertTrue( $called, 'Callback reached' );
$this->assertFalse( $flagSet, 'DBO_TRX off in callback' );
$this->assertFalse( $db->getFlag( DBO_TRX ), 'DBO_TRX still default' );
$db->clearFlag( DBO_TRX );
$flagSet = null;
$db->onTransactionIdle(
function () use ( $db, &$flagSet ) {
$flagSet = $db->getFlag( DBO_TRX );
},
__METHOD__
);
$called = false;
$db->startAtomic( __METHOD__ );
$db->onTransactionIdle( $callback, __METHOD__ );
$this->assertFalse( $called, 'Callback not reached during TRX' );
$db->endAtomic( __METHOD__ );
$this->assertTrue( $called, 'Callback reached after COMMIT' );
$this->assertFalse( $flagSet, 'DBO_TRX off in callback' );
$this->assertFalse( $db->getFlag( DBO_TRX ), 'DBO_TRX restored to default' );
@ -212,6 +211,56 @@ class DatabaseTest extends PHPUnit\Framework\TestCase {
$this->assertFalse( $db->getFlag( DBO_TRX ), 'DBO_TRX restored to default' );
}
/**
* @covers Wikimedia\Rdbms\Database::onTransactionIdle
* @covers Wikimedia\Rdbms\Database::runOnTransactionIdleCallbacks
*/
public function testTransactionIdle_TRX() {
$db = $this->getMockDB( [ 'isOpen', 'ping' ] );
$db->method( 'isOpen' )->willReturn( true );
$db->method( 'ping' )->willReturn( true );
$db->setFlag( DBO_TRX );
$lbFactory = LBFactorySingle::newFromConnection( $db );
// Ask for the connection so that LB sets internal state
// about this connection being the master connection
$lb = $lbFactory->getMainLB();
$conn = $lb->openConnection( $lb->getWriterIndex() );
$this->assertSame( $db, $conn, 'Same DB instance' );
$this->assertTrue( $db->getFlag( DBO_TRX ), 'DBO_TRX is set' );
$called = false;
$flagSet = null;
$callback = function () use ( $db, &$flagSet, &$called ) {
$called = true;
$flagSet = $db->getFlag( DBO_TRX );
};
$db->onTransactionIdle( $callback, __METHOD__ );
$this->assertTrue( $called, 'Called when idle if DBO_TRX is set' );
$this->assertFalse( $flagSet, 'DBO_TRX off in callback' );
$this->assertTrue( $db->getFlag( DBO_TRX ), 'DBO_TRX still default' );
$called = false;
$lbFactory->beginMasterChanges( __METHOD__ );
$db->onTransactionIdle( $callback, __METHOD__ );
$this->assertFalse( $called, 'Not called when lb-transaction is active' );
$lbFactory->commitMasterChanges( __METHOD__ );
$this->assertTrue( $called, 'Called when lb-transaction is committed' );
$called = false;
$lbFactory->beginMasterChanges( __METHOD__ );
$db->onTransactionIdle( $callback, __METHOD__ );
$this->assertFalse( $called, 'Not called when lb-transaction is active' );
$lbFactory->rollbackMasterChanges( __METHOD__ );
$this->assertFalse( $called, 'Not called when lb-transaction is rolled back' );
$lbFactory->commitMasterChanges( __METHOD__ );
$this->assertFalse( $called, 'Not called in next round commit' );
}
/**
* @covers Wikimedia\Rdbms\Database::onTransactionPreCommitOrIdle
* @covers Wikimedia\Rdbms\Database::runOnTransactionPreCommitCallbacks
@ -250,12 +299,13 @@ class DatabaseTest extends PHPUnit\Framework\TestCase {
* @covers Wikimedia\Rdbms\Database::runOnTransactionPreCommitCallbacks
*/
public function testTransactionPreCommitOrIdle_TRX() {
$db = $this->getMockDB( [ 'isOpen' ] );
$db = $this->getMockDB( [ 'isOpen', 'ping' ] );
$db->method( 'isOpen' )->willReturn( true );
$db->method( 'ping' )->willReturn( true );
$db->setFlag( DBO_TRX );
$lbFactory = LBFactorySingle::newFromConnection( $db );
// Ask for the connectin so that LB sets internal state
// Ask for the connection so that LB sets internal state
// about this connection being the master connection
$lb = $lbFactory->getMainLB();
$conn = $lb->openConnection( $lb->getWriterIndex() );
@ -267,11 +317,12 @@ class DatabaseTest extends PHPUnit\Framework\TestCase {
$called = true;
};
$db->onTransactionPreCommitOrIdle( $callback, __METHOD__ );
$this->assertFalse( $called, 'Not called when idle if DBO_TRX is set' );
$this->assertTrue( $called, 'Called when idle if DBO_TRX is set' );
$called = false;
$lbFactory->beginMasterChanges( __METHOD__ );
$db->onTransactionPreCommitOrIdle( $callback, __METHOD__ );
$this->assertFalse( $called, 'Not called when lb-transaction is active' );
$lbFactory->commitMasterChanges( __METHOD__ );
$this->assertTrue( $called, 'Called when lb-transaction is committed' );