From 9b5b407a1b1b4ac152c9e7eb4812943c74d62564 Mon Sep 17 00:00:00 2001 From: Aaron Schulz Date: Fri, 9 Feb 2018 15:01:40 -0800 Subject: [PATCH] rdbms: make DatabaseMysql::masterPosWait() handle inactive GTIDs Change-Id: I543deef24f6cbf99094a4f3bee7cabe768fa221a --- .../libs/rdbms/database/DatabaseMysqlBase.php | 12 +++- .../database/position/MySQLMasterPos.php | 68 +++++++++++++++---- .../rdbms/database/DatabaseMysqlBaseTest.php | 32 +++++++++ 3 files changed, 94 insertions(+), 18 deletions(-) diff --git a/includes/libs/rdbms/database/DatabaseMysqlBase.php b/includes/libs/rdbms/database/DatabaseMysqlBase.php index 3085276c64..57b75445ef 100644 --- a/includes/libs/rdbms/database/DatabaseMysqlBase.php +++ b/includes/libs/rdbms/database/DatabaseMysqlBase.php @@ -892,9 +892,15 @@ abstract class DatabaseMysqlBase extends Database { } // Call doQuery() directly, to avoid opening a transaction if DBO_TRX is set - if ( $pos->gtids ) { + if ( $pos->getGTIDs() ) { + // Ignore GTIDs from domains exclusive to the master DB (presumably inactive) + $rpos = $this->getReplicaPos(); + $gtidsWait = $rpos ? MySQLMasterPos::getCommonDomainGTIDs( $pos, $rpos ) : []; + if ( !$gtidsWait ) { + return -1; // $pos is from the wrong cluster? + } // Wait on the GTID set (MariaDB only) - $gtidArg = $this->addQuotes( implode( ',', $pos->gtids ) ); + $gtidArg = $this->addQuotes( implode( ',', $gtidsWait ) ); $res = $this->doQuery( "SELECT MASTER_GTID_WAIT($gtidArg, $timeout)" ); } else { // Wait on the binlog coordinates @@ -912,7 +918,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 ( !$pos->gtids ) { + if ( !$pos->getGTIDs() ) { // 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 diff --git a/includes/libs/rdbms/database/position/MySQLMasterPos.php b/includes/libs/rdbms/database/position/MySQLMasterPos.php index 62809cd4a8..2ee9068446 100644 --- a/includes/libs/rdbms/database/position/MySQLMasterPos.php +++ b/includes/libs/rdbms/database/position/MySQLMasterPos.php @@ -32,9 +32,15 @@ class MySQLMasterPos implements DBMasterPos { $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 ) ); + $gtids = array_filter( array_map( 'trim', explode( ',', $position ) ) ); + foreach ( $gtids as $gtid ) { + if ( !$this->parseGTID( $gtid ) ) { + throw new InvalidArgumentException( "Invalid GTID '$gtid'." ); + } + $this->gtids[] = $gtid; + } if ( !$this->gtids ) { - throw new InvalidArgumentException( "GTID set should not be empty." ); + throw new InvalidArgumentException( "Got empty GTID set." ); } } @@ -109,6 +115,13 @@ class MySQLMasterPos implements DBMasterPos { return $this->gtids ? null : "{$this->binlog}.{$this->pos[0]}"; } + /** + * @return string[] + */ + public function getGTIDs() { + return $this->gtids; + } + /** * @return string GTID set or / (e.g db1034-bin.000976/843431247) */ @@ -119,7 +132,25 @@ class MySQLMasterPos implements DBMasterPos { } /** - * @note: this returns false for multi-source replication GTID sets + * @param MySQLMasterPos $pos + * @param MySQLMasterPos $refPos + * @return string[] List of GTIDs from $pos that have domains in $refPos + */ + public static function getCommonDomainGTIDs( MySQLMasterPos $pos, MySQLMasterPos $refPos ) { + $gtidsCommon = []; + + $relevantDomains = $refPos->getGtidCoordinates(); // (domain => unused) + foreach ( $pos->gtids as $gtid ) { + list( $domain ) = self::parseGTID( $gtid ); + if ( isset( $relevantDomains[$domain] ) ) { + $gtidsCommon[] = $gtid; + } + } + + return $gtidsCommon; + } + + /** * @see https://mariadb.com/kb/en/mariadb/gtid * @see https://dev.mysql.com/doc/refman/5.6/en/replication-gtids-concepts.html * @return array Map of (domain => integer position); possibly empty @@ -127,23 +158,30 @@ class MySQLMasterPos implements DBMasterPos { protected function getGtidCoordinates() { $gtidInfos = []; foreach ( $this->gtids as $gtid ) { - $m = []; - // MariaDB style: -- - if ( preg_match( '!^(\d+)-\d+-(\d+)$!', $gtid, $m ) ) { - $gtidInfos[(int)$m[1]] = (int)$m[2]; - // MySQL style: : - } elseif ( preg_match( '!^(\w{8}-\w{4}-\w{4}-\w{4}-\w{12}):(\d+)$!', $gtid, $m ) ) { - $gtidInfos[$m[1]] = (int)$m[2]; - } else { - $gtidInfos = []; - break; // unrecognized GTID - } - + list( $domain, $pos ) = self::parseGTID( $gtid ); + $gtidInfos[$domain] = $pos; } return $gtidInfos; } + /** + * @param string $gtid + * @return array|null [domain, integer position] or null + */ + protected static function parseGTID( $gtid ) { + $m = []; + if ( preg_match( '!^(\d+)-\d+-(\d+)$!', $gtid, $m ) ) { + // MariaDB style: -- + return [ (int)$m[1], (int)$m[2] ]; + } elseif ( preg_match( '!^(\w{8}-\w{4}-\w{4}-\w{4}-\w{12}):(\d+)$!', $gtid, $m ) ) { + // MySQL style: : + return [ $m[1], (int)$m[2] ]; + } + + return null; + } + /** * @see https://dev.mysql.com/doc/refman/5.7/en/show-master-status.html * @see https://dev.mysql.com/doc/refman/5.7/en/show-slave-status.html diff --git a/tests/phpunit/includes/libs/rdbms/database/DatabaseMysqlBaseTest.php b/tests/phpunit/includes/libs/rdbms/database/DatabaseMysqlBaseTest.php index ce067aaa33..54c7d52c26 100644 --- a/tests/phpunit/includes/libs/rdbms/database/DatabaseMysqlBaseTest.php +++ b/tests/phpunit/includes/libs/rdbms/database/DatabaseMysqlBaseTest.php @@ -390,6 +390,38 @@ class DatabaseMysqlBaseTest extends PHPUnit_Framework_TestCase { ]; } + /** + * @dataProvider provideCommonDomainGTIDs + * @covers Wikimedia\Rdbms\MySQLMasterPos + */ + public function testCommonGtidDomains( MySQLMasterPos $pos, MySQLMasterPos $ref, $gtids ) { + $this->assertEquals( $gtids, MySQLMasterPos::getCommonDomainGTIDs( $pos, $ref ) ); + } + + public static function provideCommonDomainGTIDs() { + return [ + [ + new MySQLMasterPos( '255-13-99,256-12-50,257-14-50', 1 ), + new MySQLMasterPos( '255-11-1000', 1 ), + [ '255-13-99' ] + ], + [ + new MySQLMasterPos( + '2E11FA47-71CA-11E1-9E33-C80AA9429562:5,' . + '3E11FA47-71CA-11E1-9E33-C80AA9429562:99,' . + '7E11FA47-71CA-11E1-9E33-C80AA9429562:30', + 1 + ), + new MySQLMasterPos( + '1E11FA47-71CA-11E1-9E33-C80AA9429562:100,' . + '3E11FA47-71CA-11E1-9E33-C80AA9429562:66', + 1 + ), + [ '3E11FA47-71CA-11E1-9E33-C80AA9429562:99' ] + ] + ]; + } + /** * @dataProvider provideLagAmounts * @covers Wikimedia\Rdbms\DatabaseMysqlBase::getLag -- 2.20.1