LoadBalancer object injection cleanups
authorAaron Schulz <aschulz@wikimedia.org>
Tue, 2 Aug 2016 03:00:43 +0000 (20:00 -0700)
committerKrinkle <krinklemail@gmail.com>
Mon, 8 Aug 2016 23:41:05 +0000 (23:41 +0000)
Follows-up 5f921702d2.

* Move cache object creation up to LBFactory.
* Refactored some code duplication in LBFactorySimple.

Change-Id: I0a5820f5155fc545a8bf0cc4e7c27f878388682b

includes/db/loadbalancer/LBFactory.php
includes/db/loadbalancer/LBFactoryMulti.php
includes/db/loadbalancer/LBFactorySimple.php
includes/db/loadbalancer/LBFactorySingle.php
includes/db/loadbalancer/LoadBalancer.php

index 053f9f8..4078a39 100644 (file)
@@ -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 );
                }
        }
 
index 3a543ac..10a65f8 100644 (file)
@@ -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
                ] );
        }
 
index 1b0a1f3..14baf2e 100644 (file)
@@ -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,
index 79ca3a7..14c1c28 100644 (file)
@@ -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'] ) ) {
index b44b559..868b72a 100644 (file)
@@ -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 {