From 5bc9b990acd0e0302f64b8768d8c60d090927563 Mon Sep 17 00:00:00 2001 From: Aaron Schulz Date: Thu, 15 Sep 2016 20:33:25 -0700 Subject: [PATCH] Cleanups to DatabaseMysqlBase * Avoid global methods * Inject global variables * Remove $wgAllDBsAreLocalhost hack Change-Id: I54b23654def1f83518764ad697434aebfc6cef73 --- autoload.php | 2 +- includes/DefaultSettings.php | 7 ---- includes/db/DatabaseMysqlBase.php | 41 ++++++++++--------- includes/db/loadbalancer/LBFactoryMW.php | 7 +++- includes/db/loadbalancer/LBFactorySimple.php | 10 +++-- .../rdbms/database/DatabaseBase.php} | 0 includes/libs/rdbms/lbfactory/LBFactory.php | 3 +- 7 files changed, 36 insertions(+), 34 deletions(-) rename includes/{db/Database.php => libs/rdbms/database/DatabaseBase.php} (100%) diff --git a/autoload.php b/autoload.php index eedf7b412c..716e56db87 100644 --- a/autoload.php +++ b/autoload.php @@ -317,7 +317,7 @@ $wgAutoloadLocalClasses = [ 'DBUnexpectedError' => __DIR__ . '/includes/libs/rdbms/exception/DBError.php', 'DataUpdate' => __DIR__ . '/includes/deferred/DataUpdate.php', 'Database' => __DIR__ . '/includes/libs/rdbms/database/Database.php', - 'DatabaseBase' => __DIR__ . '/includes/db/Database.php', + 'DatabaseBase' => __DIR__ . '/includes/libs/rdbms/database/DatabaseBase.php', 'DatabaseInstaller' => __DIR__ . '/includes/installer/DatabaseInstaller.php', 'DatabaseLag' => __DIR__ . '/maintenance/lag.php', 'DatabaseLogEntry' => __DIR__ . '/includes/logging/LogEntry.php', diff --git a/includes/DefaultSettings.php b/includes/DefaultSettings.php index 3ab8829a59..135c3e5231 100644 --- a/includes/DefaultSettings.php +++ b/includes/DefaultSettings.php @@ -1835,13 +1835,6 @@ $wgDBmwschema = null; */ $wgSQLiteDataDir = ''; -/** - * Make all database connections secretly go to localhost. Fool the load balancer - * thinking there is an arbitrarily large cluster of servers to connect to. - * Useful for debugging. - */ -$wgAllDBsAreLocalhost = false; - /** * Shared database for multiple wikis. Commonly used for storing a user table * for single sign-on. The server for this database must be the same as for the diff --git a/includes/db/DatabaseMysqlBase.php b/includes/db/DatabaseMysqlBase.php index 06511e0b70..46c6678730 100644 --- a/includes/db/DatabaseMysqlBase.php +++ b/includes/db/DatabaseMysqlBase.php @@ -46,6 +46,11 @@ abstract class DatabaseMysqlBase extends DatabaseBase { protected $sslCAPath; /** @var string[]|null */ protected $sslCiphers; + /** @var string sql_mode value to send on connection */ + protected $sqlMode; + /** @var bool Use experimental UTF-8 transmission encoding */ + protected $utf8Mode; + /** @var string|null */ private $serverVersion = null; @@ -82,6 +87,8 @@ abstract class DatabaseMysqlBase extends DatabaseBase { $this->$var = $params[$var]; } } + $this->sqlMode = isset( $params['sqlMode'] ) ? $params['sqlMode'] : ''; + $this->utf8Mode = !empty( $params['utf8Mode'] ); } /** @@ -100,13 +107,9 @@ abstract class DatabaseMysqlBase extends DatabaseBase { * @return bool */ function open( $server, $user, $password, $dbName ) { - global $wgAllDBsAreLocalhost, $wgSQLMode; - # Close/unset connection handle $this->close(); - # Debugging hack -- fake cluster - $realServer = $wgAllDBsAreLocalhost ? 'localhost' : $server; $this->mServer = $server; $this->mUser = $user; $this->mPassword = $password; @@ -114,7 +117,7 @@ abstract class DatabaseMysqlBase extends DatabaseBase { $this->installErrorHandler(); try { - $this->mConn = $this->mysqlConnect( $realServer ); + $this->mConn = $this->mysqlConnect( $this->mServer ); } catch ( Exception $ex ) { $this->restoreErrorHandler(); throw $ex; @@ -126,14 +129,14 @@ abstract class DatabaseMysqlBase extends DatabaseBase { if ( !$error ) { $error = $this->lastError(); } - wfLogDBError( + $this->queryLogger->error( "Error connecting to {db_server}: {error}", $this->getLogContext( [ 'method' => __METHOD__, 'error' => $error, ] ) ); - wfDebug( "DB connection error\n" . + $this->queryLogger->debug( "DB connection error\n" . "Server: $server, User: $user, Password: " . substr( $password, 0, 3 ) . "..., error: " . $error . "\n" ); @@ -145,14 +148,14 @@ abstract class DatabaseMysqlBase extends DatabaseBase { $success = $this->selectDB( $dbName ); MediaWiki\restoreWarnings(); if ( !$success ) { - wfLogDBError( + $this->queryLogger->error( "Error selecting database {db_name} on server {db_server}", $this->getLogContext( [ 'method' => __METHOD__, ] ) ); - wfDebug( "Error selecting database $dbName on server {$this->mServer} " . - "from client host " . wfHostname() . "\n" ); + $this->queryLogger->debug( + "Error selecting database $dbName on server {$this->mServer}" ); $this->reportConnectionError( "Error selecting database $dbName" ); } @@ -166,8 +169,8 @@ abstract class DatabaseMysqlBase extends DatabaseBase { // Abstract over any insane MySQL defaults $set = [ 'group_concat_max_len = 262144' ]; // Set SQL mode, default is turning them all off, can be overridden or skipped with null - if ( is_string( $wgSQLMode ) ) { - $set[] = 'sql_mode = ' . $this->addQuotes( $wgSQLMode ); + if ( is_string( $this->sqlMode ) ) { + $set[] = 'sql_mode = ' . $this->addQuotes( $this->sqlMode ); } // Set any custom settings defined by site config // (e.g. https://dev.mysql.com/doc/refman/4.1/en/innodb-parameters.html) @@ -183,7 +186,7 @@ abstract class DatabaseMysqlBase extends DatabaseBase { // Use doQuery() to avoid opening implicit transactions (DBO_TRX) $success = $this->doQuery( 'SET ' . implode( ', ', $set ) ); if ( !$success ) { - wfLogDBError( + $this->queryLogger->error( 'Error setting MySQL variables on server {db_server} (check $wgSQLMode)', $this->getLogContext( [ 'method' => __METHOD__, @@ -204,9 +207,7 @@ abstract class DatabaseMysqlBase extends DatabaseBase { * @return bool */ protected function connectInitCharset() { - global $wgDBmysql5; - - if ( $wgDBmysql5 ) { + if ( $this->utf8Mode ) { // Tell the server we're communicating with it in UTF-8. // This may engage various charset conversions. return $this->mysqlSetCharset( 'utf8' ); @@ -657,7 +658,7 @@ abstract class DatabaseMysqlBase extends DatabaseBase { // Standard method: use master server ID (works with stock pt-heartbeat) $masterInfo = $this->getMasterServerInfo(); if ( !$masterInfo ) { - wfLogDBError( + $this->queryLogger->error( "Unable to query master of {db_server} for server ID", $this->getLogContext( [ 'method' => __METHOD__ @@ -680,7 +681,7 @@ abstract class DatabaseMysqlBase extends DatabaseBase { return max( $nowUnix - $timeUnix, 0.0 ); } - wfLogDBError( + $this->queryLogger->error( "Unable to find pt-heartbeat row for {db_server}", $this->getLogContext( [ 'method' => __METHOD__ @@ -984,7 +985,7 @@ abstract class DatabaseMysqlBase extends DatabaseBase { return true; } - wfDebug( __METHOD__ . " failed to acquire lock\n" ); + $this->queryLogger->debug( __METHOD__ . " failed to acquire lock\n" ); return false; } @@ -1006,7 +1007,7 @@ abstract class DatabaseMysqlBase extends DatabaseBase { return true; } - wfDebug( __METHOD__ . " failed to release lock\n" ); + $this->queryLogger->debug( __METHOD__ . " failed to release lock\n" ); return false; } diff --git a/includes/db/loadbalancer/LBFactoryMW.php b/includes/db/loadbalancer/LBFactoryMW.php index 2fb48c7f9e..49d0624710 100644 --- a/includes/db/loadbalancer/LBFactoryMW.php +++ b/includes/db/loadbalancer/LBFactoryMW.php @@ -37,7 +37,7 @@ abstract class LBFactoryMW extends LBFactory implements DestructibleService { * @TODO: inject objects via dependency framework */ public function __construct( array $conf ) { - global $wgCommandLineMode; + global $wgCommandLineMode, $wgSQLMode, $wgDBmysql5; $defaults = [ 'localDomain' => wfWikiID(), @@ -66,6 +66,11 @@ abstract class LBFactoryMW extends LBFactory implements DestructibleService { $this->agent = isset( $params['agent'] ) ? $params['agent'] : ''; $this->cliMode = isset( $params['cliMode'] ) ? $params['cliMode'] : $wgCommandLineMode; + if ( isset( $conf['serverTemplate'] ) ) { // LBFactoryMulti + $conf['serverTemplate']['sqlMode'] = $wgSQLMode; + $conf['serverTemplate']['utf8Mode'] = $wgDBmysql5; + } + parent::__construct( $conf + $defaults ); } diff --git a/includes/db/loadbalancer/LBFactorySimple.php b/includes/db/loadbalancer/LBFactorySimple.php index 09533eb135..d937ddb45d 100644 --- a/includes/db/loadbalancer/LBFactorySimple.php +++ b/includes/db/loadbalancer/LBFactorySimple.php @@ -46,7 +46,7 @@ class LBFactorySimple extends LBFactoryMW { * @return LoadBalancer */ public function newMainLB( $wiki = false ) { - global $wgDBservers, $wgDBprefix, $wgDBmwschema; + global $wgDBservers, $wgDBprefix, $wgDBmwschema, $wgSQLMode, $wgDBmysql5; if ( is_array( $wgDBservers ) ) { $servers = $wgDBservers; @@ -59,7 +59,9 @@ class LBFactorySimple extends LBFactoryMW { $server += [ 'schema' => $wgDBmwschema, 'tablePrefix' => $wgDBprefix, - 'flags' => DBO_DEFAULT + 'flags' => DBO_DEFAULT, + 'sqlMode' => $wgSQLMode, + 'utf8Mode' => $wgDBmysql5 ]; } } else { @@ -87,7 +89,9 @@ class LBFactorySimple extends LBFactoryMW { 'type' => $wgDBtype, 'load' => 1, 'flags' => $flags, - 'master' => true + 'master' => true, + 'sqlMode' => $wgSQLMode, + 'utf8Mode' => $wgDBmysql5 ] ]; } diff --git a/includes/db/Database.php b/includes/libs/rdbms/database/DatabaseBase.php similarity index 100% rename from includes/db/Database.php rename to includes/libs/rdbms/database/DatabaseBase.php diff --git a/includes/libs/rdbms/lbfactory/LBFactory.php b/includes/libs/rdbms/lbfactory/LBFactory.php index e011ff0671..49fac6afc2 100644 --- a/includes/libs/rdbms/lbfactory/LBFactory.php +++ b/includes/libs/rdbms/lbfactory/LBFactory.php @@ -445,8 +445,7 @@ abstract class LBFactory { $failed = []; foreach ( $lbs as $i => $lb ) { if ( $masterPositions[$i] ) { - // The DBMS may not support getMasterPos() or the whole - // load balancer might be fake (e.g. $wgAllDBsAreLocalhost). + // The DBMS may not support getMasterPos() if ( !$lb->waitForAll( $masterPositions[$i], $opts['timeout'] ) ) { $failed[] = $lb->getServerName( $lb->getWriterIndex() ); } -- 2.20.1