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 e73c75c..cda3154 100644 (file)
--- 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 1da85d7..49a53ef 100644 (file)
--- 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 bad04f9..e7b7627 100644 (file)
--- 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 2655659..089dfd3 100644 (file)
--- 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 e328727..353c47a 100644 (file)
--- 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 32bce6c..5a6cfa7 100644 (file)
--- 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 eda374a..bc9465b 100644 (file)
--- 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;
        }
 
        /**