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 208f506..c20c815 100644 (file)
--- 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 9ca6b11..e5bb2c0 100644 (file)
--- 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, <binlog file>/<integer>)
+        * @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 <binlog file>/<position>, 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 <binlog file>/<position> (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 fa1cfc9..9038e5a 100644 (file)
--- 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 caf1281..b2eabb1 100644 (file)
--- 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
                        ],
                ];