jenkins-bot has submitted this change and it was merged.
Change subject: Fixes to masterPosWait() for master switchovers
......................................................................
Fixes to masterPosWait() for master switchovers
* Clean up return value types and docs.
* Handle master switch-over better w.r.t the job queue due
to binlog name changes (the host portion). Previously the method
would fail and trigger read-only mode when waiting on former
master positions. Assume the the switch-over was done properly
and thus return immediately.
Bug: T126436
Change-Id: Ib8c05a5c72d03a5c98e41b23c5653fc194b6d130
---
M includes/db/DatabaseMysqlBase.php
M includes/db/DatabaseUtility.php
M includes/db/IDatabase.php
M tests/phpunit/includes/db/DatabaseMysqlBaseTest.php
4 files changed, 116 insertions(+), 33 deletions(-)
Approvals:
Gilles: Looks good to me, approved
jenkins-bot: Verified
diff --git a/includes/db/DatabaseMysqlBase.php
b/includes/db/DatabaseMysqlBase.php
index 7058061..1e27205 100644
--- a/includes/db/DatabaseMysqlBase.php
+++ b/includes/db/DatabaseMysqlBase.php
@@ -757,19 +757,13 @@
return $approxLag;
}
- /**
- * Wait for the slave to catch up to a given master position.
- * @todo Return values for this and base class are rubbish
- *
- * @param DBMasterPos|MySQLMasterPos $pos
- * @param int $timeout The maximum number of seconds to wait for
synchronisation
- * @return int Zero if the slave was past that position already,
- * greater than zero if we waited for some period of time, less than
- * zero if we timed out.
- */
function masterPosWait( DBMasterPos $pos, $timeout ) {
+ if ( !( $pos instanceof MySQLMasterPos ) ) {
+ throw new InvalidArgumentException( "Position not an
instance of MySQLMasterPos" );
+ }
+
if ( $this->lastKnownSlavePos &&
$this->lastKnownSlavePos->hasReached( $pos ) ) {
- return '0'; //
http://dev.mysql.com/doc/refman/5.0/en/miscellaneous-functions.html
+ return 0;
}
# Commit any open transactions
@@ -778,18 +772,28 @@
# Call doQuery() directly, to avoid opening a transaction if
DBO_TRX is set
$encFile = $this->addQuotes( $pos->file );
$encPos = intval( $pos->pos );
- $sql = "SELECT MASTER_POS_WAIT($encFile, $encPos, $timeout)";
- $res = $this->doQuery( $sql );
+ $res = $this->doQuery( "SELECT MASTER_POS_WAIT($encFile,
$encPos, $timeout)" );
- $status = false;
- if ( $res ) {
- $row = $this->fetchRow( $res );
- if ( $row ) {
- $status = $row[0]; // can be NULL, -1, or 0+
per the MySQL manual
- if ( ctype_digit( $status ) ) { // success
- $this->lastKnownSlavePos = $pos;
- }
+ $row = $res ? $this->fetchRow( $res ) : false;
+ if ( !$row ) {
+ throw new DBExpectedError( $this, "Failed to query
MASTER_POS_WAIT()" );
+ }
+
+ // Result can be NULL (error), -1 (timeout), or 0+ per the
MySQL manual
+ $status = ( $row[0] !== null ) ? intval( $row[0] ) : null;
+ if ( $status === null ) {
+ // T126436: jobs programmed to wait on master positions
might be referencing binlogs
+ // with an old master hostname. Such calls make
MASTER_POS_WAIT() return null. Try
+ // to detect this and treat the slave as having reached
the position; a proper master
+ // switchover already requires that the new master be
caught up before the switch.
+ $slavePos = $this->getSlavePos();
+ if ( $slavePos && !$slavePos->channelsMatch( $pos ) ) {
+ $this->lastKnownSlavePos = $slavePos;
+ $status = 0;
}
+ } elseif ( $status >= 0 ) {
+ // Remember that this position was reached to save
queries next time
+ $this->lastKnownSlavePos = $pos;
}
return $status;
@@ -1446,12 +1450,35 @@
return ( $thisPos && $thatPos && $thisPos >= $thatPos );
}
+ function channelsMatch( DBMasterPos $pos ) {
+ if ( !( $pos instanceof self ) ) {
+ throw new InvalidArgumentException( "Position not an
instance of " . __CLASS__ );
+ }
+
+ $thisBinlog = $this->getBinlogName();
+ $thatBinlog = $pos->getBinlogName();
+
+ return ( $thisBinlog !== false && $thisBinlog === $thatBinlog );
+ }
+
function __toString() {
// e.g db1034-bin.000976/843431247
return "{$this->file}/{$this->pos}";
}
/**
+ * @return string|bool
+ */
+ protected function getBinlogName() {
+ $m = [];
+ if ( preg_match( '!^(.+)\.(\d+)/(\d+)$!', (string)$this, $m ) )
{
+ return $m[1];
+ }
+
+ return false;
+ }
+
+ /**
* @return array|bool (int, int)
*/
protected function getCoordinates() {
diff --git a/includes/db/DatabaseUtility.php b/includes/db/DatabaseUtility.php
index 6fa8bf0..b6c37ee 100644
--- a/includes/db/DatabaseUtility.php
+++ b/includes/db/DatabaseUtility.php
@@ -333,6 +333,13 @@
public function hasReached( DBMasterPos $pos );
/**
+ * @param DBMasterPos $pos
+ * @return bool Whether this position appears to be for the same
channel as another
+ * @since 1.27
+ */
+ public function channelsMatch( DBMasterPos $pos );
+
+ /**
* @return string
* @since 1.27
*/
diff --git a/includes/db/IDatabase.php b/includes/db/IDatabase.php
index e1d1173..225122d 100644
--- a/includes/db/IDatabase.php
+++ b/includes/db/IDatabase.php
@@ -1183,14 +1183,13 @@
public function wasReadOnlyError();
/**
- * Wait for the slave to catch up to a given master position.
+ * Wait for the slave to catch up to a given master position
*
* @param DBMasterPos $pos
- * @param int $timeout The maximum number of seconds to wait for
- * synchronisation
- * @return int Zero if the slave was past that position already,
+ * @param int $timeout The maximum number of seconds to wait for
synchronisation
+ * @return int|null Zero if the slave was past that position already,
* greater than zero if we waited for some period of time, less than
- * zero if we timed out.
+ * zero if it timed out, and null on error
*/
public function masterPosWait( DBMasterPos $pos, $timeout );
diff --git a/tests/phpunit/includes/db/DatabaseMysqlBaseTest.php
b/tests/phpunit/includes/db/DatabaseMysqlBaseTest.php
index 8e8002c..168b2c6 100644
--- a/tests/phpunit/includes/db/DatabaseMysqlBaseTest.php
+++ b/tests/phpunit/includes/db/DatabaseMysqlBaseTest.php
@@ -247,14 +247,64 @@
];
}
- function testMasterPos() {
- $pos1 = new MySQLMasterPos( 'db1034-bin.000976', '843431247' );
- $pos2 = new MySQLMasterPos( 'db1034-bin.000976', '843431248' );
+ /**
+ * @dataProvider provideComparePositions
+ */
+ function testHasReached( MySQLMasterPos $lowerPos, MySQLMasterPos
$higherPos ) {
+ $this->assertTrue( $higherPos->hasReached( $lowerPos ) );
+ $this->assertTrue( $higherPos->hasReached( $higherPos ) );
+ $this->assertTrue( $lowerPos->hasReached( $lowerPos ) );
+ $this->assertFalse( $lowerPos->hasReached( $higherPos ) );
+ }
- $this->assertTrue( $pos1->hasReached( $pos1 ) );
- $this->assertTrue( $pos2->hasReached( $pos2 ) );
- $this->assertTrue( $pos2->hasReached( $pos1 ) );
- $this->assertFalse( $pos1->hasReached( $pos2 ) );
+ function provideComparePositions() {
+ return [
+ [
+ new MySQLMasterPos( 'db1034-bin.000976',
'843431247' ),
+ new MySQLMasterPos( 'db1034-bin.000976',
'843431248' )
+ ],
+ [
+ new MySQLMasterPos( 'db1034-bin.000976', '999'
),
+ new MySQLMasterPos( 'db1034-bin.000976', '1000'
)
+ ],
+ [
+ new MySQLMasterPos( 'db1034-bin.000976', '999'
),
+ new MySQLMasterPos( 'db1035-bin.000976', '1000'
)
+ ],
+ ];
+ }
+
+ /**
+ * @dataProvider provideChannelPositions
+ */
+ function testChannelsMatch( MySQLMasterPos $pos1, MySQLMasterPos $pos2,
$matches ) {
+ $this->assertEquals( $matches, $pos1->channelsMatch( $pos2 ) );
+ $this->assertEquals( $matches, $pos2->channelsMatch( $pos1 ) );
+ }
+
+ function provideChannelPositions() {
+ return [
+ [
+ new MySQLMasterPos( 'db1034-bin.000876', '44' ),
+ new MySQLMasterPos( 'db1034-bin.000976', '74' ),
+ true
+ ],
+ [
+ new MySQLMasterPos( 'db1052-bin.000976', '999'
),
+ new MySQLMasterPos( 'db1052-bin.000976', '1000'
),
+ true
+ ],
+ [
+ new MySQLMasterPos( 'db1066-bin.000976', '9999'
),
+ new MySQLMasterPos( 'db1035-bin.000976',
'10000' ),
+ false
+ ],
+ [
+ new MySQLMasterPos( 'db1066-bin.000976', '9999'
),
+ new MySQLMasterPos( 'trump2016.000976', '10000'
),
+ false
+ ],
+ ];
}
/**
--
To view, visit https://gerrit.wikimedia.org/r/271427
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Ib8c05a5c72d03a5c98e41b23c5653fc194b6d130
Gerrit-PatchSet: 5
Gerrit-Project: mediawiki/core
Gerrit-Branch: master
Gerrit-Owner: Aaron Schulz <[email protected]>
Gerrit-Reviewer: Aaron Schulz <[email protected]>
Gerrit-Reviewer: Gilles <[email protected]>
Gerrit-Reviewer: Jcrespo <[email protected]>
Gerrit-Reviewer: Krinkle <[email protected]>
Gerrit-Reviewer: Ori.livneh <[email protected]>
Gerrit-Reviewer: Parent5446 <[email protected]>
Gerrit-Reviewer: Tim Starling <[email protected]>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
