From e54317e9cb219a18e8067318a348697bb741ef77 Mon Sep 17 00:00:00 2001 From: Aaron Schulz Date: Sun, 27 Jan 2019 13:59:59 -0800 Subject: [PATCH] rdbms: move "maxLag" parameter up to LBFactory and add comments Also make the default value constant be define in only one place Change-Id: I1abd2b4569910031853b25f92d7cb69a1614d05b --- includes/libs/rdbms/lbfactory/ILBFactory.php | 1 + includes/libs/rdbms/lbfactory/LBFactory.php | 5 +++++ includes/libs/rdbms/lbfactory/LBFactoryMulti.php | 10 +--------- includes/libs/rdbms/lbfactory/LBFactorySimple.php | 7 ------- includes/libs/rdbms/loadbalancer/ILoadBalancer.php | 2 +- includes/libs/rdbms/loadbalancer/LoadBalancer.php | 6 ++---- 6 files changed, 10 insertions(+), 21 deletions(-) diff --git a/includes/libs/rdbms/lbfactory/ILBFactory.php b/includes/libs/rdbms/lbfactory/ILBFactory.php index 7987052206..98c06adf80 100644 --- a/includes/libs/rdbms/lbfactory/ILBFactory.php +++ b/includes/libs/rdbms/lbfactory/ILBFactory.php @@ -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] diff --git a/includes/libs/rdbms/lbfactory/LBFactory.php b/includes/libs/rdbms/lbfactory/LBFactory.php index 9a6c224051..9ec1ce125b 100644 --- a/includes/libs/rdbms/lbfactory/LBFactory.php +++ b/includes/libs/rdbms/lbfactory/LBFactory.php @@ -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 diff --git a/includes/libs/rdbms/lbfactory/LBFactoryMulti.php b/includes/libs/rdbms/lbfactory/LBFactoryMulti.php index cc6824d418..189ceee56d 100644 --- a/includes/libs/rdbms/lbfactory/LBFactoryMulti.php +++ b/includes/libs/rdbms/lbfactory/LBFactoryMulti.php @@ -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 ] diff --git a/includes/libs/rdbms/lbfactory/LBFactorySimple.php b/includes/libs/rdbms/lbfactory/LBFactorySimple.php index 6a6bb8d17a..49054e083d 100644 --- a/includes/libs/rdbms/lbfactory/LBFactorySimple.php +++ b/includes/libs/rdbms/lbfactory/LBFactorySimple.php @@ -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 ], ] ) ); diff --git a/includes/libs/rdbms/loadbalancer/ILoadBalancer.php b/includes/libs/rdbms/loadbalancer/ILoadBalancer.php index e1a3162cf6..b20bf04e45 100644 --- a/includes/libs/rdbms/loadbalancer/ILoadBalancer.php +++ b/includes/libs/rdbms/loadbalancer/ILoadBalancer.php @@ -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] diff --git a/includes/libs/rdbms/loadbalancer/LoadBalancer.php b/includes/libs/rdbms/loadbalancer/LoadBalancer.php index ab5c3cda44..60b3ac402d 100644 --- a/includes/libs/rdbms/loadbalancer/LoadBalancer.php +++ b/includes/libs/rdbms/loadbalancer/LoadBalancer.php @@ -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 ]; -- 2.20.1