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

rdbms: make Database::insertSelect() stricter about replication safety

Browse source 
Avoid the native INSERT SELECT method if a LIMIT clause is present.

Change-Id: Ibf9b8a4a42092fbc98d7ebd45167203a6a8801ee
This commit is contained in:
Aaron Schulz 2018-02-28 15:33:03 -08:00 committed by Krinkle
parent f4ec592330
commit 671368a59e
2 changed files with 48 additions and 46 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

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

@ -2446,11 +2446,16 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
return $this->query( $sql, $fname );
}
public function insertSelect(
final public function insertSelect(
$destTable, $srcTable, $varMap, $conds,
$fname = __METHOD__, $insertOptions = [], $selectOptions = [], $selectJoinConds = []
) {
if ( $this->cliMode ) {
static $hints = [ 'NO_AUTO_COLUMNS' ];
$insertOptions = (array)$insertOptions;
$selectOptions = (array)$selectOptions;
if ( $this->cliMode && $this->isInsertSelectSafe( $insertOptions, $selectOptions ) ) {
// For massive migrations with downtime, we don't want to select everything
// into memory and OOM, so do all this native on the server side if possible.
return $this->nativeInsertSelect(
@ -2459,7 +2464,7 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
$varMap,
$conds,
$fname,
$insertOptions,
array_diff( $insertOptions, $hints ),
$selectOptions,
$selectJoinConds
);
@ -2471,12 +2476,22 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
$varMap,
$conds,
$fname,
$insertOptions,
array_diff( $insertOptions, $hints ),
$selectOptions,
$selectJoinConds
);
}
/**
* @param array $insertOptions INSERT options
* @param array $selectOptions SELECT options
* @return bool Whether an INSERT SELECT with these options will be replication safe
* @since 1.31
*/
protected function isInsertSelectSafe( array $insertOptions, array $selectOptions ) {
return true;
}
/**
* Implementation of insertSelect() based on select() and insert()
*
@ -2496,8 +2511,6 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
$fname = __METHOD__,
$insertOptions = [], $selectOptions = [], $selectJoinConds = []
) {
$insertOptions = array_diff( (array)$insertOptions, [ 'NO_AUTO_COLUMNS' ] );
// For web requests, do a locking SELECT and then INSERT. This puts the SELECT burden
// on only the master (without needing row-based-replication). It also makes it easy to
// know how big the INSERT is going to be.
@ -2574,7 +2587,6 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
if ( !is_array( $insertOptions ) ) {
$insertOptions = [ $insertOptions ];
}
$insertOptions = array_diff( (array)$insertOptions, [ 'NO_AUTO_COLUMNS' ] );
$insertOptions = $this->makeInsertOptions( $insertOptions );

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

@ -67,6 +67,8 @@ abstract class DatabaseMysqlBase extends Database {
private $serverVersion = null;
/** @var bool|null */
private $insertSelectIsSafe = null;
/** @var stdClass|null */
private $replicationInfoRow = null;
/**
* Additional $params include:
@ -508,20 +510,32 @@ abstract class DatabaseMysqlBase extends Database {
return $this->nativeReplace( $table, $rows, $fname );
}
protected function nativeInsertSelect(
$destTable, $srcTable, $varMap, $conds,
$fname = __METHOD__, $insertOptions = [], $selectOptions = [], $selectJoinConds = []
) {
$isSafe = in_array( 'NO_AUTO_COLUMNS', $insertOptions, true );
if ( !$isSafe && $this->insertSelectIsSafe === null ) {
// In MySQL, an INSERT SELECT is only replication safe with row-based
// replication or if innodb_autoinc_lock_mode is 0. When those
// conditions aren't met, use non-native mode.
// While we could try to determine if the insert is safe anyway by
// checking if the target table has an auto-increment column that
// isn't set in $varMap, that seems unlikely to be worth the extra
// complexity.
$row = $this->selectRow(
protected function isInsertSelectSafe( array $insertOptions, array $selectOptions ) {
$row = $this->getReplicationSafetyInfo();
// For row-based-replication, the resulting changes will be relayed, not the query
if ( $row->binlog_format === 'ROW' ) {
return true;
}
// LIMIT requires ORDER BY on a unique key or it is non-deterministic
if ( isset( $selectOptions['LIMIT'] ) ) {
return false;
}
// In MySQL, an INSERT SELECT is only replication safe with row-based
// replication or if innodb_autoinc_lock_mode is 0. When those
// conditions aren't met, use non-native mode.
// While we could try to determine if the insert is safe anyway by
// checking if the target table has an auto-increment column that
// isn't set in $varMap, that seems unlikely to be worth the extra
// complexity.
return ( (int)$row->innodb_autoinc_lock_mode === 0 );
}
/**
* @return stdClass Process cached row
*/
private function getReplicationSafetyInfo() {
if ( $this->replicationInfoRow === null ) {
$this->replicationInfoRow = $this->selectRow(
false,
[
'innodb_autoinc_lock_mode' => '@@innodb_autoinc_lock_mode',
@ -530,33 +544,9 @@ abstract class DatabaseMysqlBase extends Database {
[],
__METHOD__
);
$this->insertSelectIsSafe = $row->binlog_format === 'ROW' ||
(int)$row->innodb_autoinc_lock_mode === 0;
}
if ( !$isSafe && !$this->insertSelectIsSafe ) {
return $this->nonNativeInsertSelect(
$destTable,
$srcTable,
$varMap,
$conds,
$fname,
$insertOptions,
$selectOptions,
$selectJoinConds
);
} else {
return parent::nativeInsertSelect(
$destTable,
$srcTable,
$varMap,
$conds,
$fname,
$insertOptions,
$selectOptions,
$selectJoinConds
);
}
return $this->replicationInfoRow;
}
/**