From fedfee628c377eeea0453ed82af02b6878bd525b Mon Sep 17 00:00:00 2001 From: Aaron Schulz Date: Tue, 22 Dec 2015 11:21:27 -0800 Subject: [PATCH] Deprecate wfWaitForSlaves() with LBFactory::waitForReplication() This has a cleaner interface and makes failure more explicit Change-Id: I5480845196383df85ba7538e15e507fa1b64948a --- autoload.php | 1 + includes/GlobalFunctions.php | 52 +++++---------- includes/db/loadbalancer/LBFactory.php | 90 ++++++++++++++++++++++++++ 3 files changed, 107 insertions(+), 36 deletions(-) diff --git a/autoload.php b/autoload.php index 8c5ec81731..c43b1812f7 100644 --- a/autoload.php +++ b/autoload.php @@ -289,6 +289,7 @@ $wgAutoloadLocalClasses = array( 'DBMasterPos' => __DIR__ . '/includes/db/DatabaseUtility.php', 'DBQueryError' => __DIR__ . '/includes/db/DatabaseError.php', 'DBReadOnlyError' => __DIR__ . '/includes/db/DatabaseError.php', + 'DBReplicationWaitError' => __DIR__ . '/includes/db/loadbalancer/LBFactory.php', 'DBSiteStore' => __DIR__ . '/includes/site/DBSiteStore.php', 'DBTransactionError' => __DIR__ . '/includes/db/DatabaseError.php', 'DBUnexpectedError' => __DIR__ . '/includes/db/DatabaseError.php', diff --git a/includes/GlobalFunctions.php b/includes/GlobalFunctions.php index e30b3715cf..62da135fc0 100644 --- a/includes/GlobalFunctions.php +++ b/includes/GlobalFunctions.php @@ -3683,55 +3683,35 @@ function wfGetNull() { * @param string|bool $cluster Cluster name accepted by LBFactory. Default: false. * @param int|null $timeout Max wait time. Default: 1 day (cli), ~10 seconds (web) * @return bool Success (able to connect and no timeouts reached) + * @deprecated since 1.27 Use LBFactory::waitForReplication */ function wfWaitForSlaves( $ifWritesSince = null, $wiki = false, $cluster = false, $timeout = null ) { - // B/C: first argument used to be "max seconds of lag"; ignore such values - $ifWritesSince = ( $ifWritesSince > 1e9 ) ? $ifWritesSince : null; - if ( $timeout === null ) { $timeout = ( PHP_SAPI === 'cli' ) ? 86400 : 10; } - // Figure out which clusters need to be checked - /** @var LoadBalancer[] $lbs */ - $lbs = array(); if ( $cluster === '*' ) { - wfGetLBFactory()->forEachLB( function ( LoadBalancer $lb ) use ( &$lbs ) { - $lbs[] = $lb; - } ); - } elseif ( $cluster !== false ) { - $lbs[] = wfGetLBFactory()->getExternalLB( $cluster ); - } else { - $lbs[] = wfGetLB( $wiki ); - } - - // Get all the master positions of applicable DBs right now. - // This can be faster since waiting on one cluster reduces the - // time needed to wait on the next clusters. - $masterPositions = array_fill( 0, count( $lbs ), false ); - foreach ( $lbs as $i => $lb ) { - if ( $lb->getServerCount() <= 1 ) { - // Bug 27975 - Don't try to wait for slaves if there are none - // Prevents permission error when getting master position - continue; - } elseif ( $ifWritesSince && $lb->lastMasterChangeTimestamp() < $ifWritesSince ) { - continue; // no writes since the last wait - } - $masterPositions[$i] = $lb->getMasterPos(); + $cluster = false; + $wiki = false; + } elseif ( $wiki === false ) { + $wiki = wfWikiID(); } - $ok = true; - foreach ( $lbs as $i => $lb ) { - if ( $masterPositions[$i] ) { - // The DBMS may not support getMasterPos() or the whole - // load balancer might be fake (e.g. $wgAllDBsAreLocalhost). - $ok = $lb->waitForAll( $masterPositions[$i], $timeout ) && $ok; - } + try { + wfGetLBFactory()->waitForReplication( array( + 'wiki' => $wiki, + 'cluster' => $cluster, + 'timeout' => $timeout, + // B/C: first argument used to be "max seconds of lag"; ignore such values + 'ifWritesSince' => ( $ifWritesSince > 1e9 ) ? $ifWritesSince : null + ) ); + } catch ( DBReplicationWaitError $e ) { + return false; } - return $ok; + return true; } /** diff --git a/includes/db/loadbalancer/LBFactory.php b/includes/db/loadbalancer/LBFactory.php index eeeca6283c..06b37b2833 100644 --- a/includes/db/loadbalancer/LBFactory.php +++ b/includes/db/loadbalancer/LBFactory.php @@ -263,6 +263,90 @@ abstract class LBFactory { return $ret; } + /** + * Waits for the slave DBs to catch up to the current master position + * + * Use this when updating very large numbers of rows, as in maintenance scripts, + * to avoid causing too much lag. Of course, this is a no-op if there are no slaves. + * + * By default this waits on all DB clusters actually used in this request. + * This makes sense when lag being waiting on is caused by the code that does this check. + * In that case, setting "ifWritesSince" can avoid the overhead of waiting for clusters + * that were not changed since the last wait check. To forcefully wait on a specific cluster + * for a given wiki, use the 'wiki' parameter. To forcefully wait on an "external" cluster, + * use the "cluster" parameter. + * + * Never call this function after a large DB write that is *still* in a transaction. + * It only makes sense to call this after the possible lag inducing changes were committed. + * + * @param array $opts Optional fields that include: + * - wiki : wait on the load balancer DBs that handles the given wiki + * - cluster : wait on the given external load balancer DBs + * - timeout : Max wait time. Default: ~60 seconds + * - ifWritesSince: Only wait if writes were done since this UNIX timestamp + * @throws DBReplicationWaitError If a timeout or error occured waiting on a DB cluster + * @since 1.27 + */ + public function waitForReplication( array $opts = array() ) { + $opts += array( + 'wiki' => false, + 'cluster' => false, + 'timeout' => 60, + 'ifWritesSince' => null + ); + + // Figure out which clusters need to be checked + /** @var LoadBalancer[] $lbs */ + $lbs = array(); + if ( $opts['cluster'] !== false ) { + $lbs[] = $this->getExternalLB( $opts['cluster'] ); + } elseif ( $opts['wiki'] !== false ) { + $lbs[] = $this->getMainLB( $opts['wiki'] ); + } else { + $this->forEachLB( function ( LoadBalancer $lb ) use ( &$lbs ) { + $lbs[] = $lb; + } ); + if ( !$lbs ) { + return; // nothing actually used + } + } + + // Get all the master positions of applicable DBs right now. + // This can be faster since waiting on one cluster reduces the + // time needed to wait on the next clusters. + $masterPositions = array_fill( 0, count( $lbs ), false ); + foreach ( $lbs as $i => $lb ) { + if ( $lb->getServerCount() <= 1 ) { + // Bug 27975 - Don't try to wait for slaves if there are none + // Prevents permission error when getting master position + continue; + } elseif ( $opts['ifWritesSince'] + && $lb->lastMasterChangeTimestamp() < $opts['ifWritesSince'] + ) { + continue; // no writes since the last wait + } + $masterPositions[$i] = $lb->getMasterPos(); + } + + $failed = array(); + foreach ( $lbs as $i => $lb ) { + if ( $masterPositions[$i] ) { + // The DBMS may not support getMasterPos() or the whole + // load balancer might be fake (e.g. $wgAllDBsAreLocalhost). + if ( !$lb->waitForAll( $masterPositions[$i], $opts['timeout'] ) ) { + $failed[] = $lb->getServerName( $lb->getWriterIndex() ); + } + } + } + + if ( $failed ) { + throw new DBReplicationWaitError( + "Could not wait for slaves to catch up to " . + implode( ', ', $failed ) + ); + } + } + /** * Disable the ChronologyProtector for all load balancers * @@ -329,3 +413,9 @@ class DBAccessError extends MWException { "This is not allowed." ); } } + +/** + * Exception class for replica DB wait timeouts + */ +class DBReplicationWaitError extends Exception { +} -- 2.20.1