From afd24909166eb3ef83b29bd1c8c22dedcc80dd26 Mon Sep 17 00:00:00 2001 From: Aaron Schulz Date: Mon, 1 Aug 2016 20:00:43 -0700 Subject: [PATCH] LoadBalancer object injection cleanups Follows-up 5f921702d2. * Move cache object creation up to LBFactory. * Refactored some code duplication in LBFactorySimple. Change-Id: I0a5820f5155fc545a8bf0cc4e7c27f878388682b --- includes/db/loadbalancer/LBFactory.php | 28 +++++++++++++++----- includes/db/loadbalancer/LBFactoryMulti.php | 4 ++- includes/db/loadbalancer/LBFactorySimple.php | 25 ++++++++--------- includes/db/loadbalancer/LBFactorySingle.php | 8 ++++-- includes/db/loadbalancer/LoadBalancer.php | 15 +++++------ 5 files changed, 49 insertions(+), 31 deletions(-) diff --git a/includes/db/loadbalancer/LBFactory.php b/includes/db/loadbalancer/LBFactory.php index 053f9f816d..4078a3926e 100644 --- a/includes/db/loadbalancer/LBFactory.php +++ b/includes/db/loadbalancer/LBFactory.php @@ -31,15 +31,16 @@ use MediaWiki\Logger\LoggerFactory; * @ingroup Database */ abstract class LBFactory implements DestructibleService { - /** @var ChronologyProtector */ protected $chronProt; - /** @var TransactionProfiler */ protected $trxProfiler; - /** @var LoggerInterface */ - protected $logger; + protected $trxLogger; + /** @var BagOStuff */ + protected $srvCache; + /** @var WANObjectCache */ + protected $wanCache; /** @var string|bool Reason all LBs are read-only or false if not */ protected $readOnlyReason = false; @@ -49,15 +50,28 @@ abstract class LBFactory implements DestructibleService { /** * Construct a factory based on a configuration array (typically from $wgLBFactoryConf) * @param array $conf + * @TODO: inject objects via dependency framework */ public function __construct( array $conf ) { if ( isset( $conf['readOnlyReason'] ) && is_string( $conf['readOnlyReason'] ) ) { $this->readOnlyReason = $conf['readOnlyReason']; } - $this->chronProt = $this->newChronologyProtector(); $this->trxProfiler = Profiler::instance()->getTransactionProfiler(); - $this->logger = LoggerFactory::getInstance( 'DBTransaction' ); + // Use APC/memcached style caching, but avoids loops with CACHE_DB (T141804) + $cache = ObjectCache::getLocalServerInstance(); + if ( $cache->getQoS( $cache::ATTR_EMULATION ) > $cache::QOS_EMULATION_SQL ) { + $this->srvCache = $cache; + } else { + $this->srvCache = new EmptyBagOStuff(); + } + $wCache = ObjectCache::getMainWANInstance(); + if ( $wCache->getQoS( $wCache::ATTR_EMULATION ) > $wCache::QOS_EMULATION_SQL ) { + $this->wanCache = $wCache; + } else { + $this->wanCache = WANObjectCache::newEmpty(); + } + $this->trxLogger = LoggerFactory::getInstance( 'DBTransaction' ); } /** @@ -261,7 +275,7 @@ abstract class LBFactory implements DestructibleService { foreach ( $callersByDB as $db => $callers ) { $msg .= "$db: " . implode( '; ', $callers ) . "\n"; } - $this->logger->info( $msg ); + $this->trxLogger->info( $msg ); } } diff --git a/includes/db/loadbalancer/LBFactoryMulti.php b/includes/db/loadbalancer/LBFactoryMulti.php index 3a543acc5b..10a65f85e3 100644 --- a/includes/db/loadbalancer/LBFactoryMulti.php +++ b/includes/db/loadbalancer/LBFactoryMulti.php @@ -317,7 +317,9 @@ class LBFactoryMulti extends LBFactory { 'servers' => $this->makeServerArray( $template, $loads, $groupLoads ), 'loadMonitor' => $this->loadMonitorClass, 'readOnlyReason' => $readOnlyReason, - 'trxProfiler' => $this->trxProfiler + 'trxProfiler' => $this->trxProfiler, + 'srvCache' => $this->srvCache, + 'wanCache' => $this->wanCache ] ); } diff --git a/includes/db/loadbalancer/LBFactorySimple.php b/includes/db/loadbalancer/LBFactorySimple.php index 1b0a1f3f55..14baf2e7b0 100644 --- a/includes/db/loadbalancer/LBFactorySimple.php +++ b/includes/db/loadbalancer/LBFactorySimple.php @@ -84,12 +84,7 @@ class LBFactorySimple extends LBFactory { ] ]; } - return new LoadBalancer( [ - 'servers' => $servers, - 'loadMonitor' => $this->loadMonitorClass, - 'readOnlyReason' => $this->readOnlyReason, - 'trxProfiler' => $this->trxProfiler - ] ); + return $this->newLoadBalancer( $servers ); } /** @@ -118,12 +113,7 @@ class LBFactorySimple extends LBFactory { throw new MWException( __METHOD__ . ": Unknown cluster \"$cluster\"" ); } - return new LoadBalancer( [ - 'servers' => $wgExternalServers[$cluster], - 'loadMonitor' => $this->loadMonitorClass, - 'readOnlyReason' => $this->readOnlyReason, - 'trxProfiler' => $this->trxProfiler - ] ); + return $this->newLoadBalancer( $wgExternalServers[$cluster] ); } /** @@ -141,6 +131,17 @@ class LBFactorySimple extends LBFactory { return $this->extLBs[$cluster]; } + private function newLoadBalancer( array $servers ) { + return new LoadBalancer( [ + 'servers' => $servers, + 'loadMonitor' => $this->loadMonitorClass, + 'readOnlyReason' => $this->readOnlyReason, + 'trxProfiler' => $this->trxProfiler, + 'srvCache' => $this->srvCache, + 'wanCache' => $this->wanCache + ] ); + } + /** * Execute a function for each tracked load balancer * The callback is called with the load balancer as the first parameter, diff --git a/includes/db/loadbalancer/LBFactorySingle.php b/includes/db/loadbalancer/LBFactorySingle.php index 79ca3a70d2..14c1c283e6 100644 --- a/includes/db/loadbalancer/LBFactorySingle.php +++ b/includes/db/loadbalancer/LBFactorySingle.php @@ -37,7 +37,9 @@ class LBFactorySingle extends LBFactory { $this->lb = new LoadBalancerSingle( [ 'readOnlyReason' => $this->readOnlyReason, - 'trxProfiler' => $this->trxProfiler + 'trxProfiler' => $this->trxProfiler, + 'srvCache' => $this->srvCache, + 'wanCache' => $this->wanCache ] + $conf ); } @@ -106,7 +108,9 @@ class LoadBalancerSingle extends LoadBalancer { 'load' => 1, ] ], - 'trxProfiler' => $this->trxProfiler + 'trxProfiler' => isset( $params['trxProfiler'] ) ? $params['trxProfiler'] : null, + 'srvCache' => isset( $params['srvCache'] ) ? $params['srvCache'] : null, + 'wanCache' => isset( $params['wanCache'] ) ? $params['wanCache'] : null ] ); if ( isset( $params['readOnlyReason'] ) ) { diff --git a/includes/db/loadbalancer/LoadBalancer.php b/includes/db/loadbalancer/LoadBalancer.php index b44b559b9e..868b72a29b 100644 --- a/includes/db/loadbalancer/LoadBalancer.php +++ b/includes/db/loadbalancer/LoadBalancer.php @@ -91,6 +91,8 @@ class LoadBalancer { * - 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] + * - srvCache : BagOStuff object [optional] + * - wanCache : WANObjectCache object [optional] * @throws MWException */ public function __construct( array $params ) { @@ -138,21 +140,16 @@ class LoadBalancer { } } - // Use APC/memcached style caching, but avoids loops with CACHE_DB (T141804) - // @TODO: inject these in via LBFactory at some point - $cache = ObjectCache::getLocalServerInstance(); - if ( $cache->getQoS( $cache::ATTR_EMULATION ) > $cache::QOS_EMULATION_SQL ) { - $this->srvCache = $cache; + if ( isset( $params['srvCache'] ) ) { + $this->srvCache = $params['srvCache']; } else { $this->srvCache = new EmptyBagOStuff(); } - $wCache = ObjectCache::getMainWANInstance(); - if ( $wCache->getQoS( $wCache::ATTR_EMULATION ) > $wCache::QOS_EMULATION_SQL ) { - $this->wanCache = $wCache; + if ( isset( $params['wanCache'] ) ) { + $this->wanCache = $params['wanCache']; } else { $this->wanCache = WANObjectCache::newEmpty(); } - if ( isset( $params['trxProfiler'] ) ) { $this->trxProfiler = $params['trxProfiler']; } else { -- 2.20.1