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

rdbms: Add ATOMIC_CANCELABLE flag for micro-optimization

Browse source 
Aaron is concerned about the extra time added to atomic sections within
an outer transaction if we do a SAVEPOINT and RELEASE. He wants a flag
so callers have to specifically opt-in to use of savepoints.

Change-Id: I64cf5033ced464863d28dd49d9173856a9c1e1c0
This commit is contained in:
Brad Jorsch 2018-03-20 11:57:04 -04:00 committed by Aaron Schulz
parent 52aeaa7a5f
commit 3365e83d96
4 changed files with 62 additions and 23 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

4
includes/libs/rdbms/database/DBConnRef.php
View file

@ -511,7 +511,9 @@ class DBConnRef implements IDatabase {
return $this->__call( __FUNCTION__, func_get_args() );
}
public function startAtomic( $fname = __METHOD__ ) {
public function startAtomic(
$fname = __METHOD__, $cancelable = IDatabase::ATOMIC_NOT_CANCELABLE
) {
return $this->__call( __FUNCTION__, func_get_args() );
}

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

@ -2518,7 +2518,7 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
}
try {
$this->startAtomic( $fname );
$this->startAtomic( $fname, self::ATOMIC_CANCELABLE );
$affectedRowCount = 0;
foreach ( $rows as $row ) {
// Delete rows which collide with this one
@ -2623,7 +2623,7 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
$affectedRowCount = 0;
try {
$this->startAtomic( $fname );
$this->startAtomic( $fname, self::ATOMIC_CANCELABLE );
# Update any existing conflicting row(s)
if ( $where !== false ) {
$ok = $this->update( $table, $set, $where, $fname );
@ -2779,7 +2779,7 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
try {
$affectedRowCount = 0;
$this->startAtomic( $fname );
$this->startAtomic( $fname, self::ATOMIC_CANCELABLE );
$rows = [];
$ok = true;
foreach ( $res as $row ) {
@ -3095,7 +3095,7 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
$this->trxPreCommitCallbacks[] = [ $callback, $fname ];
} else {
// No transaction is active nor will start implicitly, so make one for this callback
$this->startAtomic( __METHOD__ );
$this->startAtomic( __METHOD__, self::ATOMIC_CANCELABLE );
try {
call_user_func( $callback );
$this->endAtomic( __METHOD__ );
@ -3279,7 +3279,10 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
$this->query( 'ROLLBACK TO SAVEPOINT ' . $this->addIdentifierQuotes( $identifier ), $fname );
}
final public function startAtomic( $fname = __METHOD__ ) {
final public function startAtomic(
$fname = __METHOD__, $cancelable = self::ATOMIC_NOT_CANCELABLE
) {
$savepointId = $cancelable === self::ATOMIC_CANCELABLE ? 'n/a' : null;
if ( !$this->trxLevel ) {
$this->begin( $fname, self::TRANSACTION_INTERNAL );
// If DBO_TRX is set, a series of startAtomic/endAtomic pairs will result
@ -3287,8 +3290,7 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
if ( !$this->getFlag( self::DBO_TRX ) ) {
$this->trxAutomaticAtomic = true;
}
$savepointId = null;
} else {
} elseif ( $cancelable === self::ATOMIC_CANCELABLE ) {
$savepointId = 'wikimedia_rdbms_atomic' . ++$this->trxAtomicCounter;
if ( strlen( $savepointId ) > 30 ) { // 30 == Oracle's identifier length limit (pre 12c)
$this->queryLogger->warning(
@ -3316,9 +3318,9 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
throw new DBUnexpectedError( $this, "Invalid atomic section ended (got $fname)." );
}
if ( !$savepointId ) {
if ( !$this->trxAtomicLevels && $this->trxAutomaticAtomic ) {
$this->commit( $fname, self::FLUSHING_INTERNAL );
} else {
} elseif ( $savepointId && $savepointId !== 'n/a' ) {
$this->doReleaseSavepoint( $savepointId, $fname );
}
}
@ -3333,10 +3335,13 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
if ( $savedFname !== $fname ) {
throw new DBUnexpectedError( $this, "Invalid atomic section ended (got $fname)." );
}
if ( !$savepointId ) {
throw new DBUnexpectedError( $this, "Uncancelable atomic section canceled (got $fname)." );
}
if ( !$this->trxAtomicLevels && $this->trxAutomaticAtomic ) {
$this->rollback( $fname, self::FLUSHING_INTERNAL );
} else {
} elseif ( $savepointId !== 'n/a' ) {
$this->doRollbackToSavepoint( $savepointId, $fname );
}
@ -3344,7 +3349,7 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
}
final public function doAtomicSection( $fname, callable $callback ) {
$this->startAtomic( $fname );
$this->startAtomic( $fname, self::ATOMIC_CANCELABLE );
try {
$res = call_user_func_array( $callback, [ $this, $fname ] );
} catch ( Exception $e ) {

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

@ -49,6 +49,11 @@ interface IDatabase {
/** @var string Transaction is requested internally via DBO_TRX/startAtomic() */
const TRANSACTION_INTERNAL = 'implicit';
/** @var string Atomic section is not cancelable */
const ATOMIC_NOT_CANCELABLE = '';
/** @var string Atomic section is cancelable */
const ATOMIC_CANCELABLE = 'cancelable';
/** @var string Transaction operation comes from service managing all DBs */
const FLUSHING_ALL_PEERS = 'flush';
/** @var string Transaction operation comes from the database class internally */
@ -1580,11 +1585,11 @@ interface IDatabase {
/**
* Begin an atomic section of statements
*
* If a transaction has been started already, sets a savepoint and tracks
* the given section name to make sure the transaction is not committed
* pre-maturely. This function can be used in layers (with sub-sections),
* so use a stack to keep track of the different atomic sections. If there
* is no transaction, one is started implicitly.
* If a transaction has been started already, (optionally) sets a savepoint
* and tracks the given section name to make sure the transaction is not
* committed pre-maturely. This function can be used in layers (with
* sub-sections), so use a stack to keep track of the different atomic
* sections. If there is no transaction, one is started implicitly.
*
* The goal of this function is to create an atomic section of SQL queries
* without having to start a new transaction if it already exists.
@ -1596,9 +1601,11 @@ interface IDatabase {
*
* @since 1.23
* @param string $fname
* @param string $cancelable Pass self::ATOMIC_CANCELABLE to use a
* savepoint and enable self::cancelAtomic() for this section.
* @throws DBError
*/
public function startAtomic( $fname = __METHOD__ );
public function startAtomic( $fname = __METHOD__, $cancelable = self::ATOMIC_NOT_CANCELABLE );
/**
* Ends an atomic section of SQL statements
@ -1626,6 +1633,8 @@ interface IDatabase {
* Note that a call to IDatabase::rollback() will also roll back any open
* atomic sections.
*
* @note As a micro-optimization to save a few DB calls, this method may only
* be called when startAtomic() was called with the ATOMIC_CANCELABLE flag.
* @since 1.31
* @see IDatabase::startAtomic
* @param string $fname

31
tests/phpunit/includes/libs/rdbms/database/DatabaseSQLTest.php
View file

@ -1,5 +1,6 @@
<?php
use Wikimedia\Rdbms\IDatabase;
use Wikimedia\Rdbms\LikeMatch;
use Wikimedia\Rdbms\Database;
@ -1366,7 +1367,7 @@ class DatabaseSQLTest extends PHPUnit\Framework\TestCase {
$this->database->endAtomic( __METHOD__ );
$this->assertLastSql( 'BEGIN; COMMIT' );
$this->database->startAtomic( __METHOD__ );
$this->database->startAtomic( __METHOD__, IDatabase::ATOMIC_CANCELABLE );
$this->database->cancelAtomic( __METHOD__ );
$this->assertLastSql( 'BEGIN; ROLLBACK' );
@ -1374,18 +1375,24 @@ class DatabaseSQLTest extends PHPUnit\Framework\TestCase {
$this->database->startAtomic( __METHOD__ );
$this->database->endAtomic( __METHOD__ );
$this->database->commit( __METHOD__ );
$this->assertLastSql( 'BEGIN; COMMIT' );
$this->database->begin( __METHOD__ );
$this->database->startAtomic( __METHOD__, IDatabase::ATOMIC_CANCELABLE );
$this->database->endAtomic( __METHOD__ );
$this->database->commit( __METHOD__ );
// phpcs:ignore Generic.Files.LineLength
$this->assertLastSql( 'BEGIN; SAVEPOINT wikimedia_rdbms_atomic1; RELEASE SAVEPOINT wikimedia_rdbms_atomic1; COMMIT' );
$this->database->begin( __METHOD__ );
$this->database->startAtomic( __METHOD__ );
$this->database->startAtomic( __METHOD__, IDatabase::ATOMIC_CANCELABLE );
$this->database->cancelAtomic( __METHOD__ );
$this->database->commit( __METHOD__ );
// phpcs:ignore Generic.Files.LineLength
$this->assertLastSql( 'BEGIN; SAVEPOINT wikimedia_rdbms_atomic1; ROLLBACK TO SAVEPOINT wikimedia_rdbms_atomic1; COMMIT' );
$this->database->startAtomic( __METHOD__ );
$this->database->startAtomic( __METHOD__ );
$this->database->startAtomic( __METHOD__, IDatabase::ATOMIC_CANCELABLE );
$this->database->startAtomic( __METHOD__, IDatabase::ATOMIC_CANCELABLE );
$this->database->cancelAtomic( __METHOD__ );
$this->database->endAtomic( __METHOD__ );
// phpcs:ignore Generic.Files.LineLength
@ -1458,4 +1465,20 @@ class DatabaseSQLTest extends PHPUnit\Framework\TestCase {
}
}
/**
* @covers \Wikimedia\Rdbms\Database::cancelAtomic
*/
public function testUncancellableAtomicSection() {
$this->database->startAtomic( __METHOD__ );
try {
$this->database->cancelAtomic( __METHOD__ );
$this->fail( 'Expected exception not thrown' );
} catch ( DBUnexpectedError $ex ) {
$this->assertSame(
'Uncancelable atomic section canceled (got ' . __METHOD__ . ').',
$ex->getMessage()
);
}
}
}