From: Aaron Schulz Date: Wed, 7 Feb 2018 09:39:24 +0000 (-0800) Subject: rdbms: avoid "SHOW MASTER/SLAVE STATUS" queries in the GTID case X-Git-Tag: 1.31.0-rc.0~608^2 X-Git-Url: http://git.cyclocoop.org/url?a=commitdiff_plain;h=a114bd9f4d9da42613051d0180885022313fb300;p=lhc%2Fweb%2Fwiklou.git rdbms: avoid "SHOW MASTER/SLAVE STATUS" queries in the GTID case The binlog file/pos where only being used in __toString() for the GTID case. Make that method use the GTID set instead and avoid querying the old-fashioned binlog fields all together in that case. The STATUS queries involve some global lock contention. Bug: T180918 Change-Id: I18123a702e4f554b87bf5f90017b248062e73049 --- diff --git a/includes/libs/rdbms/database/DatabaseMysqlBase.php b/includes/libs/rdbms/database/DatabaseMysqlBase.php index 208f506520..c20c815f1d 100644 --- a/includes/libs/rdbms/database/DatabaseMysqlBase.php +++ b/includes/libs/rdbms/database/DatabaseMysqlBase.php @@ -889,17 +889,15 @@ abstract class DatabaseMysqlBase extends Database { return 0; // already reached this point for sure } - $useGTID = ( $this->useGTIDs && $pos->gtids ); - // Call doQuery() directly, to avoid opening a transaction if DBO_TRX is set - if ( $useGTID ) { + if ( $pos->gtids ) { // Wait on the GTID set (MariaDB only) $gtidArg = $this->addQuotes( implode( ',', $pos->gtids ) ); $res = $this->doQuery( "SELECT MASTER_GTID_WAIT($gtidArg, $timeout)" ); } else { // Wait on the binlog coordinates - $encFile = $this->addQuotes( $pos->file ); - $encPos = intval( $pos->pos ); + $encFile = $this->addQuotes( $pos->getLogFile() ); + $encPos = intval( $pos->pos[1] ); $res = $this->doQuery( "SELECT MASTER_POS_WAIT($encFile, $encPos, $timeout)" ); } @@ -912,7 +910,7 @@ abstract class DatabaseMysqlBase extends Database { // Result can be NULL (error), -1 (timeout), or 0+ per the MySQL manual $status = ( $row[0] !== null ) ? intval( $row[0] ) : null; if ( $status === null ) { - if ( !$useGTID ) { + if ( !$pos->gtids ) { // T126436: jobs programmed to wait on master positions might be referencing // binlogs with an old master hostname; this makes MASTER_POS_WAIT() return null. // Try to detect this case and treat the replica DB as having reached the given @@ -938,24 +936,26 @@ abstract class DatabaseMysqlBase extends Database { * @return MySQLMasterPos|bool */ public function getReplicaPos() { - $res = $this->query( 'SHOW SLAVE STATUS', __METHOD__ ); - $row = $this->fetchObject( $res ); + $now = microtime( true ); - if ( $row ) { - $pos = $row->Exec_Master_Log_Pos; - // Also fetch the last-applied GTID set (MariaDB) - if ( $this->useGTIDs ) { - $res = $this->query( "SHOW GLOBAL VARIABLES LIKE 'gtid_slave_pos'", __METHOD__ ); - $gtidRow = $this->fetchObject( $res ); - $gtidSet = $gtidRow ? $gtidRow->Value : ''; - } else { - $gtidSet = ''; + if ( $this->useGTIDs ) { + $res = $this->query( "SELECT @@global.gtid_slave_pos AS Value", __METHOD__ ); + $gtidRow = $this->fetchObject( $res ); + if ( $gtidRow && strlen( $gtidRow->Value ) ) { + return new MySQLMasterPos( $gtidRow->Value, $now ); } + } - return new MySQLMasterPos( $row->Relay_Master_Log_File, $pos, $gtidSet ); - } else { - return false; + $res = $this->query( 'SHOW SLAVE STATUS', __METHOD__ ); + $row = $this->fetchObject( $res ); + if ( $row && strlen( $row->Relay_Master_Log_File ) ) { + return new MySQLMasterPos( + "{$row->Relay_Master_Log_File}/{$row->Exec_Master_Log_Pos}", + $now + ); } + + return false; } /** @@ -964,23 +964,23 @@ abstract class DatabaseMysqlBase extends Database { * @return MySQLMasterPos|bool */ public function getMasterPos() { - $res = $this->query( 'SHOW MASTER STATUS', __METHOD__ ); - $row = $this->fetchObject( $res ); + $now = microtime( true ); - if ( $row ) { - // Also fetch the last-written GTID set (MariaDB) - if ( $this->useGTIDs ) { - $res = $this->query( "SHOW GLOBAL VARIABLES LIKE 'gtid_binlog_pos'", __METHOD__ ); - $gtidRow = $this->fetchObject( $res ); - $gtidSet = $gtidRow ? $gtidRow->Value : ''; - } else { - $gtidSet = ''; + if ( $this->useGTIDs ) { + $res = $this->query( "SELECT @@global.gtid_binlog_pos AS Value", __METHOD__ ); + $gtidRow = $this->fetchObject( $res ); + if ( $gtidRow && strlen( $gtidRow->Value ) ) { + return new MySQLMasterPos( $gtidRow->Value, $now ); } + } - return new MySQLMasterPos( $row->File, $row->Position, $gtidSet ); - } else { - return false; + $res = $this->query( 'SHOW MASTER STATUS', __METHOD__ ); + $row = $this->fetchObject( $res ); + if ( $row && strlen( $row->File ) ) { + return new MySQLMasterPos( "{$row->File}/{$row->Position}", $now ); } + + return false; } public function serverIsReadOnly() { diff --git a/includes/libs/rdbms/database/position/MySQLMasterPos.php b/includes/libs/rdbms/database/position/MySQLMasterPos.php index 9ca6b111eb..e5bb2c0147 100644 --- a/includes/libs/rdbms/database/position/MySQLMasterPos.php +++ b/includes/libs/rdbms/database/position/MySQLMasterPos.php @@ -13,9 +13,9 @@ use InvalidArgumentException; * that GTID sets are complete (e.g. include all domains on the server). */ class MySQLMasterPos implements DBMasterPos { - /** @var string Binlog file */ - public $file; - /** @var int Binglog file position */ + /** @var string|null Binlog file base name */ + public $binlog; + /** @var int[]|null Binglog file position tuple */ public $pos; /** @var string[] GTID list */ public $gtids = []; @@ -23,29 +23,29 @@ class MySQLMasterPos implements DBMasterPos { public $asOfTime = 0.0; /** - * @param string $file Binlog file name - * @param int $pos Binlog position - * @param string $gtid Comma separated GTID set [optional] + * @param string $position One of (comma separated GTID list, /) + * @param float $asOfTime UNIX timestamp */ - function __construct( $file, $pos, $gtid = '' ) { - $this->file = $file; - $this->pos = $pos; - $this->gtids = array_map( 'trim', explode( ',', $gtid ) ); - $this->asOfTime = microtime( true ); - } + public function __construct( $position, $asOfTime ) { + $m = []; + if ( preg_match( '!^(.+)\.(\d+)/(\d+)$!', $position, $m ) ) { + $this->binlog = $m[1]; // ideally something like host name + $this->pos = [ (int)$m[2], (int)$m[3] ]; + } else { + $this->gtids = array_map( 'trim', explode( ',', $position ) ); + if ( !$this->gtids ) { + throw new InvalidArgumentException( "GTID set should not be empty." ); + } + } - /** - * @return string /, e.g db1034-bin.000976/843431247 - */ - function __toString() { - return "{$this->file}/{$this->pos}"; + $this->asOfTime = $asOfTime; } - function asOfTime() { + public function asOfTime() { return $this->asOfTime; } - function hasReached( DBMasterPos $pos ) { + public function hasReached( DBMasterPos $pos ) { if ( !( $pos instanceof self ) ) { throw new InvalidArgumentException( "Position not an instance of " . __CLASS__ ); } @@ -75,7 +75,7 @@ class MySQLMasterPos implements DBMasterPos { return false; } - function channelsMatch( DBMasterPos $pos ) { + public function channelsMatch( DBMasterPos $pos ) { if ( !( $pos instanceof self ) ) { throw new InvalidArgumentException( "Position not an instance of " . __CLASS__ ); } @@ -95,6 +95,22 @@ class MySQLMasterPos implements DBMasterPos { return ( $thisBinPos && $thatBinPos && $thisBinPos['binlog'] === $thatBinPos['binlog'] ); } + /** + * @return string|null + */ + public function getLogFile() { + return $this->gtids ? null : "{$this->binlog}.{$this->pos[0]}"; + } + + /** + * @return string GTID set or / (e.g db1034-bin.000976/843431247) + */ + public function __toString() { + return $this->gtids + ? implode( ',', $this->gtids ) + : $this->getLogFile() . "/{$this->pos[1]}"; + } + /** * @note: this returns false for multi-source replication GTID sets * @see https://mariadb.com/kb/en/mariadb/gtid @@ -127,11 +143,8 @@ class MySQLMasterPos implements DBMasterPos { * @return array|bool (binlog, (integer file number, integer position)) or false */ protected function getBinlogCoordinates() { - $m = []; - if ( preg_match( '!^(.+)\.(\d+)/(\d+)$!', (string)$this, $m ) ) { - return [ 'binlog' => $m[1], 'pos' => [ (int)$m[2], (int)$m[3] ] ]; - } - - return false; + return ( $this->binlog !== null && $this->pos !== null ) + ? [ 'binlog' => $this->binlog, 'pos' => $this->pos ] + : false; } } diff --git a/tests/phpunit/includes/db/LBFactoryTest.php b/tests/phpunit/includes/db/LBFactoryTest.php index fa1cfc98ee..9038e5a6ab 100644 --- a/tests/phpunit/includes/db/LBFactoryTest.php +++ b/tests/phpunit/includes/db/LBFactoryTest.php @@ -194,12 +194,12 @@ class LBFactoryTest extends MediaWikiTestCase { * @covers \Wikimedia\Rdbms\ChronologyProtector */ public function testChronologyProtector() { - // (a) First HTTP request - $m1Pos = new MySQLMasterPos( 'db1034-bin.000976', '843431247' ); - $m2Pos = new MySQLMasterPos( 'db1064-bin.002400', '794074907' ); - $now = microtime( true ); + // (a) First HTTP request + $m1Pos = new MySQLMasterPos( 'db1034-bin.000976/843431247', $now ); + $m2Pos = new MySQLMasterPos( 'db1064-bin.002400/794074907', $now ); + // Master DB 1 $mockDB1 = $this->getMockBuilder( DatabaseMysqli::class ) ->disableOriginalConstructor() diff --git a/tests/phpunit/includes/libs/rdbms/database/DatabaseMysqlBaseTest.php b/tests/phpunit/includes/libs/rdbms/database/DatabaseMysqlBaseTest.php index caf1281e65..b2eabb1fc2 100644 --- a/tests/phpunit/includes/libs/rdbms/database/DatabaseMysqlBaseTest.php +++ b/tests/phpunit/includes/libs/rdbms/database/DatabaseMysqlBaseTest.php @@ -216,6 +216,14 @@ class DatabaseMysqlBaseTest extends PHPUnit_Framework_TestCase { $db->listViews( '' ) ); } + public function testBinLogName() { + $pos = new MySQLMasterPos( "db1052.2424/4643", 1 ); + + $this->assertEquals( "db1052", $pos->binlog ); + $this->assertEquals( "db1052.2424", $pos->getLogFile() ); + $this->assertEquals( [ 2424, 4643 ], $pos->pos ); + } + /** * @dataProvider provideComparePositions * @covers Wikimedia\Rdbms\MySQLMasterPos @@ -237,53 +245,55 @@ class DatabaseMysqlBaseTest extends PHPUnit_Framework_TestCase { } public static function provideComparePositions() { + $now = microtime( true ); + return [ // Binlog style [ - new MySQLMasterPos( 'db1034-bin.000976', '843431247' ), - new MySQLMasterPos( 'db1034-bin.000976', '843431248' ), + new MySQLMasterPos( 'db1034-bin.000976/843431247', $now ), + new MySQLMasterPos( 'db1034-bin.000976/843431248', $now ), true ], [ - new MySQLMasterPos( 'db1034-bin.000976', '999' ), - new MySQLMasterPos( 'db1034-bin.000976', '1000' ), + new MySQLMasterPos( 'db1034-bin.000976/999', $now ), + new MySQLMasterPos( 'db1034-bin.000976/1000', $now ), true ], [ - new MySQLMasterPos( 'db1034-bin.000976', '999' ), - new MySQLMasterPos( 'db1035-bin.000976', '1000' ), + new MySQLMasterPos( 'db1034-bin.000976/999', $now ), + new MySQLMasterPos( 'db1035-bin.000976/1000', $now ), false ], // MySQL GTID style [ - new MySQLMasterPos( 'db1-bin.2', '1', '3E11FA47-71CA-11E1-9E33-C80AA9429562:23' ), - new MySQLMasterPos( 'db1-bin.2', '2', '3E11FA47-71CA-11E1-9E33-C80AA9429562:24' ), + new MySQLMasterPos( '3E11FA47-71CA-11E1-9E33-C80AA9429562:23', $now ), + new MySQLMasterPos( '3E11FA47-71CA-11E1-9E33-C80AA9429562:24', $now ), true ], [ - new MySQLMasterPos( 'db1-bin.2', '1', '3E11FA47-71CA-11E1-9E33-C80AA9429562:99' ), - new MySQLMasterPos( 'db1-bin.2', '2', '3E11FA47-71CA-11E1-9E33-C80AA9429562:100' ), + new MySQLMasterPos( '3E11FA47-71CA-11E1-9E33-C80AA9429562:99', $now ), + new MySQLMasterPos( '3E11FA47-71CA-11E1-9E33-C80AA9429562:100', $now ), true ], [ - new MySQLMasterPos( 'db1-bin.2', '1', '3E11FA47-71CA-11E1-9E33-C80AA9429562:99' ), - new MySQLMasterPos( 'db1-bin.2', '2', '1E11FA47-71CA-11E1-9E33-C80AA9429562:100' ), + new MySQLMasterPos( '3E11FA47-71CA-11E1-9E33-C80AA9429562:99', $now ), + new MySQLMasterPos( '1E11FA47-71CA-11E1-9E33-C80AA9429562:100', $now ), false ], // MariaDB GTID style [ - new MySQLMasterPos( 'db1-bin.2', '1', '255-11-23' ), - new MySQLMasterPos( 'db1-bin.2', '2', '255-11-24' ), + new MySQLMasterPos( '255-11-23', $now ), + new MySQLMasterPos( '255-11-24', $now ), true ], [ - new MySQLMasterPos( 'db1-bin.2', '1', '255-11-99' ), - new MySQLMasterPos( 'db1-bin.2', '2', '255-11-100' ), + new MySQLMasterPos( '255-11-99', $now ), + new MySQLMasterPos( '255-11-100', $now ), true ], [ - new MySQLMasterPos( 'db1-bin.2', '1', '255-11-999' ), - new MySQLMasterPos( 'db1-bin.2', '2', '254-11-1000' ), + new MySQLMasterPos( '255-11-999', $now ), + new MySQLMasterPos( '254-11-1000', $now ), false ], ]; @@ -296,28 +306,33 @@ class DatabaseMysqlBaseTest extends PHPUnit_Framework_TestCase { public function testChannelsMatch( MySQLMasterPos $pos1, MySQLMasterPos $pos2, $matches ) { $this->assertEquals( $matches, $pos1->channelsMatch( $pos2 ) ); $this->assertEquals( $matches, $pos2->channelsMatch( $pos1 ) ); + + $roundtripPos = new MySQLMasterPos( (string)$pos1, 1 ); + $this->assertEquals( (string)$pos1, (string)$roundtripPos ); } public static function provideChannelPositions() { + $now = microtime( true ); + return [ [ - new MySQLMasterPos( 'db1034-bin.000876', '44' ), - new MySQLMasterPos( 'db1034-bin.000976', '74' ), + new MySQLMasterPos( 'db1034-bin.000876/44', $now ), + new MySQLMasterPos( 'db1034-bin.000976/74', $now ), true ], [ - new MySQLMasterPos( 'db1052-bin.000976', '999' ), - new MySQLMasterPos( 'db1052-bin.000976', '1000' ), + new MySQLMasterPos( 'db1052-bin.000976/999', $now ), + new MySQLMasterPos( 'db1052-bin.000976/1000', $now ), true ], [ - new MySQLMasterPos( 'db1066-bin.000976', '9999' ), - new MySQLMasterPos( 'db1035-bin.000976', '10000' ), + new MySQLMasterPos( 'db1066-bin.000976/9999', $now ), + new MySQLMasterPos( 'db1035-bin.000976/10000', $now ), false ], [ - new MySQLMasterPos( 'db1066-bin.000976', '9999' ), - new MySQLMasterPos( 'trump2016.000976', '10000' ), + new MySQLMasterPos( 'db1066-bin.000976/9999', $now ), + new MySQLMasterPos( 'trump2016.000976/10000', $now ), false ], ];