rdbms: move "maxLag" parameter up to LBFactory and add comments
authorAaron Schulz <aschulz@wikimedia.org>
Sun, 27 Jan 2019 21:59:59 +0000 (13:59 -0800)
committerAaron Schulz <aschulz@wikimedia.org>
Tue, 5 Feb 2019 23:59:44 +0000 (15:59 -0800)
Also make the default value constant be define in only one place

Change-Id: I1abd2b4569910031853b25f92d7cb69a1614d05b

includes/libs/rdbms/lbfactory/ILBFactory.php
includes/libs/rdbms/lbfactory/LBFactory.php
includes/libs/rdbms/lbfactory/LBFactoryMulti.php
includes/libs/rdbms/lbfactory/LBFactorySimple.php
includes/libs/rdbms/loadbalancer/ILoadBalancer.php
includes/libs/rdbms/loadbalancer/LoadBalancer.php

index 7987052..98c06ad 100644 (file)
@@ -48,6 +48,7 @@ interface ILBFactory {
         *  - wanCache: WANObjectCache object [optional]
         *  - hostname: The name of the current server [optional]
         *  - cliMode: Whether the execution context is a CLI script. [optional]
+        *  - maxLag: Try to avoid DB replicas with lag above this many seconds [optional]
         *  - profiler: Class name or instance with profileIn()/profileOut() methods. [optional]
         *  - trxProfiler: TransactionProfiler instance. [optional]
         *  - replLogger: PSR-3 logger instance. [optional]
index 9a6c224..9ec1ce1 100644 (file)
@@ -95,6 +95,9 @@ abstract class LBFactory implements ILBFactory {
        /** @var string|null */
        private $defaultGroup = null;
 
+       /** @var int|null */
+       protected $maxLag;
+
        const ROUND_CURSORY = 'cursory';
        const ROUND_BEGINNING = 'within-begin';
        const ROUND_COMMITTING = 'within-commit';
@@ -110,6 +113,7 @@ abstract class LBFactory implements ILBFactory {
                        ? DatabaseDomain::newFromId( $conf['localDomain'] )
                        : DatabaseDomain::newUnspecified();
 
+               $this->maxLag = $conf['maxLag'] ?? null;
                if ( isset( $conf['readOnlyReason'] ) && is_string( $conf['readOnlyReason'] ) ) {
                        $this->readOnlyReason = $conf['readOnlyReason'];
                }
@@ -588,6 +592,7 @@ abstract class LBFactory implements ILBFactory {
                        'hostname' => $this->hostname,
                        'cliMode' => $this->cliMode,
                        'agent' => $this->agent,
+                       'maxLag' => $this->maxLag,
                        'defaultGroup' => $this->defaultGroup,
                        'chronologyCallback' => function ( ILoadBalancer $lb ) {
                                // Defer ChronologyProtector construction in case setRequestInfo() ends up
index cc6824d..189ceee 100644 (file)
@@ -107,12 +107,6 @@ class LBFactoryMulti extends LBFactory {
        /** @var string */
        private $lastSection;
 
-       /** @var int */
-       private $maxLag = self::MAX_LAG_DEFAULT;
-
-       /** @var int Default 'maxLag' when unspecified */
-       const MAX_LAG_DEFAULT = 10;
-
        /**
         * @see LBFactory::__construct()
         *
@@ -166,7 +160,6 @@ class LBFactoryMulti extends LBFactory {
         *                                 storage cluster.
         *   - masterTemplateOverrides     Server configuration map overrides for all master servers.
         *   - loadMonitorClass            Name of the LoadMonitor class to always use.
-        *   - maxLag                      Avoid replica DBs with more lag than this many seconds.
         *   - readOnlyBySection           A map of section name to read-only message.
         *                                 Missing or false for read/write.
         */
@@ -178,7 +171,7 @@ class LBFactoryMulti extends LBFactory {
                $optional = [ 'groupLoadsBySection', 'groupLoadsByDB', 'hostsByName',
                        'externalLoads', 'externalTemplateOverrides', 'templateOverridesByServer',
                        'templateOverridesByCluster', 'templateOverridesBySection', 'masterTemplateOverrides',
-                       'readOnlyBySection', 'maxLag', 'loadMonitorClass' ];
+                       'readOnlyBySection', 'loadMonitorClass' ];
 
                foreach ( $required as $key ) {
                        if ( !isset( $conf[$key] ) ) {
@@ -318,7 +311,6 @@ class LBFactoryMulti extends LBFactory {
                        $this->baseLoadBalancerParams(),
                        [
                                'servers' => $this->makeServerArray( $template, $loads, $groupLoads ),
-                               'maxLag' => $this->maxLag,
                                'loadMonitor' => [ 'class' => $this->loadMonitorClass ],
                                'readOnlyReason' => $readOnlyReason
                        ]
index 6a6bb8d..49054e0 100644 (file)
@@ -41,11 +41,6 @@ class LBFactorySimple extends LBFactory {
 
        /** @var string */
        private $loadMonitorClass;
-       /** @var int */
-       private $maxLag;
-
-       /** @var int Default 'maxLag' when unspecified */
-       const MAX_LAG_DEFAULT = 10;
 
        /**
         * @see LBFactory::__construct()
@@ -73,7 +68,6 @@ class LBFactorySimple extends LBFactory {
 
                $this->externalClusters = $conf['externalClusters'] ?? [];
                $this->loadMonitorClass = $conf['loadMonitorClass'] ?? 'LoadMonitor';
-               $this->maxLag = $conf['maxLag'] ?? self::MAX_LAG_DEFAULT;
        }
 
        /**
@@ -130,7 +124,6 @@ class LBFactorySimple extends LBFactory {
                        $this->baseLoadBalancerParams(),
                        [
                                'servers' => $servers,
-                               'maxLag' => $this->maxLag,
                                'loadMonitor' => [ 'class' => $this->loadMonitorClass ],
                        ]
                ) );
index e1a3162..b20bf04 100644 (file)
@@ -101,7 +101,7 @@ interface ILoadBalancer {
         *  - loadMonitor : Name of a class used to fetch server lag and load.
         *  - readOnlyReason : Reason the master DB is read-only if so [optional]
         *  - waitTimeout : Maximum time to wait for replicas for consistency [optional]
-        *  - maxLag: Avoid replica DB servers with more lag than this [optional]
+        *  - maxLag: Try to avoid DB replicas with lag above this many seconds [optional]
         *  - srvCache : BagOStuff object for server cache [optional]
         *  - wanCache : WANObjectCache object [optional]
         *  - chronologyCallback: Callback to run before the first connection attempt [optional]
index ab5c3cd..60b3ac4 100644 (file)
@@ -85,7 +85,7 @@ class LoadBalancer implements ILoadBalancer {
        /** @var string Alternate ID string for the domain instead of DatabaseDomain::getId() */
        private $localDomainIdAlias;
        /** @var int */
-       private $maxLag = self::MAX_LAG_DEFAULT;
+       private $maxLag;
 
        /** @var string Current server name */
        private $hostname;
@@ -200,9 +200,7 @@ class LoadBalancer implements ILoadBalancer {
                        $this->readOnlyReason = $params['readOnlyReason'];
                }
 
-               if ( isset( $params['maxLag'] ) ) {
-                       $this->maxLag = $params['maxLag'];
-               }
+               $this->maxLag = $params['maxLag'] ?? self::MAX_LAG_DEFAULT;
 
                $this->loadMonitorConfig = $params['loadMonitor'] ?? [ 'class' => 'LoadMonitorNull' ];
                $this->loadMonitorConfig += [ 'lagWarnThreshold' => $this->maxLag ];