Merge "rdbms: add IDatabase::lockForUpdate() convenience method"

This commit is contained in:
jenkins-bot 2018-07-11 19:52:31 +00:00 committed by Gerrit Code Review
commit 216865344b
8 changed files with 104 additions and 25 deletions

View file

@ -337,13 +337,7 @@ class Category {
// Lock the `category` row before locking `categorylinks` rows to try
// to avoid deadlocks with LinksDeletionUpdate (T195397)
$dbw->selectField(
'category',
1,
[ 'cat_title' => $this->mName ],
__METHOD__,
[ 'FOR UPDATE' ]
);
$dbw->lockForUpdate( 'category', [ 'cat_title' => $this->mName ], __METHOD__ );
// Lock all the `categorylinks` records and gaps for this category;
// this is a separate query due to postgres/oracle limitations

View file

@ -254,6 +254,8 @@ class MergeHistory {
return $permCheck;
}
$this->dbw->startAtomic( __METHOD__ );
$this->dbw->update(
'revision',
[ 'rev_page' => $this->dest->getArticleID() ],
@ -264,17 +266,17 @@ class MergeHistory {
// Check if this did anything
$this->revisionsMerged = $this->dbw->affectedRows();
if ( $this->revisionsMerged < 1 ) {
$this->dbw->endAtomic( __METHOD__ );
$status->fatal( 'mergehistory-fail-no-change' );
return $status;
}
// Make the source page a redirect if no revisions are left
$haveRevisions = $this->dbw->selectField(
$haveRevisions = $this->dbw->lockForUpdate(
'revision',
'rev_timestamp',
[ 'rev_page' => $this->source->getArticleID() ],
__METHOD__,
[ 'FOR UPDATE' ]
__METHOD__
);
if ( !$haveRevisions ) {
if ( $reason ) {
@ -350,6 +352,8 @@ class MergeHistory {
Hooks::run( 'ArticleMergeComplete', [ $this->source, $this->dest ] );
$this->dbw->endAtomic( __METHOD__ );
return $status;
}
}

View file

@ -3344,19 +3344,15 @@ class LocalFileMoveBatch {
$status = $repo->newGood();
$dbw = $this->db;
$hasCurrent = $dbw->selectField(
$hasCurrent = $dbw->lockForUpdate(
'image',
'1',
[ 'img_name' => $this->oldName ],
__METHOD__,
[ 'FOR UPDATE' ]
__METHOD__
);
$oldRowCount = $dbw->selectRowCount(
$oldRowCount = $dbw->lockForUpdate(
'oldimage',
'*',
[ 'oi_name' => $this->oldName ],
__METHOD__,
[ 'FOR UPDATE' ]
__METHOD__
);
if ( $hasCurrent ) {

View file

@ -287,6 +287,12 @@ class DBConnRef implements IDatabase {
return $this->__call( __FUNCTION__, func_get_args() );
}
public function lockForUpdate(
$table, $conds = '', $fname = __METHOD__, $options = [], $join_conds = []
) {
return $this->__call( __FUNCTION__, func_get_args() );
}
public function fieldExists( $table, $field, $fname = __METHOD__ ) {
return $this->__call( __FUNCTION__, func_get_args() );
}

View file

@ -1876,6 +1876,22 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
return $column;
}
public function lockForUpdate(
$table, $conds = '', $fname = __METHOD__, $options = [], $join_conds = []
) {
if ( !$this->trxLevel && !$this->getFlag( self::DBO_TRX ) ) {
throw new DBUnexpectedError(
$this,
__METHOD__ . ': no transaction is active nor is DBO_TRX set'
);
}
$options = (array)$options;
$options[] = 'FOR UPDATE';
return $this->selectRowCount( $table, '*', $conds, $fname, $options, $join_conds );
}
/**
* Removes most variables from an SQL query and replaces them with X or N for numbers.
* It's only slightly flawed. Don't use for anything important.

View file

@ -848,6 +848,21 @@ interface IDatabase {
$tables, $var = '*', $conds = '', $fname = __METHOD__, $options = [], $join_conds = []
);
/**
* Lock all rows meeting the given conditions/options FOR UPDATE
*
* @param array|string $table Table names
* @param array|string $conds Filters on the table
* @param string $fname Function name for profiling
* @param array $options Options for select ("FOR UPDATE" is added automatically)
* @param array $join_conds Join conditions
* @return int Number of matching rows found (and locked)
* @since 1.32
*/
public function lockForUpdate(
$table, $conds = '', $fname = __METHOD__, $options = [], $join_conds = []
);
/**
* Determines whether a field exists in a table
*

View file

@ -2537,20 +2537,16 @@ class WikiPage implements Page, IDBAccessObject {
// Note array_intersect() preserves keys from the first arg, and we're
// assuming $revQuery has `revision` primary and isn't using subtables
// for anything we care about.
$res = $dbw->select(
$dbw->lockForUpdate(
array_intersect(
$revQuery['tables'],
[ 'revision', 'revision_comment_temp', 'revision_actor_temp' ]
),
'1',
[ 'rev_page' => $id ],
__METHOD__,
'FOR UPDATE',
[],
$revQuery['joins']
);
foreach ( $res as $row ) {
// Fetch all rows in case the DB needs that to properly lock them.
}
// If SCHEMA_COMPAT_WRITE_OLD is set, also select all extra fields we still write,
// so we can copy it to the archive table.

View file

@ -256,6 +256,58 @@ class DatabaseSQLTest extends PHPUnit\Framework\TestCase {
];
}
/**
* @dataProvider provideLockForUpdate
* @covers Wikimedia\Rdbms\Database::lockForUpdate
*/
public function testLockForUpdate( $sql, $sqlText ) {
$this->database->startAtomic( __METHOD__ );
$this->database->lockForUpdate(
$sql['tables'],
$sql['conds'] ?? [],
__METHOD__,
$sql['options'] ?? [],
$sql['join_conds'] ?? []
);
$this->database->endAtomic( __METHOD__ );
$this->assertLastSql( "BEGIN; $sqlText; COMMIT" );
}
public static function provideLockForUpdate() {
return [
[
[
'tables' => [ 'table' ],
'conds' => [ 'field' => [ 1, 2, 3, 4 ] ],
],
"SELECT COUNT(*) AS rowcount FROM " .
"(SELECT 1 FROM table WHERE field IN ('1','2','3','4') " .
"FOR UPDATE) tmp_count"
],
[
[
'tables' => [ 'table', 't2' => 'table2' ],
'conds' => [ 'field' => 'text' ],
'options' => [ 'LIMIT' => 1, 'ORDER BY' => 'field' ],
'join_conds' => [ 't2' => [
'LEFT JOIN', 'tid = t2.id'
] ],
],
"SELECT COUNT(*) AS rowcount FROM " .
"(SELECT 1 FROM table LEFT JOIN table2 t2 ON ((tid = t2.id)) " .
"WHERE field = 'text' ORDER BY field LIMIT 1 FOR UPDATE) tmp_count"
],
[
[
'tables' => 'table',
],
"SELECT COUNT(*) AS rowcount FROM " .
"(SELECT 1 FROM table FOR UPDATE) tmp_count"
],
];
}
/**
* @covers Wikimedia\Rdbms\Subquery
* @dataProvider provideSelectRowCount