Aaron Schulz has uploaded a new change for review.

  https://gerrit.wikimedia.org/r/310924

Change subject: Move LBFactorySimple to /libs/rdbms
......................................................................

Move LBFactorySimple to /libs/rdbms

* Refactored LBFactory a bit to make this possible.
* Move newChronologyProtector() up to LBFactory and
  make a lazy-loading method instead.
* Move appendPreShutdownTimeAsQuery() up to LBFactory.
* Inject the web request values for LBFactory from Setup.php.
* Remove unused laggedSlaveUsed() method.

Change-Id: Ie8a38a6f4d6359680eb6a5be24a34e30b9816479
---
M autoload.php
M includes/ServiceWiring.php
M includes/Setup.php
M includes/db/loadbalancer/LBFactoryMW.php
M includes/db/loadbalancer/LBFactoryMulti.php
M includes/libs/rdbms/lbfactory/LBFactory.php
R includes/libs/rdbms/lbfactory/LBFactorySimple.php
7 files changed, 146 insertions(+), 135 deletions(-)


  git pull ssh://gerrit.wikimedia.org:29418/mediawiki/core 
refs/changes/24/310924/1

diff --git a/autoload.php b/autoload.php
index 96c8190..15d1297 100644
--- a/autoload.php
+++ b/autoload.php
@@ -656,7 +656,7 @@
        'LBFactory' => __DIR__ . '/includes/libs/rdbms/lbfactory/LBFactory.php',
        'LBFactoryMW' => __DIR__ . '/includes/db/loadbalancer/LBFactoryMW.php',
        'LBFactoryMulti' => __DIR__ . 
'/includes/db/loadbalancer/LBFactoryMulti.php',
-       'LBFactorySimple' => __DIR__ . 
'/includes/db/loadbalancer/LBFactorySimple.php',
+       'LBFactorySimple' => __DIR__ . 
'/includes/libs/rdbms/lbfactory/LBFactorySimple.php',
        'LBFactorySingle' => __DIR__ . 
'/includes/db/loadbalancer/LBFactorySingle.php',
        'LCStore' => __DIR__ . '/includes/cache/localisation/LCStore.php',
        'LCStoreCDB' => __DIR__ . '/includes/cache/localisation/LCStoreCDB.php',
