Various dependency injection cleanups to LoadBalancer

* Inject wfWikiID() and MWExceptionHandler into LoadBalancer.
* Factor out LBFactory duplication into baseLoadBalancerParams().
* Remove $wgDBtype hack. Presumably, sites with others DBs would
  not have multiple servers configured if does not work anyway.
* Make use of injected TransactionProfiler rather than calling
  Profiler::instance()->getTransactionProfiler().
* Avoid use of trivial wfSplitWikiID() function.
* Make DBConnRef enforce its arguments more strongly and
  optimize getWiki() to avoid causing a connection attempt.
* Avoid deprecated method call in LBFactory::destroyInstance().

Change-Id: If134b62d4f48cd68cb48ccbe149e72f12aa26819

diff --git a/includes/db/CloneDatabase.php b/includes/db/CloneDatabase.php
index 577c98d..cab9438 100644 (file)
--- a/includes/db/CloneDatabase.php
+++ b/includes/db/CloneDatabase.php
@@ -129,8 +129,9 @@ class CloneDatabase {
         */
        public static function changePrefix( $prefix ) {
                global $wgDBprefix;
-               wfGetLBFactory()->forEachLB( function( $lb ) use ( $prefix ) {
-                       $lb->forEachOpenConnection( function ( $db ) use ( $prefix ) {
+               wfGetLBFactory()->forEachLB( function( LoadBalancer $lb ) use ( $prefix ) {
+                       $lb->setDomainPrefix( $prefix );
+                       $lb->forEachOpenConnection( function ( DatabaseBase $db ) use ( $prefix ) {
                                $db->tablePrefix( $prefix );
                        } );
                } );

diff --git a/includes/db/DBConnRef.php b/includes/db/DBConnRef.php
index 9997f2c..8604295 100644 (file)
--- a/includes/db/DBConnRef.php
+++ b/includes/db/DBConnRef.php
@@ -17,16 +17,22 @@ class DBConnRef implements IDatabase {
        /** @var array|null */
        private $params;
 
+       const FLD_INDEX = 0;
+       const FLD_GROUP = 1;
+       const FLD_WIKI = 2;
+
        /**
         * @param LoadBalancer $lb
-        * @param DatabaseBase|array $conn Connection or (server index, group, wiki ID) array
+        * @param DatabaseBase|array $conn Connection or (server index, group, wiki ID)
         */
        public function __construct( LoadBalancer $lb, $conn ) {
                $this->lb = $lb;
                if ( $conn instanceof DatabaseBase ) {
                        $this->conn = $conn;
-               } else {
+               } elseif ( count( $conn ) >= 3 && $conn[self::FLD_WIKI] !== false ) {
                        $this->params = $conn;
+               } else {
+                       throw new InvalidArgumentException( "Missing lazy connection arguments." );
                }
        }
 
@@ -136,6 +142,11 @@ class DBConnRef implements IDatabase {
        }
 
        public function getWikiID() {
+               if ( $this->conn === null ) {
+                       // Avoid triggering a connection
+                       return $this->params[self::FLD_WIKI];
+               }
+
                return $this->__call( __FUNCTION__, func_get_args() );
        }
 

diff --git a/includes/db/loadbalancer/LBFactory.php b/includes/db/loadbalancer/LBFactory.php
index cd8dff3..df1d948 100644 (file)
--- a/includes/db/loadbalancer/LBFactory.php
+++ b/includes/db/loadbalancer/LBFactory.php
@@ -147,7 +147,7 @@ abstract class LBFactory implements DestructibleService {
         * @deprecated since 1.27, use LBFactory::destroy()
         */
        public static function destroyInstance() {
-               self::singleton()->destroy();
+               MediaWikiServices::getInstance()->getDBLoadBalancerFactory()->destroy();
        }
 
        /**
@@ -620,6 +620,20 @@ abstract class LBFactory implements DestructibleService {
                } );
        }
 
+       /**
+        * Base parameters to LoadBalancer::__construct()
+        */
+       final protected function baseLoadBalancerParams() {
+               return [
+                       'readOnlyReason' => $this->readOnlyReason,
+                       'trxProfiler' => $this->trxProfiler,
+                       'srvCache' => $this->srvCache,
+                       'wanCache' => $this->wanCache,
+                       'localDomain' => wfWikiID(),
+                       'errorLogger' => [ MWExceptionHandler::class, 'logException' ]
+               ];
+       }
+
        /**
         * @param LoadBalancer $lb
         */

diff --git a/includes/db/loadbalancer/LBFactoryMulti.php b/includes/db/loadbalancer/LBFactoryMulti.php
index e860840..d486e13 100644 (file)
--- a/includes/db/loadbalancer/LBFactoryMulti.php
+++ b/includes/db/loadbalancer/LBFactoryMulti.php
@@ -311,15 +311,14 @@ class LBFactoryMulti extends LBFactory {
         * @return LoadBalancer
         */
        private function newLoadBalancer( $template, $loads, $groupLoads, $readOnlyReason ) {
-               $lb = new LoadBalancer( [
-                       'servers' => $this->makeServerArray( $template, $loads, $groupLoads ),
-                       'loadMonitor' => $this->loadMonitorClass,
-                       'readOnlyReason' => $readOnlyReason,
-                       'trxProfiler' => $this->trxProfiler,
-                       'srvCache' => $this->srvCache,
-                       'wanCache' => $this->wanCache
-               ] );
-
+               $lb = new LoadBalancer( array_merge(
+                       $this->baseLoadBalancerParams(),
+                       [
+                               'servers' => $this->makeServerArray( $template, $loads, $groupLoads ),
+                               'loadMonitor' => $this->loadMonitorClass,
+                               'readOnlyReason' => $readOnlyReason
+                       ]
+               ) );
                $this->initLoadBalancer( $lb );
 
                return $lb;

diff --git a/includes/db/loadbalancer/LBFactorySimple.php b/includes/db/loadbalancer/LBFactorySimple.php
index b6fb0d2..69c4abb 100644 (file)
--- a/includes/db/loadbalancer/LBFactorySimple.php
+++ b/includes/db/loadbalancer/LBFactorySimple.php
@@ -131,15 +131,13 @@ class LBFactorySimple extends LBFactory {
        }
 
        private function newLoadBalancer( array $servers ) {
-               $lb = new LoadBalancer( [
-                       'servers' => $servers,
-                       'loadMonitor' => $this->loadMonitorClass,
-                       'readOnlyReason' => $this->readOnlyReason,
-                       'trxProfiler' => $this->trxProfiler,
-                       'srvCache' => $this->srvCache,
-                       'wanCache' => $this->wanCache
-               ] );
-
+               $lb = new LoadBalancer( array_merge(
+                       $this->baseLoadBalancerParams(),
+                       [
+                               'servers' => $servers,
+                               'loadMonitor' => $this->loadMonitorClass,
+                       ]
+               ) );
                $this->initLoadBalancer( $lb );
 
                return $lb;

diff --git a/includes/db/loadbalancer/LBFactorySingle.php b/includes/db/loadbalancer/LBFactorySingle.php
index 14cec0e..de82a1f 100644 (file)
--- a/includes/db/loadbalancer/LBFactorySingle.php
+++ b/includes/db/loadbalancer/LBFactorySingle.php
@@ -35,12 +35,7 @@ class LBFactorySingle extends LBFactory {
        public function __construct( array $conf ) {
                parent::__construct( $conf );
 
-               $this->lb = new LoadBalancerSingle( [
-                       'readOnlyReason' => $this->readOnlyReason,
-                       'trxProfiler' => $this->trxProfiler,
-                       'srvCache' => $this->srvCache,
-                       'wanCache' => $this->wanCache
-               ] + $conf );
+               $this->lb = new LoadBalancerSingle( array_merge( $this->baseLoadBalancerParams(), $conf ) );
        }
 
        /**

diff --git a/includes/db/loadbalancer/LoadBalancer.php b/includes/db/loadbalancer/LoadBalancer.php
index 42044a7..8223ad6 100644 (file)
--- a/includes/db/loadbalancer/LoadBalancer.php
+++ b/includes/db/loadbalancer/LoadBalancer.php
@@ -74,6 +74,10 @@ class LoadBalancer {
        private $trxRoundId = false;
        /** @var array[] Map of (name => callable) */
        private $trxRecurringCallbacks = [];
+       /** @var string Local Wiki ID and default for selectDB() calls */
+       private $localDomain;
+       /** @var callable Exception logger */
+       private $errorLogger;
 
        /** @var integer Warn when this many connection are held */
        const CONN_HELD_WARN_THRESHOLD = 10;
@@ -97,6 +101,8 @@ class LoadBalancer {
         *  - waitTimeout : Maximum time to wait for replicas for consistency [optional]
         *  - srvCache : BagOStuff object [optional]
         *  - wanCache : WANObjectCache object [optional]
+        *  - localDomain: The wiki ID of the "local"/"current" wiki [optional]
+        *  - errorLogger: Callback that takes an Exception and logs it [optional]
         * @throws MWException
         */
        public function __construct( array $params ) {
@@ -107,6 +113,7 @@ class LoadBalancer {
                $this->mWaitTimeout = isset( $params['waitTimeout'] )
                        ? $params['waitTimeout']
                        : self::POS_WAIT_TIMEOUT;
+               $this->localDomain = isset( $params['localDomain'] ) ? $params['localDomain'] : '';
 
                $this->mReadIndex = -1;
                $this->mWriteIndex = -1;
@@ -161,6 +168,12 @@ class LoadBalancer {
                } else {
                        $this->trxProfiler = new TransactionProfiler();
                }
+
+               $this->errorLogger = isset( $params['errorLogger'] )
+                       ? $params['errorLogger']
+                       : function ( Exception $e ) {
+                               trigger_error( E_WARNING, $e->getMessage() );
+                       };
        }
 
        /**
@@ -240,16 +253,9 @@ class LoadBalancer {
         * @return bool|int|string
         */
        public function getReaderIndex( $group = false, $wiki = false ) {
-               global $wgDBtype;
-
-               # @todo FIXME: For now, only go through all this for mysql databases
-               if ( $wgDBtype != 'mysql' ) {
-                       return $this->getWriterIndex();
-               }
-
                if ( count( $this->mServers ) == 1 ) {
                        # Skip the load balancing if there's only one server
-                       return 0;
+                       return $this->getWriterIndex();
                } elseif ( $group === false && $this->mReadIndex >= 0 ) {
                        # Shortcut if generic reader exists already
                        return $this->mReadIndex;
@@ -273,7 +279,7 @@ class LoadBalancer {
                        throw new MWException( "Empty server array given to LoadBalancer" );
                }
 
-               # Scale the configured load ratios according to the dynamic load (if the load monitor supports it)
+               # Scale the configured load ratios according to the dynamic load if supported
                $this->getLoadMonitor()->scaleLoads( $nonErrorLoads, $group, $wiki );
 
                $laggedReplicaMode = false;
@@ -532,7 +538,7 @@ class LoadBalancer {
                                ' with invalid server index' );
                }
 
-               if ( $wiki === wfWikiID() ) {
+               if ( $wiki === $this->localDomain ) {
                        $wiki = false;
                }
 
@@ -581,8 +587,7 @@ class LoadBalancer {
                if ( $this->connsOpened > $oldConnsOpened ) {
                        $host = $conn->getServer();
                        $dbname = $conn->getDBname();
-                       $trxProf = Profiler::instance()->getTransactionProfiler();
-                       $trxProf->recordConnection( $host, $dbname, $masterOnly );
+                       $this->trxProfiler->recordConnection( $host, $dbname, $masterOnly );
                }
 
                if ( $masterOnly ) {
@@ -668,6 +673,8 @@ class LoadBalancer {
         * @return DBConnRef
         */
        public function getLazyConnectionRef( $db, $groups = [], $wiki = false ) {
+               $wiki = ( $wiki !== false ) ? $wiki : $this->localDomain;
+
                return new DBConnRef( $this, [ $db, $groups, $wiki ] );
        }
 
@@ -738,7 +745,8 @@ class LoadBalancer {
         * @return DatabaseBase
         */
        private function openForeignConnection( $i, $wiki ) {
-               list( $dbName, $prefix ) = wfSplitWikiID( $wiki );
+               list( $dbName, $prefix ) = explode( '-', $wiki, 2 ) + [ '', '' ];
+
                if ( isset( $this->mConns['foreignUsed'][$i][$wiki] ) ) {
                        // Reuse an already-used connection
                        $conn = $this->mConns['foreignUsed'][$i][$wiki];
@@ -1075,7 +1083,7 @@ class LoadBalancer {
                                try {
                                        $conn->commit( $fname, $conn::FLUSHING_ALL_PEERS );
                                } catch ( DBError $e ) {
-                                       MWExceptionHandler::logException( $e );
+                                       call_user_func( $this->errorLogger, $e );
                                        $failures[] = "{$conn->getServer()}: {$e->getMessage()}";
                                }
                                if ( $restore && $conn->getLBInfo( 'master' ) ) {
@@ -1175,7 +1183,7 @@ class LoadBalancer {
                                try {
                                        $conn->flushSnapshot( $fname );
                                } catch ( DBError $e ) {
-                                       MWExceptionHandler::logException( $e );
+                                       call_user_func( $this->errorLogger, $e );
                                        $failures[] = "{$conn->getServer()}: {$e->getMessage()}";
                                }
                                $conn->setTrxEndCallbackSuppression( false );
@@ -1210,7 +1218,7 @@ class LoadBalancer {
                                                $conn->flushSnapshot( $fname );
                                        }
                                } catch ( DBError $e ) {
-                                       MWExceptionHandler::logException( $e );
+                                       call_user_func( $this->errorLogger, $e );
                                        $failures[] = "{$conn->getServer()}: {$e->getMessage()}";
                                }
                                if ( $restore ) {
@@ -1719,4 +1727,16 @@ class LoadBalancer {
                        }
                );
        }
+
+       /**
+        * Set a new table prefix for the existing local wiki ID for testing
+        *
+        * @param string $prefix
+        * @since 1.28
+        */
+       public function setDomainPrefix( $prefix ) {
+               list( $dbName, ) = explode( '-', $this->localDomain, 2 );
+
+               $this->localDomain = "{$dbName}-{$prefix}";
+       }
 }