From: Aaron Schulz Date: Sat, 10 Oct 2015 00:22:14 +0000 (-0700) Subject: Make DB handles inherit configured read-only mode X-Git-Tag: 1.31.0-rc.0~9323^2 X-Git-Url: https://git.heureux-cyclage.org/?p=lhc%2Fweb%2Fwiklou.git;a=commitdiff_plain;h=52010e6d21d8bdefa1c89fbc9421850185cc5011 Make DB handles inherit configured read-only mode LBFactory inherits $wgReadOnly, the LBs inherit any LBFactory read only mode, and Database objects inherit any LB read-only mode. Add some methods callers can use to check if a DB/LB handle is read-only before trying writes. Additionally: * Fix 5ec1e47475 regression where readOnlyBySection read-only mode would not affect wfReadOnly() but only lagged-slave read-only mode for LBFactoryMulti. * Catch errors when getLaggedSlaveMode() is called after master connection and object is established. * Make getLaggedSlaveMode() a no-op if there are no slaves. * Make string/false logic for read-only consistent everywhere. * Remove mLaggedSlaveMode "m" prefix. Change-Id: Ice3224caae564aa5ffb41b424c23d1593229117a --- diff --git a/includes/GlobalFunctions.php b/includes/GlobalFunctions.php index e73c75cb49..cda3154253 100644 --- a/includes/GlobalFunctions.php +++ b/includes/GlobalFunctions.php @@ -1355,24 +1355,14 @@ function wfReadOnlyReason() { return $readOnly; } - static $autoReadOnly = null; - if ( $autoReadOnly === null ) { + static $lbReadOnly = null; + if ( $lbReadOnly === null ) { // Callers use this method to be aware that data presented to a user // may be very stale and thus allowing submissions can be problematic. - try { - if ( wfGetLB()->getLaggedSlaveMode() ) { - $autoReadOnly = 'The database has been automatically locked ' . - 'while the slave database servers catch up to the master'; - } else { - $autoReadOnly = false; - } - } catch ( DBConnectionError $e ) { - $autoReadOnly = 'The database has been automatically locked ' . - 'until the slave database servers become available'; - } + $lbReadOnly = wfGetLB()->getReadOnlyReason(); } - return $autoReadOnly; + return $lbReadOnly; } /** diff --git a/includes/db/Database.php b/includes/db/Database.php index 1da85d7b3c..49a53ef30c 100644 --- a/includes/db/Database.php +++ b/includes/db/Database.php @@ -927,8 +927,8 @@ abstract class DatabaseBase implements IDatabase { $isWriteQuery = $this->isWriteQuery( $sql ); if ( $isWriteQuery ) { - $reason = $this->getLBInfo( 'readOnlyReason' ); - if ( is_string( $reason ) ) { + $reason = $this->getReadOnlyReason(); + if ( $reason !== false ) { throw new DBReadOnlyError( $this, "Database is read-only: $reason" ); } # Set a flag indicating that writes have been done @@ -4286,6 +4286,16 @@ abstract class DatabaseBase implements IDatabase { // no-op } + /** + * @return string|bool Reason this DB is read-only or false if it is not + * @since 1.27 + */ + public function getReadOnlyReason() { + $reason = $this->getLBInfo( 'readOnlyReason' ); + + return is_string( $reason ) ? $reason : false; + } + /** * @since 1.19 * @return string diff --git a/includes/db/loadbalancer/LBFactory.php b/includes/db/loadbalancer/LBFactory.php index bad04f9c1f..e7b762740a 100644 --- a/includes/db/loadbalancer/LBFactory.php +++ b/includes/db/loadbalancer/LBFactory.php @@ -29,11 +29,17 @@ abstract class LBFactory { /** @var LBFactory */ private static $instance; + /** @var string|bool Reason all LBs are read-only or false if not */ + protected $readOnlyReason = false; + /** * Construct a factory based on a configuration array (typically from $wgLBFactoryConf) * @param array $conf */ public function __construct( array $conf ) { + if ( isset( $conf['readOnlyReason'] ) && is_string( $conf['readOnlyReason'] ) ) { + $this->readOnlyReason = $conf['readOnlyReason']; + } } /** @@ -55,8 +61,11 @@ abstract class LBFactory { if ( is_null( self::$instance ) ) { $class = self::getLBFactoryClass( $wgLBFactoryConf ); - - self::$instance = new $class( $wgLBFactoryConf ); + $config = $wgLBFactoryConf; + if ( !isset( $config['readOnlyReason'] ) ) { + $config['readOnlyReason'] = wfConfiguredReadOnlyReason(); + } + self::$instance = new $class( $config ); } return self::$instance; diff --git a/includes/db/loadbalancer/LBFactoryMulti.php b/includes/db/loadbalancer/LBFactoryMulti.php index 2655659b77..089dfd36fb 100644 --- a/includes/db/loadbalancer/LBFactoryMulti.php +++ b/includes/db/loadbalancer/LBFactoryMulti.php @@ -209,19 +209,27 @@ class LBFactoryMulti extends LBFactory { public function newMainLB( $wiki = false ) { list( $dbName, ) = $this->getDBNameAndPrefix( $wiki ); $section = $this->getSectionForWiki( $wiki ); - $groupLoads = array(); if ( isset( $this->groupLoadsByDB[$dbName] ) ) { $groupLoads = $this->groupLoadsByDB[$dbName]; + } else { + $groupLoads = array(); } if ( isset( $this->groupLoadsBySection[$section] ) ) { $groupLoads = array_merge_recursive( $groupLoads, $this->groupLoadsBySection[$section] ); } + $readOnlyReason = $this->readOnlyReason; + // Use the LB-specific read-only reason if everything isn't already read-only + if ( $readOnlyReason === false && isset( $this->readOnlyBySection[$section] ) ) { + $readOnlyReason = $this->readOnlyBySection[$section]; + } + return $this->newLoadBalancer( $this->serverTemplate, $this->sectionLoads[$section], - $groupLoads + $groupLoads, + $readOnlyReason ); } @@ -259,7 +267,12 @@ class LBFactoryMulti extends LBFactory { $template = $this->templateOverridesByCluster[$cluster] + $template; } - return $this->newLoadBalancer( $template, $this->externalLoads[$cluster], array() ); + return $this->newLoadBalancer( + $template, + $this->externalLoads[$cluster], + array(), + $this->readOnlyReason + ); } /** @@ -283,16 +296,15 @@ class LBFactoryMulti extends LBFactory { * @param array $template * @param array $loads * @param array $groupLoads + * @param string|bool $readOnlyReason * @return LoadBalancer */ - private function newLoadBalancer( $template, $loads, $groupLoads ) { - $servers = $this->makeServerArray( $template, $loads, $groupLoads ); - $lb = new LoadBalancer( array( - 'servers' => $servers, - 'loadMonitor' => $this->loadMonitorClass + private function newLoadBalancer( $template, $loads, $groupLoads, $readOnlyReason ) { + return new LoadBalancer( array( + 'servers' => $this->makeServerArray( $template, $loads, $groupLoads ), + 'loadMonitor' => $this->loadMonitorClass, + 'readOnlyReason' => $readOnlyReason ) ); - - return $lb; } /** diff --git a/includes/db/loadbalancer/LBFactorySimple.php b/includes/db/loadbalancer/LBFactorySimple.php index e328727daf..353c47a478 100644 --- a/includes/db/loadbalancer/LBFactorySimple.php +++ b/includes/db/loadbalancer/LBFactorySimple.php @@ -89,7 +89,8 @@ class LBFactorySimple extends LBFactory { return new LoadBalancer( array( 'servers' => $servers, - 'loadMonitor' => $this->loadMonitorClass + 'loadMonitor' => $this->loadMonitorClass, + 'readOnlyReason' => $this->readOnlyReason ) ); } @@ -121,7 +122,8 @@ class LBFactorySimple extends LBFactory { return new LoadBalancer( array( 'servers' => $wgExternalServers[$cluster], - 'loadMonitor' => $this->loadMonitorClass + 'loadMonitor' => $this->loadMonitorClass, + 'readOnlyReason' => $this->readOnlyReason ) ); } diff --git a/includes/db/loadbalancer/LBFactorySingle.php b/includes/db/loadbalancer/LBFactorySingle.php index 32bce6c8b1..5a6cfa753d 100644 --- a/includes/db/loadbalancer/LBFactorySingle.php +++ b/includes/db/loadbalancer/LBFactorySingle.php @@ -35,6 +35,7 @@ class LBFactorySingle extends LBFactory { public function __construct( array $conf ) { parent::__construct( $conf ); + $conf['readOnlyReason'] = $this->readOnlyReason; $this->lb = new LoadBalancerSingle( $conf ); } @@ -93,12 +94,21 @@ class LoadBalancerSingle extends LoadBalancer { */ public function __construct( array $params ) { $this->db = $params['connection']; - parent::__construct( array( 'servers' => array( array( - 'type' => $this->db->getType(), - 'host' => $this->db->getServer(), - 'dbname' => $this->db->getDBname(), - 'load' => 1, - ) ) ) ); + + parent::__construct( array( + 'servers' => array( + array( + 'type' => $this->db->getType(), + 'host' => $this->db->getServer(), + 'dbname' => $this->db->getDBname(), + 'load' => 1, + ) + ) + ) ); + + if ( isset( $params['readOnlyReason'] ) ) { + $this->db->setLBInfo( 'readOnlyReason', $params['readOnlyReason'] ); + } } /** diff --git a/includes/db/loadbalancer/LoadBalancer.php b/includes/db/loadbalancer/LoadBalancer.php index eda374a1b8..bc9465ba60 100644 --- a/includes/db/loadbalancer/LoadBalancer.php +++ b/includes/db/loadbalancer/LoadBalancer.php @@ -55,9 +55,13 @@ class LoadBalancer { /** @var bool|DBMasterPos False if not set */ private $mWaitForPos; /** @var bool Whether the generic reader fell back to a lagged slave */ - private $mLaggedSlaveMode; + private $laggedSlaveMode = false; + /** @var bool Whether the generic reader fell back to a lagged slave */ + private $slavesDownMode = false; /** @var string The last DB selection or connection error */ private $mLastError = 'Unknown error'; + /** @var string|bool Reason the LB is read-only or false if not */ + private $readOnlyReason = false; /** @var integer Total connections opened */ private $connsOpened = 0; @@ -68,8 +72,9 @@ class LoadBalancer { /** * @param array $params Array with keys: - * servers Required. Array of server info structures. - * loadMonitor Name of a class used to fetch server lag and load. + * - servers : Required. Array of server info structures. + * - loadMonitor : Name of a class used to fetch server lag and load. + * - readOnlyReason : Reason the master DB is read-only if so [optional] * @throws MWException */ public function __construct( array $params ) { @@ -87,10 +92,13 @@ class LoadBalancer { 'foreignFree' => array() ); $this->mLoads = array(); $this->mWaitForPos = false; - $this->mLaggedSlaveMode = false; $this->mErrorConnection = false; $this->mAllowLagged = false; + if ( isset( $params['readOnlyReason'] ) && is_string( $params['readOnlyReason'] ) ) { + $this->readOnlyReason = $params['readOnlyReason']; + } + if ( isset( $params['loadMonitor'] ) ) { $this->mLoadMonitorClass = $params['loadMonitor']; } else { @@ -154,7 +162,7 @@ class LoadBalancer { /** * @param array $loads * @param bool|string $wiki Wiki to get non-lagged for - * @param float $maxLag Restrict the maximum allowed lag to this many seconds + * @param int $maxLag Restrict the maximum allowed lag to this many seconds * @return bool|int|string */ private function getRandomNonLagged( array $loads, $wiki = false, $maxLag = self::MAX_LAG ) { @@ -329,7 +337,7 @@ class LoadBalancer { $this->mReadIndex = $i; # Record if the generic reader index is in "lagged slave" mode if ( $laggedSlaveMode ) { - $this->mLaggedSlaveMode = true; + $this->laggedSlaveMode = true; } } $serverName = $this->getServerName( $i ); @@ -352,7 +360,7 @@ class LoadBalancer { if ( $i > 0 ) { if ( !$this->doWait( $i ) ) { $this->mServers[$i]['slave pos'] = $this->getAnyOpenConnection( $i )->getSlavePos(); - $this->mLaggedSlaveMode = true; + $this->laggedSlaveMode = true; } } } @@ -548,12 +556,9 @@ class LoadBalancer { $trxProf->recordConnection( $host, $dbname, $masterOnly ); } - # Make master connections read only if in lagged slave mode - if ( $masterOnly && $this->getServerCount() > 1 && $this->getLaggedSlaveMode( $wiki ) ) { - $conn->setLBInfo( 'readOnlyReason', - 'The database has been automatically locked ' . - 'while the slave database servers catch up to the master' - ); + if ( $masterOnly ) { + # Make master-requested DB handles inherit any read-only mode setting + $conn->setLBInfo( 'readOnlyReason', $this->getReadOnlyReason( $wiki ) ); } return $conn; @@ -1147,10 +1152,19 @@ class LoadBalancer { * @return bool Whether the generic connection for reads is highly "lagged" */ public function getLaggedSlaveMode( $wiki = false ) { - # Get a generic reader connection - $this->getConnection( DB_SLAVE, false, $wiki ); + // No-op if there is only one DB (also avoids recursion) + if ( !$this->laggedSlaveMode && $this->getServerCount() > 1 ) { + try { + // See if laggedSlaveMode gets set + $this->getConnection( DB_SLAVE, false, $wiki ); + } catch ( DBConnectionError $e ) { + // Avoid expensive re-connect attempts and failures + $this->slavesDownMode = true; + $this->laggedSlaveMode = true; + } + } - return $this->mLaggedSlaveMode; + return $this->laggedSlaveMode; } /** @@ -1159,7 +1173,29 @@ class LoadBalancer { * @since 1.27 */ public function laggedSlaveUsed() { - return $this->mLaggedSlaveMode; + return $this->laggedSlaveMode; + } + + /** + * @note This method may trigger a DB connection if not yet done + * @param string|bool $wiki Wiki ID, or false for the current wiki + * @return string|bool Reason the master is read-only or false if it is not + * @since 1.27 + */ + public function getReadOnlyReason( $wiki = false ) { + if ( $this->readOnlyReason !== false ) { + return $this->readOnlyReason; + } elseif ( $this->getLaggedSlaveMode( $wiki ) ) { + if ( $this->slavesDownMode ) { + return 'The database has been automatically locked ' . + 'until the slave database servers become available'; + } else { + return 'The database has been automatically locked ' . + 'while the slave database servers catch up to the master.'; + } + } + + return false; } /**