Avoid global state in DatabaseBase::factory()/query()

Change-Id: Ibb4f1c0dafea071a1c34e0cd5b5c15b8b4bb7bc6

diff --git a/includes/MediaWiki.php b/includes/MediaWiki.php
index 9bbbd35..2aa4b80 100644 (file)
--- a/includes/MediaWiki.php
+++ b/includes/MediaWiki.php
@@ -517,6 +517,7 @@ class MediaWiki {
         */
        public function run() {
                try {
+                       $this->setDBProfilingAgent();
                        try {
                                $this->main();
                        } catch ( ErrorPageError $e ) {
@@ -533,6 +534,15 @@ class MediaWiki {
                $this->doPostOutputShutdown( 'normal' );
        }
 
+       private function setDBProfilingAgent() {
+               $services = MediaWikiServices::getInstance();
+               // Add a comment for easy SHOW PROCESSLIST interpretation
+               $name = $this->context->getUser()->getName();
+               $services->getDBLoadBalancerFactory()->setAgentName(
+                       mb_strlen( $name ) > 15 ? mb_substr( $name, 0, 15 ) . '...' : $name
+               );
+       }
+
        /**
         * @see MediaWiki::preOutputCommit()
         * @param callable $postCommitWork [default: null]

diff --git a/includes/db/Database.php b/includes/db/Database.php
index 3fa1335..232bd38 100644 (file)
--- a/includes/db/Database.php
+++ b/includes/db/Database.php
@@ -62,8 +62,10 @@ abstract class DatabaseBase implements IDatabase, LoggerAwareInterface {
        protected $mDBname;
        /** @var array[] $aliases Map of (table => (dbname, schema, prefix) map) */
        protected $tableAliases = [];
-       /** @var bool */
+       /** @var bool Whether this PHP instance is for a CLI script */
        protected $cliMode;
+       /** @var string Agent name for query profiling */
+       protected $agent;
 
        /** @var BagOStuff APC cache */
        protected $srvCache;
@@ -251,6 +253,9 @@ abstract class DatabaseBase implements IDatabase, LoggerAwareInterface {
                $this->cliMode = isset( $params['cliMode'] )
                        ? $params['cliMode']
                        : ( PHP_SAPI === 'cli' );
+               $this->agent = isset( $params['agent'] )
+                       ? str_replace( '/', '-', $params['agent'] ) // escape for comment
+                       : '';
 
                $this->mFlags = $flags;
                if ( $this->mFlags & DBO_DEFAULT ) {
@@ -308,8 +313,6 @@ abstract class DatabaseBase implements IDatabase, LoggerAwareInterface {
         * @throws InvalidArgumentException If the database driver or extension cannot be found
         */
        final public static function factory( $dbType, $p = [] ) {
-               global $wgCommandLineMode;
-
                $canonicalDBTypes = [
                        'mysql' => [ 'mysqli', 'mysql' ],
                        'postgres' => [],
@@ -367,7 +370,6 @@ abstract class DatabaseBase implements IDatabase, LoggerAwareInterface {
                                $p['schema'] = isset( $defaultSchemas[$dbType] ) ? $defaultSchemas[$dbType] : null;
                        }
                        $p['foreign'] = isset( $p['foreign'] ) ? $p['foreign'] : false;
-                       $p['cliMode'] = $wgCommandLineMode;
 
                        $conn = new $class( $p );
                        if ( isset( $p['connLogger'] ) ) {
@@ -379,7 +381,9 @@ abstract class DatabaseBase implements IDatabase, LoggerAwareInterface {
                        if ( isset( $p['errorLogger'] ) ) {
                                $conn->errorLogger = $p['errorLogger'];
                        } else {
-                               $conn->errorLogger = [ MWExceptionHandler::class, 'logException' ];
+                               $conn->errorLogger = function ( Exception $e ) {
+                                       trigger_error( get_class( $e ) . ': ' . $e->getMessage(), E_WARNING );
+                               };
                        }
                } else {
                        $conn = null;
@@ -831,8 +835,6 @@ abstract class DatabaseBase implements IDatabase, LoggerAwareInterface {
        }
 
        public function query( $sql, $fname = __METHOD__, $tempIgnore = false ) {
-               global $wgUser;
-
                $priorWritesPending = $this->writesOrCallbacksPending();
                $this->mLastQuery = $sql;
 
@@ -846,20 +848,9 @@ abstract class DatabaseBase implements IDatabase, LoggerAwareInterface {
                        $this->mDoneWrites = microtime( true );
                }
 
-               # Add a comment for easy SHOW PROCESSLIST interpretation
-               if ( is_object( $wgUser ) && $wgUser->isItemLoaded( 'name' ) ) {
-                       $userName = $wgUser->getName();
-                       if ( mb_strlen( $userName ) > 15 ) {
-                               $userName = mb_substr( $userName, 0, 15 ) . '...';
-                       }
-                       $userName = str_replace( '/', '', $userName );
-               } else {
-                       $userName = '';
-               }
-
                // Add trace comment to the begin of the sql string, right after the operator.
                // Or, for one-word queries (like "BEGIN" or COMMIT") add it to the end (bug 42598)
-               $commentedSql = preg_replace( '/\s|$/', " /* $fname $userName */ ", $sql, 1 );
+               $commentedSql = preg_replace( '/\s|$/', " /* $fname {$this->agent} */ ", $sql, 1 );
 
                # Start implicit transactions that wrap the request if DBO_TRX is enabled
                if ( !$this->mTrxLevel && $this->getFlag( DBO_TRX )

diff --git a/includes/db/loadbalancer/LBFactoryMW.php b/includes/db/loadbalancer/LBFactoryMW.php
index 87fd81b..33c48a5 100644 (file)
--- a/includes/db/loadbalancer/LBFactoryMW.php
+++ b/includes/db/loadbalancer/LBFactoryMW.php
@@ -37,6 +37,8 @@ abstract class LBFactoryMW extends LBFactory implements DestructibleService {
         * @TODO: inject objects via dependency framework
         */
        public function __construct( array $conf ) {
+               global $wgCommandLineMode;
+
                $defaults = [
                        'domain' => wfWikiID(),
                        'hostname' => wfHostname(),
@@ -61,6 +63,9 @@ abstract class LBFactoryMW extends LBFactory implements DestructibleService {
                        $defaults['wanCache'] = $wCache;
                }
 
+               $this->agent = isset( $params['agent'] ) ? $params['agent'] : '';
+               $this->cliMode = isset( $params['cliMode'] ) ? $params['cliMode'] : $wgCommandLineMode;
+
                parent::__construct( $conf + $defaults );
        }
 

diff --git a/includes/libs/rdbms/lbfactory/LBFactory.php b/includes/libs/rdbms/lbfactory/LBFactory.php
index feae4bd..107a7e2 100644 (file)
--- a/includes/libs/rdbms/lbfactory/LBFactory.php
+++ b/includes/libs/rdbms/lbfactory/LBFactory.php
@@ -62,6 +62,11 @@ abstract class LBFactory {
        /** @var callable[] */
        protected $replicationWaitCallbacks = [];
 
+       /** @var bool Whether this PHP instance is for a CLI script */
+       protected $cliMode;
+       /** @var string Agent name for query profiling */
+       protected $agent;
+
        const SHUTDOWN_NO_CHRONPROT = 0; // don't save DB positions at all
        const SHUTDOWN_CHRONPROT_ASYNC = 1; // save DB positions, but don't wait on remote DCs
        const SHUTDOWN_CHRONPROT_SYNC = 2; // save DB positions, waiting on all DCs
@@ -75,6 +80,7 @@ abstract class LBFactory {
         */
        public function __construct( array $conf ) {
                $this->domain = isset( $conf['domain'] ) ? $conf['domain'] : '';
+
                if ( isset( $conf['readOnlyReason'] ) && is_string( $conf['readOnlyReason'] ) ) {
                        $this->readOnlyReason = $conf['readOnlyReason'];
                }
@@ -91,7 +97,7 @@ abstract class LBFactory {
                $this->errorLogger = isset( $conf['errorLogger'] )
                        ? $conf['errorLogger']
                        : function ( Exception $e ) {
-                               trigger_error( E_WARNING, $e->getMessage() );
+                               trigger_error( E_WARNING, get_class( $e ) . ': ' . $e->getMessage() );
                        };
                $this->hostname = isset( $conf['hostname'] )
                        ? $conf['hostname']
@@ -105,6 +111,8 @@ abstract class LBFactory {
                        : new TransactionProfiler();
 
                $this->ticket = mt_rand();
+               $this->cliMode = isset( $params['cliMode'] ) ? $params['cliMode'] : PHP_SAPI === 'cli';
+               $this->agent = isset( $params['agent'] ) ? $params['agent'] : '';
        }
 
        /**
@@ -554,7 +562,7 @@ abstract class LBFactory {
                        isset( $_GET['cpPosTime'] ) ? $_GET['cpPosTime'] : null
                );
                $chronProt->setLogger( $this->replLogger );
-               if ( PHP_SAPI === 'cli' ) {
+               if ( $this->cliMode ) {
                        $chronProt->setEnabled( false );
                }
 
@@ -609,7 +617,9 @@ abstract class LBFactory {
                        'connLogger' => $this->connLogger,
                        'replLogger' => $this->replLogger,
                        'errorLogger' => $this->errorLogger,
-                       'hostname' => $this->hostname
+                       'hostname' => $this->hostname,
+                       'cliMode' => $this->cliMode,
+                       'agent' => $this->agent
                ];
        }
 
@@ -641,4 +651,12 @@ abstract class LBFactory {
        public function closeAll() {
                $this->forEachLBCallMethod( 'closeAll', [] );
        }
+
+       /**
+        * @param string $agent Agent name for query profiling
+        * @since 1.28
+        */
+       public function setAgentName( $agent ) {
+               $this->agent = $agent;
+       }
 }

diff --git a/includes/libs/rdbms/loadbalancer/LoadBalancer.php b/includes/libs/rdbms/loadbalancer/LoadBalancer.php
index cea7523..628e452 100644 (file)
--- a/includes/libs/rdbms/loadbalancer/LoadBalancer.php
+++ b/includes/libs/rdbms/loadbalancer/LoadBalancer.php
@@ -88,6 +88,10 @@ class LoadBalancer implements ILoadBalancer {
        private $localDomain;
        /** @var string Current server name */
        private $host;
+       /** @var bool Whether this PHP instance is for a CLI script */
+       protected $cliMode;
+       /** @var string Agent name for query profiling */
+       protected $agent;
 
        /** @var callable Exception logger */
        private $errorLogger;
@@ -175,7 +179,7 @@ class LoadBalancer implements ILoadBalancer {
                $this->errorLogger = isset( $params['errorLogger'] )
                        ? $params['errorLogger']
                        : function ( Exception $e ) {
-                               trigger_error( E_WARNING, $e->getMessage() );
+                               trigger_error( get_class( $e ) . ': ' . $e->getMessage(), E_WARNING );
                        };
 
                foreach ( [ 'replLogger', 'connLogger', 'queryLogger', 'perfLogger' ] as $key ) {
@@ -185,6 +189,8 @@ class LoadBalancer implements ILoadBalancer {
                $this->host = isset( $params['hostname'] )
                        ? $params['hostname']
                        : ( gethostname() ?: 'unknown' );
+               $this->cliMode = isset( $params['cliMode'] ) ? $params['cliMode'] : PHP_SAPI === 'cli';
+               $this->agent = isset( $params['agent'] ) ? $params['agent'] : '';
        }
 
        /**
@@ -810,6 +816,9 @@ class LoadBalancer implements ILoadBalancer {
                $server['connLogger'] = $this->connLogger;
                $server['queryLogger'] = $this->queryLogger;
                $server['trxProfiler'] = $this->trxProfiler;
+               $server['cliMode'] = $this->cliMode;
+               $server['errorLogger'] = $this->errorLogger;
+               $server['agent'] = $this->agent;
 
                // Create a live connection object
                try {

diff --git a/maintenance/Maintenance.php b/maintenance/Maintenance.php
index 1fbca14..7e0fb45 100644 (file)
--- a/maintenance/Maintenance.php
+++ b/maintenance/Maintenance.php
@@ -553,8 +553,19 @@ abstract class Maintenance {
         * Set triggers like when to try to run deferred updates
         * @since 1.28
         */
-       public function setTriggers() {
+       public function setAgentAndTriggers() {
+               if ( function_exists( 'posix_getpwuid' ) ) {
+                       $agent = posix_getpwuid( posix_geteuid() )['name'];
+               } else {
+                       $agent = 'sysadmin';
+               }
+               $agent .= '@' . wfHostname();
+
                $lbFactory = MediaWikiServices::getInstance()->getDBLoadBalancerFactory();
+               // Add a comment for easy SHOW PROCESSLIST interpretation
+               $lbFactory->setAgentName(
+                       mb_strlen( $agent ) > 15 ? mb_substr( $agent, 0, 15 ) . '...' : $agent
+               );
                self::setLBFactoryTriggers( $lbFactory );
        }
 

diff --git a/maintenance/doMaintenance.php b/maintenance/doMaintenance.php
index 890fe45..60b24a2 100644 (file)
--- a/maintenance/doMaintenance.php
+++ b/maintenance/doMaintenance.php
@@ -104,7 +104,7 @@ $maintenance->checkRequiredExtensions();
 
 // A good time when no DBs have writes pending is around lag checks.
 // This avoids having long running scripts just OOM and lose all the updates.
-$maintenance->setTriggers();
+$maintenance->setAgentAndTriggers();
 
 // Do the work
 $maintenance->execute();