diff --git a/includes/ServiceWiring.php b/includes/ServiceWiring.php
index 4ab412e..f054533 100644
--- a/includes/ServiceWiring.php
+++ b/includes/ServiceWiring.php
@@ -43,15 +43,53 @@
 
 return [
        'DBLoadBalancerFactory' => function( MediaWikiServices $services ) {
-               $config = $services->getMainConfig()->get( 'LBFactoryConf' );
+               $mainConfig = $services->getMainConfig();
 
-               $class = LBFactoryMW::getLBFactoryClass( $config );
-               if ( !isset( $config['readOnlyReason'] ) ) {
+               $lbConf = $mainConfig->get( 'LBFactoryConf' );
+               if ( !isset( $lbConf['readOnlyReason'] ) ) {
                        // TODO: replace the global 
wfConfiguredReadOnlyReason() with a service.
-                       $config['readOnlyReason'] = 
wfConfiguredReadOnlyReason();
+                       $lbConf['readOnlyReason'] = 
wfConfiguredReadOnlyReason();
                }
 
-               return new $class( $config );
+               $class = LBFactoryMW::getLBFactoryClass( $lbConf );
+               if ( $class === 'LBFactorySimple' ) {
+                       if ( is_array( $mainConfig->get( 'DBservers' ) ) ) {
+                               foreach ( $mainConfig->get( 'DBservers' ) as $i 
=> $server ) {
+                                       $lbConf['servers'][$i] = $server + [
+                                               'schema' => $mainConfig->get( 
'DBmwschema' ),
+                                               'tablePrefix' => 
$mainConfig->get( 'DBprefix' ),
+                                               'flags' => DBO_DEFAULT,
+                                       ];
+                                       if ( $i == 0 ) {
+                                               
$lbConf['servers'][$i]['master'] = true;
+                                       } else {
+                                               
$lbConf['servers'][$i]['replica'] = true;
+                                       }
+                               }
+                       } else {
+                               $flags = DBO_DEFAULT;
+                               $flags |= $mainConfig->get( 'DebugDumpSql' ) ? 
DBO_DEBUG : 0;
+                               $flags |= $mainConfig->get( 'DBssl' ) ? DBO_SSL 
: 0;
+                               $flags |= $mainConfig->get( 'DBcompress' ) ? 
DBO_COMPRESS : 0;
+                               $lbConf['servers'] = [
+                                       [
+                                               'host' => $mainConfig->get( 
'DBserver' ),
+                                               'user' => $mainConfig->get( 
'DBuser' ),
+                                               'password' => $mainConfig->get( 
'DBpassword' ),
+                                               'dbname' => $mainConfig->get( 
'DBname' ),
+                                               'schema' => $mainConfig->get( 
'DBmwschema' ),
+                                               'tablePrefix' => 
$mainConfig->get( 'DBprefix' ),
+                                               'type' => $mainConfig->get( 
'DBtype' ),
+                                               'load' => 1,
+                                               'flags' => $flags,
+                                               'master' => true
+                                       ]
+                               ];
+                       }
+                       $lbConf['externalServers'] = $mainConfig->get( 
'ExternalServers' );
+               }
+
+               return new $class( $lbConf );
        },
 
        'DBLoadBalancer' => function( MediaWikiServices $services ) {
diff --git a/includes/Setup.php b/includes/Setup.php
index ddf5b89..cfcb42e 100644
--- a/includes/Setup.php
+++ b/includes/Setup.php
@@ -504,8 +504,9 @@
 // Reset the global service locator, so any services that have already been 
created will be
 // re-created while taking into account any custom settings and extensions.
 MediaWikiServices::resetGlobalInstance( new GlobalVarConfig(), 'quick' );
-// Apply $wgSharedDB table aliases for the local LB (all non-foreign DB 
connections)
+
 if ( $wgSharedDB && $wgSharedTables ) {
+       // Apply $wgSharedDB table aliases for the local LB (all non-foreign DB 
connections)
        MediaWikiServices::getInstance()->getDBLoadBalancer()->setTableAliases(
                array_fill_keys(
                        $wgSharedTables,
@@ -516,6 +517,12 @@
                        ]
                )
        );
+       // Set user IP/agent information for causal consistency purposes
+       
MediaWikiServices::getInstance()->getDBLoadBalancerFactory()->setRequestInfo( [
+               'IPAddress' => $wgRequest->getIP(),
+               'UserAgent' => $wgRequest->getHeader( 'User-Agent' ),
+               'ChronologyProtection' => $wgRequest->getHeader( 
'ChronologyProtection' )
+       ] );
 }
 
 // Define a constant that indicates that the bootstrapping of the service 
locator
diff --git a/includes/db/loadbalancer/LBFactoryMW.php 
b/includes/db/loadbalancer/LBFactoryMW.php
index 87fd81b..3523a93 100644
--- a/includes/db/loadbalancer/LBFactoryMW.php
+++ b/includes/db/loadbalancer/LBFactoryMW.php
@@ -93,57 +93,4 @@
 
                return $class;
        }
-
-       /**
-        * @return bool
-        * @since 1.27
-        * @deprecated Since 1.28; use laggedReplicaUsed()
-        */
-       public function laggedSlaveUsed() {
-               return $this->laggedReplicaUsed();
-       }
-
-       protected function newChronologyProtector() {
-               $request = RequestContext::getMain()->getRequest();
-               $chronProt = new ChronologyProtector(
-                       ObjectCache::getMainStashInstance(),
-                       [
-                               'ip' => $request->getIP(),
-                               'agent' => $request->getHeader( 'User-Agent' ),
-                       ],
-                       $request->getFloat( 'cpPosTime', $request->getCookie( 
'cpPosTime', '' ) )
-               );
-               if ( PHP_SAPI === 'cli' ) {
-                       $chronProt->setEnabled( false );
-               } elseif ( $request->getHeader( 'ChronologyProtection' ) === 
'false' ) {
-                       // Request opted out of using position wait logic. This 
is useful for requests
-                       // done by the job queue or background ETL that do not 
have a meaningful session.
-                       $chronProt->setWaitEnabled( false );
-               }
-
-               return $chronProt;
-       }
-
-       /**
-        * Append ?cpPosTime parameter to a URL for ChronologyProtector 
purposes if needed
-        *
-        * Note that unlike cookies, this works accross domains
-        *
-        * @param string $url
-        * @param float $time UNIX timestamp just before shutdown() was called
-        * @return string
-        * @since 1.28
-        */
-       public function appendPreShutdownTimeAsQuery( $url, $time ) {
-               $usedCluster = 0;
-               $this->forEachLB( function ( LoadBalancer $lb ) use ( 
&$usedCluster ) {
-                       $usedCluster |= ( $lb->getServerCount() > 1 );
-               } );
-
-               if ( !$usedCluster ) {
-                       return $url; // no master/replica clusters touched
-               }
-
-               return wfAppendQuery( $url, [ 'cpPosTime' => $time ] );
-       }
 }
