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

Make insertSelect() do two separate queries in non-CLI mode

Browse source 
This avoids slave lag and makes query time account easier.
It also avoids table-level autoinc locking and slave drift
with statement-based replication in some setups.

Also refactored the use of $wgCommandLine mode in
DatabaseBase slightly, so that it can be injected.

Change-Id: I2dba6024ecf32c9ee24a3080cce3b02568c1458b
This commit is contained in:
Aaron Schulz 2016-08-27 10:10:46 -07:00
parent b99c3a3664
commit 52d2ebb30a
7 changed files with 111 additions and 21 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

5
includes/DefaultSettings.php
View file

@ -1904,7 +1904,7 @@ $wgSharedSchema = false;
* to several groups, the most specific group defined here is used.
*
* - flags: bit field
* - DBO_DEFAULT -- turns on DBO_TRX only if !$wgCommandLineMode (recommended)
* - DBO_DEFAULT -- turns on DBO_TRX only if "cliMode" is off (recommended)
* - DBO_DEBUG -- equivalent of $wgDebugDumpSql
* - DBO_TRX -- wrap entire request in a transaction
* - DBO_NOBUFFER -- turn off buffering (not useful in LocalSettings.php)
@ -1915,6 +1915,9 @@ $wgSharedSchema = false;
*
* - max lag: (optional) Maximum replication lag before a slave will taken out of rotation
* - is static: (optional) Set to true if the dataset is static and no replication is used.
* - cliMode: (optional) Connection handles will not assume that requests are short-lived
* nor that INSERT..SELECT can be rewritten into a buffered SELECT and INSERT.
* [Default: uses value of $wgCommandLineMode]
*
* These and any other user-defined properties will be assigned to the mLBInfo member
* variable of the Database object.

56
includes/db/Database.php
View file