diff --git a/includes/db/loadbalancer/LBFactoryMulti.php 
b/includes/db/loadbalancer/LBFactoryMulti.php
index 95bc8f4..1f7f528 100644
--- a/includes/db/loadbalancer/LBFactoryMulti.php
+++ b/includes/db/loadbalancer/LBFactoryMulti.php
@@ -254,7 +254,7 @@
                $section = $this->getSectionForWiki( $wiki );
                if ( !isset( $this->mainLBs[$section] ) ) {
                        $lb = $this->newMainLB( $wiki );
-                       $this->chronProt->initLB( $lb );
+                       $this->getChronologyProtector()->initLB( $lb );
                        $this->mainLBs[$section] = $lb;
                }
 
@@ -295,7 +295,7 @@
        public function getExternalLB( $cluster, $wiki = false ) {
                if ( !isset( $this->extLBs[$cluster] ) ) {
                        $this->extLBs[$cluster] = $this->newExternalLB( 
$cluster, $wiki );
-                       $this->chronProt->initLB( $this->extLBs[$cluster] );
+                       $this->getChronologyProtector()->initLB( 
$this->extLBs[$cluster] );
                }
 
                return $this->extLBs[$cluster];
diff --git a/includes/libs/rdbms/lbfactory/LBFactory.php 
b/includes/libs/rdbms/lbfactory/LBFactory.php
index feae4bd..fe520d5 100644
--- a/includes/libs/rdbms/lbfactory/LBFactory.php
+++ b/includes/libs/rdbms/lbfactory/LBFactory.php
@@ -49,10 +49,13 @@
        /** @var WANObjectCache */
        protected $wanCache;
 
-       /** @var string Local domain */
-       protected $domain;
        /** @var string Local hostname of the app server */
        protected $hostname;
+       /** @var array Web request information about the client */
+       protected $requestInfo;
+
+       /** @var string Local domain */
+       protected $domain;
        /** @var mixed */
        protected $ticket;
        /** @var string|bool String if a requested DBO_TRX transaction round is 
active */
@@ -99,10 +102,16 @@
 
                $this->chronProt = isset( $conf['chronProt'] )
                        ? $conf['chronProt']
-                       : $this->newChronologyProtector();
+                       : null;
                $this->trxProfiler = isset( $conf['trxProfiler'] )
                        ? $conf['trxProfiler']
                        : new TransactionProfiler();
+
+               $this->requestInfo = [
+                       'IPAddress' => isset( $_SERVER[ 'REMOTE_ADDR' ] ) ? 
$_SERVER[ 'REMOTE_ADDR' ] : '',
+                       'UserAgent' => isset( $_SERVER['HTTP_USER_AGENT'] ) ? 
$_SERVER['HTTP_USER_AGENT'] : '',
+                       'ChronologyProtection' => 'true'
+               ];
 
                $this->ticket = mt_rand();
        }
@@ -527,7 +536,7 @@
         * @since 1.28
         */
        public function getChronologyProtectorTouched( $dbName ) {
-               return $this->chronProt->getTouched( $dbName );
+               return $this->getChronologyProtector()->getTouched( $dbName );
        }
 
        /**
@@ -538,27 +547,38 @@
         * @since 1.27
         */
        public function disableChronologyProtection() {
-               $this->chronProt->setEnabled( false );
+               $this->getChronologyProtector()->setEnabled( false );
        }
 
        /**
         * @return ChronologyProtector
         */