@ -59,6 +59,8 @@ abstract class DatabaseBase implements IDatabase {
protected $mPassword;
/** @var string */
protected $mDBname;
/** @var bool */
protected $cliMode;
/** @var BagOStuff APC cache */
protected $srvCache;
@ -568,7 +570,7 @@ abstract class DatabaseBase implements IDatabase {
* @param array $params Parameters passed from DatabaseBase::factory()
*/
function __construct( array $params ) {
global $wgDBprefix, $wgDBmwschema, $wgCommandLineMode;
global $wgDBprefix, $wgDBmwschema;
$this->srvCache = ObjectCache::getLocalServerInstance( 'hash' );
@ -581,9 +583,13 @@ abstract class DatabaseBase implements IDatabase {
$schema = $params['schema'];
$foreign = $params['foreign'];
$this->cliMode = isset( $params['cliMode'] )
? $params['cliMode']
: ( PHP_SAPI === 'cli' );
$this->mFlags = $flags;
if ( $this->mFlags & DBO_DEFAULT ) {
if ( $wgCommandLineMode ) {
if ( $this->cliMode ) {
$this->mFlags &= ~DBO_TRX;
} else {
$this->mFlags |= DBO_TRX;
@ -654,6 +660,8 @@ abstract class DatabaseBase implements IDatabase {
* @return DatabaseBase|null DatabaseBase subclass or null
*/
final public static function factory( $dbType, $p = [] ) {
global $wgCommandLineMode;
$canonicalDBTypes = [
'mysql' => [ 'mysqli', 'mysql' ],
'postgres' => [],
@ -711,6 +719,7 @@ abstract class DatabaseBase implements IDatabase {
$p['schema'] = isset( $defaultSchemas[$dbType] ) ? $defaultSchemas[$dbType] : null;
}
$p['foreign'] = isset( $p['foreign'] ) ? $p['foreign'] : false;
$p['cliMode'] = $wgCommandLineMode;
return new $class( $p );
} else {
@ -1013,7 +1022,7 @@ abstract class DatabaseBase implements IDatabase {
* queries, like inserting a row can take a long time due to row locking. This method
* uses some simple heuristics to discount those cases.
*
* @param string $sql
* @param string $sql A SQL write query
* @param float $runtime Total runtime, including RTT
*/
private function updateTrxWriteQueryTime( $sql, $runtime ) {
@ -2411,7 +2420,46 @@ abstract class DatabaseBase implements IDatabase {
return $this->query( $sql, $fname );
}
public function insertSelect( $destTable, $srcTable, $varMap, $conds,
public function insertSelect(
$destTable, $srcTable, $varMap, $conds,
$fname = __METHOD__, $insertOptions = [], $selectOptions = []
) {
if ( $this->cliMode ) {
// 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(
$destTable,
$srcTable,
$varMap,
$conds,
$fname,
$insertOptions,
$selectOptions
);
}
// 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.
$fields = [];
foreach ( $varMap as $dstColumn => $sourceColumnOrSql ) {
$fields[] = $this->fieldNameWithAlias( $sourceColumnOrSql, $dstColumn );
}
$selectOptions[] = 'FOR UPDATE';
$res = $this->select( $srcTable, implode( ',', $fields ), $conds, $fname, $selectOptions );
if ( !$res ) {
return false;
}
$rows = [];
foreach ( $res as $row ) {
$rows[] = (array)$row;
}
return $this->insert( $destTable, $rows, $fname, $insertOptions );
}
public function nativeInsertSelect( $destTable, $srcTable, $varMap, $conds,
$fname = __METHOD__,
$insertOptions = [], $selectOptions = []
) {

4
includes/db/DatabaseMssql.php
View file

@ -730,12 +730,12 @@ class DatabaseMssql extends Database {
* @return null|ResultWrapper
* @throws Exception
*/
public function insertSelect( $destTable, $srcTable, $varMap, $conds, $fname = __METHOD__,
public function nativeInsertSelect( $destTable, $srcTable, $varMap, $conds, $fname = __METHOD__,
$insertOptions = [], $selectOptions = []
) {
$this->mScrollableCursor = false;
try {
$ret = parent::insertSelect(
$ret = parent::nativeInsertSelect(
$destTable,
$srcTable,
$varMap,

2
includes/db/DatabaseOracle.php
View file

@ -732,7 +732,7 @@ class DatabaseOracle extends Database {
return oci_free_statement( $stmt );
}
function insertSelect( $destTable, $srcTable, $varMap, $conds, $fname = __METHOD__,
function nativeInsertSelect( $destTable, $srcTable, $varMap, $conds, $fname = __METHOD__,
$insertOptions = [], $selectOptions = []
) {
$destTable = $this->tableName( $destTable );

2
includes/db/DatabasePostgres.php
View file

@ -904,7 +904,7 @@ __INDEXATTR__;
* @param array $selectOptions
* @return bool
*/
function insertSelect( $destTable, $srcTable, $varMap, $conds, $fname = __METHOD__,
function nativeInsertSelect( $destTable, $srcTable, $varMap, $conds, $fname = __METHOD__,
$insertOptions = [], $selectOptions = [] ) {
$destTable = $this->tableName( $destTable );

45
tests/phpunit/includes/db/DatabaseSQLTest.php
View file

@ -5,15 +5,12 @@
* This is a non DBMS depending test.
*/
class DatabaseSQLTest extends MediaWikiTestCase {
/**
* @var DatabaseTestHelper
*/
/** @var DatabaseTestHelper */
private $database;
protected function setUp() {
parent::setUp();
$this->database = new DatabaseTestHelper( __CLASS__ );
$this->database = new DatabaseTestHelper( __CLASS__, [ 'cliMode' => true ] );
}
protected function assertLastSql( $sqlText ) {
@ -23,6 +20,10 @@ class DatabaseSQLTest extends MediaWikiTestCase {
);
}
protected function assertLastSqlDb( $sqlText, $db ) {
$this->assertEquals( $db->getLastSqls(), $sqlText );
}
/**
* @dataProvider provideSelect
* @covers DatabaseBase::select
@ -354,7 +355,7 @@ class DatabaseSQLTest extends MediaWikiTestCase {
* @dataProvider provideInsertSelect
* @covers DatabaseBase::insertSelect
*/
public function testInsertSelect( $sql, $sqlText ) {
public function testInsertSelect( $sql, $sqlTextNative, $sqlSelect, $sqlInsert ) {
$this->database->insertSelect(
$sql['destTable'],
$sql['srcTable'],
@ -364,7 +365,22 @@ class DatabaseSQLTest extends MediaWikiTestCase {
isset( $sql['insertOptions'] ) ? $sql['insertOptions'] : [],
isset( $sql['selectOptions'] ) ? $sql['selectOptions'] : []
);
$this->assertLastSql( $sqlText );
$this->assertLastSql( $sqlTextNative );
$dbWeb = new DatabaseTestHelper( __CLASS__, [ 'cliMode' => false ] );
$dbWeb->forceNextResult( [
array_flip( array_keys( $sql['varMap'] ) )
] );
$dbWeb->insertSelect(
$sql['destTable'],
$sql['srcTable'],
$sql['varMap'],
$sql['conds'],
__METHOD__,
isset( $sql['insertOptions'] ) ? $sql['insertOptions'] : [],
isset( $sql['selectOptions'] ) ? $sql['selectOptions'] : []
);
$this->assertLastSqlDb( implode( '; ', [ $sqlSelect, $sqlInsert ] ), $dbWeb );
}
public static function provideInsertSelect() {
@ -379,7 +395,10 @@ class DatabaseSQLTest extends MediaWikiTestCase {
"INSERT INTO insert_table " .
"(field_insert,field) " .
"SELECT field_select,field2 " .
"FROM select_table"
"FROM select_table",
"SELECT field_select AS field_insert,field2 AS field " .
"FROM select_table WHERE * FOR UPDATE",
"INSERT INTO insert_table (field_insert,field) VALUES ('0','1')"
],
[
[
@ -392,7 +411,10 @@ class DatabaseSQLTest extends MediaWikiTestCase {
"(field_insert,field) " .
"SELECT field_select,field2 " .
"FROM select_table " .
"WHERE field = '2'"
"WHERE field = '2'",
"SELECT field_select AS field_insert,field2 AS field FROM " .
"select_table WHERE field = '2' FOR UPDATE",
"INSERT INTO insert_table (field_insert,field) VALUES ('0','1')"
],
[
[
@ -408,7 +430,10 @@ class DatabaseSQLTest extends MediaWikiTestCase {
"SELECT field_select,field2 " .
"FROM select_table " .
"WHERE field = '2' " .
"ORDER BY field"
"ORDER BY field",
"SELECT field_select AS field_insert,field2 AS field " .
"FROM select_table WHERE field = '2' ORDER BY field FOR UPDATE",
"INSERT IGNORE INTO insert_table (field_insert,field) VALUES ('0','1')"
],
];
}

18
tests/phpunit/includes/db/DatabaseTestHelper.php
View file

@ -19,17 +19,21 @@ class DatabaseTestHelper extends DatabaseBase {
*/
protected $lastSqls = [];
/** @var array List of row arrays */
protected $nextResult = [];
/**
* Array of tables to be considered as existing by tableExist()
* Use setExistingTables() to alter.
*/
protected $tablesExists;
public function __construct( $testName ) {
public function __construct( $testName, array $opts = [] ) {
$this->testName = $testName;
$this->profiler = new ProfilerStub( [] );
$this->trxProfiler = new TransactionProfiler();
$this->cliMode = isset( $opts['cliMode'] ) ? $opts['cliMode'] : true;
}
/**
@ -47,6 +51,13 @@ class DatabaseTestHelper extends DatabaseBase {
$this->tablesExists = (array)$tablesExists;
}
/**
* @param mixed $res Use an array of row arrays to set row result
*/
public function forceNextResult( $res ) {
$this->nextResult = $res;
}
protected function addSql( $sql ) {
// clean up spaces before and after some words and the whole string
$this->lastSqls[] = trim( preg_replace(
@ -168,6 +179,9 @@ class DatabaseTestHelper extends DatabaseBase {
}
protected function doQuery( $sql ) {
return [];
$res = $this->nextResult;
$this->nextResult = [];
return new FakeResultWrapper( $res );
}
}