-       protected function newChronologyProtector() {
-               $chronProt = new ChronologyProtector(
+       protected function getChronologyProtector() {
+               if ( $this->chronProt ) {
+                       return $this->chronProt;
+               }
+
+               $this->chronProt = new ChronologyProtector(
                        $this->memCache,
                        [
-                               'ip' => isset( $_SERVER[ 'REMOTE_ADDR' ] ) ? 
$_SERVER[ 'REMOTE_ADDR' ] : '',
-                               'agent' => isset( $_SERVER['HTTP_USER_AGENT'] ) 
? $_SERVER['HTTP_USER_AGENT'] : ''
+                               'ip' => $this->requestInfo['IPAddress'],
+                               'agent' => $this->requestInfo['UserAgent'],
                        ],
                        isset( $_GET['cpPosTime'] ) ? $_GET['cpPosTime'] : null
                );
-               $chronProt->setLogger( $this->replLogger );
+               $this->chronProt->setLogger( $this->replLogger );
                if ( PHP_SAPI === 'cli' ) {
-                       $chronProt->setEnabled( false );
+                       $this->chronProt->setEnabled( false );
+               } elseif ( $this->requestInfo['ChronologyProtection'] === 
'false' ) {
+                       // Request opted out of using position wait logic. This 
is useful for requests
+                       // done by the job queue or background ETL that do not 
have a meaningful session.
+                       $this->chronProt->setWaitEnabled( false );
                }
 
-               return $chronProt;
+               $this->replLogger->debug( __METHOD__ . ': using request info ' .
+                       json_encode( $this->requestInfo, JSON_PRETTY_PRINT ) );
+
+               return $this->chronProt;
        }
 
        /**
@@ -641,4 +661,38 @@
        public function closeAll() {
                $this->forEachLBCallMethod( 'closeAll', [] );
        }
+
+       /**
+        * Append ?cpPosTime parameter to a URL for ChronologyProtector 
purposes if needed
+        *
+        * Note that unlike cookies, this works accross domains
+        *
+        * @param string $url
+        * @param float $time UNIX timestamp just before shutdown() was called
+        * @return string
+        * @since 1.28
+        */
+       public function appendPreShutdownTimeAsQuery( $url, $time ) {
+               $usedCluster = 0;
+               $this->forEachLB( function ( ILoadBalancer $lb ) use ( 
&$usedCluster ) {
+                       $usedCluster |= ( $lb->getServerCount() > 1 );
+               } );
+
+               if ( !$usedCluster ) {
+                       return $url; // no master/replica clusters touched
+               }
+
+               return strpos( $url, '?' ) === false ? "$url?cpPosTime=$time" : 
"$url&cpPosTime=$time";
+       }
+
+       /**
+        * @param array $info Map of fields, including:
+        *   - IPAddress : IP address
+        *   - UserAgent : User-Agent HTTP header
+        *   - ChronologyProtection : cookie/header value specifying 
ChronologyProtector usage
+        * @since 1.28
+        */
+       public function setRequestInfo( array $info ) {
+               $this->requestInfo = $info + $this->requestInfo;
+       }
 }
diff --git a/includes/db/loadbalancer/LBFactorySimple.php 
b/includes/libs/rdbms/lbfactory/LBFactorySimple.php
similarity index 60%
rename from includes/db/loadbalancer/LBFactorySimple.php
rename to includes/libs/rdbms/lbfactory/LBFactorySimple.php
index 09533eb..40579ab 100644
--- a/includes/db/loadbalancer/LBFactorySimple.php
+++ b/includes/libs/rdbms/lbfactory/LBFactorySimple.php
@@ -30,78 +30,44 @@
        /** @var LoadBalancer[] */
        private $extLBs = [];
 
+       /** @var array[] Map of (server index => server config) */
+       private $servers = [];
+       /** @var array[] Map of (cluster => (server index => server config)) */
+       private $externalClusters = [];
+
        /** @var string */
        private $loadMonitorClass;
 
        public function __construct( array $conf ) {
                parent::__construct( $conf );
 
+               $this->servers = isset( $conf['servers'] )
+                       ? $conf['servers']
+                       : [];
+               $this->externalClusters = isset( $conf['externalClusters'] )
+                       ? $conf['externalClusters']
+                       : [];
                $this->loadMonitorClass = isset( $conf['loadMonitorClass'] )
                        ? $conf['loadMonitorClass']
                        : null;
        }
 
        /**
-        * @param bool|string $wiki
+        * @param bool|string $domain
         * @return LoadBalancer
         */
-       public function newMainLB( $wiki = false ) {
-               global $wgDBservers, $wgDBprefix, $wgDBmwschema;
-
-               if ( is_array( $wgDBservers ) ) {
-                       $servers = $wgDBservers;
-                       foreach ( $servers as $i => &$server ) {
-                               if ( $i == 0 ) {
-                                       $server['master'] = true;
-                               } else {
-                                       $server['replica'] = true;
-                               }
-                               $server += [
-                                       'schema' => $wgDBmwschema,
-                                       'tablePrefix' => $wgDBprefix,
-                                       'flags' => DBO_DEFAULT
-                               ];
-                       }
-               } else {
-                       global $wgDBserver, $wgDBuser, $wgDBpassword, 
$wgDBname, $wgDBtype, $wgDebugDumpSql;
-                       global $wgDBssl, $wgDBcompress;
-
-                       $flags = DBO_DEFAULT;
-                       if ( $wgDebugDumpSql ) {
-                               $flags |= DBO_DEBUG;
-                       }
-                       if ( $wgDBssl ) {
-                               $flags |= DBO_SSL;
-                       }
-                       if ( $wgDBcompress ) {
-                               $flags |= DBO_COMPRESS;
-                       }
-
-                       $servers = [ [
-                               'host' => $wgDBserver,
-                               'user' => $wgDBuser,
-                               'password' => $wgDBpassword,
-                               'dbname' => $wgDBname,
-                               'schema' => $wgDBmwschema,
-                               'tablePrefix' => $wgDBprefix,
-                               'type' => $wgDBtype,
-                               'load' => 1,
-                               'flags' => $flags,
-                               'master' => true
-                       ] ];
-               }
-
-               return $this->newLoadBalancer( $servers );
+       public function newMainLB( $domain = false ) {
+               return $this->newLoadBalancer( $this->servers );
        }
 
        /**
-        * @param bool|string $wiki
+        * @param bool|string $domain
         * @return LoadBalancer
         */
-       public function getMainLB( $wiki = false ) {
+       public function getMainLB( $domain = false ) {
                if ( !isset( $this->mainLB ) ) {
-                       $this->mainLB = $this->newMainLB( $wiki );
-                       $this->chronProt->initLB( $this->mainLB );
+                       $this->mainLB = $this->newMainLB( $domain );
+                       $this->getChronologyProtector()->initLB( $this->mainLB 
);
                }
 
                return $this->mainLB;
@@ -109,28 +75,27 @@
 
        /**
         * @param string $cluster
-        * @param bool|string $wiki
+        * @param bool|string $domain
         * @return LoadBalancer
         * @throws InvalidArgumentException
         */
-       protected function newExternalLB( $cluster, $wiki = false ) {
-               global $wgExternalServers;
-               if ( !isset( $wgExternalServers[$cluster] ) ) {
+       protected function newExternalLB( $cluster, $domain = false ) {
+               if ( !isset( $this->externalClusters[$cluster] ) ) {
                        throw new InvalidArgumentException( __METHOD__ . ": 
Unknown cluster \"$cluster\"" );
                }
 
-               return $this->newLoadBalancer( $wgExternalServers[$cluster] );
+               return $this->newLoadBalancer( 
$this->externalClusters[$cluster] );
        }
 
        /**
         * @param string $cluster
-        * @param bool|string $wiki
+        * @param bool|string $domain
         * @return array
         */
-       public function getExternalLB( $cluster, $wiki = false ) {
+       public function getExternalLB( $cluster, $domain = false ) {
                if ( !isset( $this->extLBs[$cluster] ) ) {
-                       $this->extLBs[$cluster] = $this->newExternalLB( 
$cluster, $wiki );
-                       $this->chronProt->initLB( $this->extLBs[$cluster] );
+                       $this->extLBs[$cluster] = $this->newExternalLB( 
$cluster, $domain );
+                       $this->getChronologyProtector()->initLB( 
$this->extLBs[$cluster] );
                }
 
                return $this->extLBs[$cluster];

-- 
To view, visit https://gerrit.wikimedia.org/r/310924
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ie8a38a6f4d6359680eb6a5be24a34e30b9816479
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/core
Gerrit-Branch: master
Gerrit-Owner: Aaron Schulz <asch...@wikimedia.org>

_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits

Reply via email to