From: jenkins-bot Date: Fri, 18 Aug 2017 01:38:01 +0000 (+0000) Subject: Merge "rdbms: Support secondary autocommit connections in LoadBalancer" X-Git-Tag: 1.31.0-rc.0~2367 X-Git-Url: http://git.cyclocoop.org/%7B%24admin_url%7Dmes_infos.php?a=commitdiff_plain;h=3ebc79c08a9832e26147d5b27967ca9668b80328;hp=149c2ada6920db63c3fb8569eb2511f9afb92311;p=lhc%2Fweb%2Fwiklou.git Merge "rdbms: Support secondary autocommit connections in LoadBalancer" --- diff --git a/includes/jobqueue/JobQueueDB.php b/includes/jobqueue/JobQueueDB.php index b7cc133a75..b5f331b35f 100644 --- a/includes/jobqueue/JobQueueDB.php +++ b/includes/jobqueue/JobQueueDB.php @@ -184,15 +184,22 @@ class JobQueueDB extends JobQueue { * @return void */ protected function doBatchPush( array $jobs, $flags ) { - DeferredUpdates::addUpdate( - new AutoCommitUpdate( - $this->getMasterDB(), - __METHOD__, - function ( IDatabase $dbw, $fname ) use ( $jobs, $flags ) { - $this->doBatchPushInternal( $dbw, $jobs, $flags, $fname ); - } - ), - DeferredUpdates::PRESEND + $dbw = $this->getMasterDB(); + // In general, there will be two cases here: + // a) sqlite; DB connection is probably a regular round-aware handle. + // If the connection is busy with a transaction, then defer the job writes + // until right before the main round commit step. Any errors that bubble + // up will rollback the main commit round. + // b) mysql/postgres; DB connection is generally a separate CONN_TRX_AUTO handle. + // No transaction is active nor will be started by writes, so enqueue the jobs + // now so that any errors will show up immediately as the interface expects. Any + // errors that bubble up will rollback the main commit round. + $fname = __METHOD__; + $dbw->onTransactionPreCommitOrIdle( + function () use ( $dbw, $jobs, $flags, $fname ) { + $this->doBatchPushInternal( $dbw, $jobs, $flags, $fname ); + }, + $fname ); } @@ -771,7 +778,12 @@ class JobQueueDB extends JobQueue { ? $lbFactory->getExternalLB( $this->cluster ) : $lbFactory->getMainLB( $this->wiki ); - return $lb->getConnectionRef( $index, [], $this->wiki ); + return ( $lb->getServerType( $lb->getWriterIndex() ) !== 'sqlite' ) + // Keep a separate connection to avoid contention and deadlocks; + // However, SQLite has the opposite behavior due to DB-level locking. + ? $lb->getConnectionRef( $index, [], $this->wiki, $lb::CONN_TRX_AUTO ) + // Jobs insertion will be defered until the PRESEND stage to reduce contention. + : $lb->getConnectionRef( $index, [], $this->wiki ); } /** diff --git a/includes/libs/rdbms/database/DBConnRef.php b/includes/libs/rdbms/database/DBConnRef.php index eb0e954e16..ef2953ec9c 100644 --- a/includes/libs/rdbms/database/DBConnRef.php +++ b/includes/libs/rdbms/database/DBConnRef.php @@ -23,16 +23,17 @@ class DBConnRef implements IDatabase { const FLD_INDEX = 0; const FLD_GROUP = 1; const FLD_DOMAIN = 2; + const FLD_FLAGS = 3; /** * @param ILoadBalancer $lb Connection manager for $conn - * @param Database|array $conn New connection handle or (server index, query groups, domain) + * @param Database|array $conn Database handle or (server index, query groups, domain, flags) */ public function __construct( ILoadBalancer $lb, $conn ) { $this->lb = $lb; if ( $conn instanceof Database ) { $this->conn = $conn; // live handle - } elseif ( count( $conn ) >= 3 && $conn[self::FLD_DOMAIN] !== false ) { + } elseif ( count( $conn ) >= 4 && $conn[self::FLD_DOMAIN] !== false ) { $this->params = $conn; } else { throw new InvalidArgumentException( "Missing lazy connection arguments." ); @@ -41,8 +42,8 @@ class DBConnRef implements IDatabase { function __call( $name, array $arguments ) { if ( $this->conn === null ) { - list( $db, $groups, $wiki ) = $this->params; - $this->conn = $this->lb->getConnection( $db, $groups, $wiki ); + list( $db, $groups, $wiki, $flags ) = $this->params; + $this->conn = $this->lb->getConnection( $db, $groups, $wiki, $flags ); } return call_user_func_array( [ $this->conn, $name ], $arguments ); diff --git a/includes/libs/rdbms/loadbalancer/ILoadBalancer.php b/includes/libs/rdbms/loadbalancer/ILoadBalancer.php index c9403929a0..22a58055d7 100644 --- a/includes/libs/rdbms/loadbalancer/ILoadBalancer.php +++ b/includes/libs/rdbms/loadbalancer/ILoadBalancer.php @@ -76,14 +76,17 @@ use InvalidArgumentException; * @ingroup Database */ interface ILoadBalancer { - /** @var integer Request a replica DB connection */ + /** @var int Request a replica DB connection */ const DB_REPLICA = -1; - /** @var integer Request a master DB connection */ + /** @var int Request a master DB connection */ const DB_MASTER = -2; /** @var string Domain specifier when no specific database needs to be selected */ const DOMAIN_ANY = ''; + /** @var int DB handle should have DBO_TRX disabled and the caller will leave it as such */ + const CONN_TRX_AUTO = 1; + /** * Construct a manager of IDatabase connection objects * @@ -168,14 +171,17 @@ interface ILoadBalancer { /** * Get a connection by index * + * Avoid using CONN_TRX_AUTO with sqlite (e.g. check getServerType() first) + * * @param int $i Server index or DB_MASTER/DB_REPLICA * @param array|string|bool $groups Query group(s), or false for the generic reader * @param string|bool $domain Domain ID, or false for the current domain + * @param int $flags Bitfield of CONN_* class constants * * @throws DBError * @return Database */ - public function getConnection( $i, $groups = [], $domain = false ); + public function getConnection( $i, $groups = [], $domain = false, $flags = 0 ); /** * Mark a foreign connection as being available for reuse under a different DB domain @@ -193,42 +199,51 @@ interface ILoadBalancer { * * The handle's methods simply wrap those of a Database handle * + * Avoid using CONN_TRX_AUTO with sqlite (e.g. check getServerType() first) + * * @see ILoadBalancer::getConnection() for parameter information * * @param int $i Server index or DB_MASTER/DB_REPLICA * @param array|string|bool $groups Query group(s), or false for the generic reader * @param string|bool $domain Domain ID, or false for the current domain + * @param int $flags Bitfield of CONN_* class constants (e.g. CONN_TRX_AUTO) * @return DBConnRef */ - public function getConnectionRef( $i, $groups = [], $domain = false ); + public function getConnectionRef( $i, $groups = [], $domain = false, $flags = 0 ); /** * Get a database connection handle reference without connecting yet * * The handle's methods simply wrap those of a Database handle * + * Avoid using CONN_TRX_AUTO with sqlite (e.g. check getServerType() first) + * * @see ILoadBalancer::getConnection() for parameter information * * @param int $i Server index or DB_MASTER/DB_REPLICA * @param array|string|bool $groups Query group(s), or false for the generic reader * @param string|bool $domain Domain ID, or false for the current domain + * @param int $flags Bitfield of CONN_* class constants (e.g. CONN_TRX_AUTO) * @return DBConnRef */ - public function getLazyConnectionRef( $i, $groups = [], $domain = false ); + public function getLazyConnectionRef( $i, $groups = [], $domain = false, $flags = 0 ); /** * Get a maintenance database connection handle reference for migrations and schema changes * * The handle's methods simply wrap those of a Database handle * + * Avoid using CONN_TRX_AUTO with sqlite (e.g. check getServerType() first) + * * @see ILoadBalancer::getConnection() for parameter information * * @param int $db Server index or DB_MASTER/DB_REPLICA * @param array|string|bool $groups Query group(s), or false for the generic reader * @param string|bool $domain Domain ID, or false for the current domain + * @param int $flags Bitfield of CONN_* class constants (e.g. CONN_TRX_AUTO) * @return MaintainableDBConnRef */ - public function getMaintenanceConnectionRef( $db, $groups = [], $domain = false ); + public function getMaintenanceConnectionRef( $db, $groups = [], $domain = false, $flags = 0 ); /** * Open a connection to the server given by the specified index @@ -236,14 +251,17 @@ interface ILoadBalancer { * The index must be an actual index into the array. If a connection to the server is * already open and not considered an "in use" foreign connection, this simply returns it. * + * Avoid using CONN_TRX_AUTO with sqlite (e.g. check getServerType() first) + * * @note If disable() was called on this LoadBalancer, this method will throw a DBAccessError. * * @param int $i Server index (does not support DB_MASTER/DB_REPLICA) * @param string|bool $domain Domain ID, or false for the current domain + * @param int $flags Bitfield of CONN_* class constants (e.g. CONN_TRX_AUTO) * @return Database|bool Returns false on errors * @throws DBAccessError */ - public function openConnection( $i, $domain = false ); + public function openConnection( $i, $domain = false, $flags = 0 ); /** * @return int @@ -253,7 +271,7 @@ interface ILoadBalancer { /** * Returns true if the specified index is a valid server index * - * @param string $i + * @param int $i * @return bool */ public function haveIndex( $i ); @@ -261,7 +279,7 @@ interface ILoadBalancer { /** * Returns true if the specified index is valid and has non-zero load * - * @param string $i + * @param int $i * @return bool */ public function isNonZeroLoad( $i ); @@ -275,12 +293,21 @@ interface ILoadBalancer { /** * Get the host name or IP address of the server with the specified index - * Prefer a readable name if available. - * @param string $i - * @return string + * + * @param int $i + * @return string Readable name if available or IP/host otherwise */ public function getServerName( $i ); + /** + * Get DB type of the server with the specified index + * + * @param int $i + * @return string One of (mysql,postgres,sqlite,...) or "unknown" for bad indexes + * @since 1.30 + */ + public function getServerType( $i ); + /** * Return the server info structure for a given index, or false if the index is invalid. * @param int $i diff --git a/includes/libs/rdbms/loadbalancer/LoadBalancer.php b/includes/libs/rdbms/loadbalancer/LoadBalancer.php index 56a7fbb981..1665a5ef7d 100644 --- a/includes/libs/rdbms/loadbalancer/LoadBalancer.php +++ b/includes/libs/rdbms/loadbalancer/LoadBalancer.php @@ -41,7 +41,7 @@ use Exception; class LoadBalancer implements ILoadBalancer { /** @var array[] Map of (server index => server config array) */ private $mServers; - /** @var Database[][][] Map of local/foreignUsed/foreignFree => server index => IDatabase[] */ + /** @var Database[][][] Map of (connection category => server index => IDatabase[]) */ private $mConns; /** @var float[] Map of (server index => weight) */ private $mLoads; @@ -128,11 +128,22 @@ class LoadBalancer implements ILoadBalancer { const KEY_FOREIGN_FREE = 'foreignFree'; const KEY_FOREIGN_INUSE = 'foreignInUse'; + const KEY_LOCAL_NOROUND = 'localAutoCommit'; + const KEY_FOREIGN_FREE_NOROUND = 'foreignFreeAutoCommit'; + const KEY_FOREIGN_INUSE_NOROUND = 'foreignInUseAutoCommit'; + public function __construct( array $params ) { if ( !isset( $params['servers'] ) ) { throw new InvalidArgumentException( __CLASS__ . ': missing servers parameter' ); } $this->mServers = $params['servers']; + foreach ( $this->mServers as $i => $server ) { + if ( $i == 0 ) { + $this->mServers[$i]['master'] = true; + } else { + $this->mServers[$i]['replica'] = true; + } + } $this->localDomain = isset( $params['localDomain'] ) ? DatabaseDomain::newFromId( $params['localDomain'] ) @@ -150,9 +161,14 @@ class LoadBalancer implements ILoadBalancer { $this->mReadIndex = -1; $this->mConns = [ + // Connection were transaction rounds may be applied self::KEY_LOCAL => [], self::KEY_FOREIGN_INUSE => [], - self::KEY_FOREIGN_FREE => [] + self::KEY_FOREIGN_FREE => [], + // Auto-committing counterpart connections that ignore transaction rounds + self::KEY_LOCAL_NOROUND => [], + self::KEY_FOREIGN_INUSE_NOROUND => [], + self::KEY_FOREIGN_FREE_NOROUND => [] ]; $this->mLoads = []; $this->mWaitForPos = false; @@ -601,16 +617,7 @@ class LoadBalancer implements ILoadBalancer { return $ok; } - /** - * @see ILoadBalancer::getConnection() - * - * @param int $i - * @param array $groups - * @param bool $domain - * @return Database - * @throws DBConnectionError - */ - public function getConnection( $i, $groups = [], $domain = false ) { + public function getConnection( $i, $groups = [], $domain = false, $flags = 0 ) { if ( $i === null || $i === false ) { throw new InvalidArgumentException( 'Attempt to call ' . __METHOD__ . ' with invalid server index' ); @@ -657,7 +664,7 @@ class LoadBalancer implements ILoadBalancer { } # Now we have an explicit index into the servers array - $conn = $this->openConnection( $i, $domain ); + $conn = $this->openConnection( $i, $domain, $flags ); if ( !$conn ) { // Throw an exception $this->reportConnectionError(); @@ -707,20 +714,29 @@ class LoadBalancer implements ILoadBalancer { return; // DBConnRef handle probably survived longer than the LoadBalancer } + if ( $conn->getLBInfo( 'autoCommitOnly' ) ) { + $connFreeKey = self::KEY_FOREIGN_FREE_NOROUND; + $connInUseKey = self::KEY_FOREIGN_INUSE_NOROUND; + } else { + $connFreeKey = self::KEY_FOREIGN_FREE; + $connInUseKey = self::KEY_FOREIGN_INUSE; + } + $domain = $conn->getDomainID(); - if ( !isset( $this->mConns[self::KEY_FOREIGN_INUSE][$serverIndex][$domain] ) ) { + if ( !isset( $this->mConns[$connInUseKey][$serverIndex][$domain] ) ) { throw new InvalidArgumentException( __METHOD__ . ": connection $serverIndex/$domain not found; it may have already been freed." ); - } elseif ( $this->mConns[self::KEY_FOREIGN_INUSE][$serverIndex][$domain] !== $conn ) { + } elseif ( $this->mConns[$connInUseKey][$serverIndex][$domain] !== $conn ) { throw new InvalidArgumentException( __METHOD__ . ": connection $serverIndex/$domain mismatched; it may have already been freed." ); } + $conn->setLBInfo( 'foreignPoolRefCount', --$refCount ); if ( $refCount <= 0 ) { - $this->mConns[self::KEY_FOREIGN_FREE][$serverIndex][$domain] = $conn; - unset( $this->mConns[self::KEY_FOREIGN_INUSE][$serverIndex][$domain] ); - if ( !$this->mConns[self::KEY_FOREIGN_INUSE][$serverIndex] ) { - unset( $this->mConns[ self::KEY_FOREIGN_INUSE ][$serverIndex] ); // clean up + $this->mConns[$connFreeKey][$serverIndex][$domain] = $conn; + unset( $this->mConns[$connInUseKey][$serverIndex][$domain] ); + if ( !$this->mConns[$connInUseKey][$serverIndex] ) { + unset( $this->mConns[$connInUseKey][$serverIndex] ); // clean up } $this->connLogger->debug( __METHOD__ . ": freed connection $serverIndex/$domain" ); } else { @@ -729,33 +745,26 @@ class LoadBalancer implements ILoadBalancer { } } - public function getConnectionRef( $db, $groups = [], $domain = false ) { + public function getConnectionRef( $db, $groups = [], $domain = false, $flags = 0 ) { $domain = ( $domain !== false ) ? $domain : $this->localDomain; - return new DBConnRef( $this, $this->getConnection( $db, $groups, $domain ) ); + return new DBConnRef( $this, $this->getConnection( $db, $groups, $domain, $flags ) ); } - public function getLazyConnectionRef( $db, $groups = [], $domain = false ) { + public function getLazyConnectionRef( $db, $groups = [], $domain = false, $flags = 0 ) { $domain = ( $domain !== false ) ? $domain : $this->localDomain; - return new DBConnRef( $this, [ $db, $groups, $domain ] ); + return new DBConnRef( $this, [ $db, $groups, $domain, $flags ] ); } - public function getMaintenanceConnectionRef( $db, $groups = [], $domain = false ) { + public function getMaintenanceConnectionRef( $db, $groups = [], $domain = false, $flags = 0 ) { $domain = ( $domain !== false ) ? $domain : $this->localDomain; - return new MaintainableDBConnRef( $this, $this->getConnection( $db, $groups, $domain ) ); + return new MaintainableDBConnRef( + $this, $this->getConnection( $db, $groups, $domain, $flags ) ); } - /** - * @see ILoadBalancer::openConnection() - * - * @param int $i - * @param bool $domain - * @return bool|Database - * @throws DBAccessError - */ - public function openConnection( $i, $domain = false ) { + public function openConnection( $i, $domain = false, $flags = 0 ) { if ( $this->localDomain->equals( $domain ) || $domain === $this->localDomainIdAlias ) { $domain = false; // local connection requested } @@ -767,26 +776,38 @@ class LoadBalancer implements ILoadBalancer { $this->chronProt->initLB( $this ); } + // Check if an auto-commit connection is being requested. If so, it will not reuse the + // main set of DB connections but rather its own pool since: + // a) those are usually set to implicitly use transaction rounds via DBO_TRX + // b) those must support the use of explicit transaction rounds via beginMasterChanges() + $autoCommit = ( ( $flags & self::CONN_TRX_AUTO ) == self::CONN_TRX_AUTO ); + if ( $domain !== false ) { - $conn = $this->openForeignConnection( $i, $domain ); - } elseif ( isset( $this->mConns[self::KEY_LOCAL][$i][0] ) ) { - $conn = $this->mConns[self::KEY_LOCAL][$i][0]; + // Connection is to a foreign domain + $conn = $this->openForeignConnection( $i, $domain, $flags ); } else { - if ( !isset( $this->mServers[$i] ) || !is_array( $this->mServers[$i] ) ) { - throw new InvalidArgumentException( "No server with index '$i'." ); - } - // Open a new connection - $server = $this->mServers[$i]; - $server['serverIndex'] = $i; - $conn = $this->reallyOpenConnection( $server, false ); - $serverName = $this->getServerName( $i ); - if ( $conn->isOpen() ) { - $this->connLogger->debug( "Connected to database $i at '$serverName'." ); - $this->mConns[self::KEY_LOCAL][$i][0] = $conn; + // Connection is to the local domain + $connKey = $autoCommit ? self::KEY_LOCAL_NOROUND : self::KEY_LOCAL; + if ( isset( $this->mConns[$connKey][$i][0] ) ) { + $conn = $this->mConns[$connKey][$i][0]; } else { - $this->connLogger->warning( "Failed to connect to database $i at '$serverName'." ); - $this->errorConnection = $conn; - $conn = false; + if ( !isset( $this->mServers[$i] ) || !is_array( $this->mServers[$i] ) ) { + throw new InvalidArgumentException( "No server with index '$i'." ); + } + // Open a new connection + $server = $this->mServers[$i]; + $server['serverIndex'] = $i; + $server['autoCommitOnly'] = $autoCommit; + $conn = $this->reallyOpenConnection( $server, false ); + $host = $this->getServerName( $i ); + if ( $conn->isOpen() ) { + $this->connLogger->debug( "Connected to database $i at '$host'." ); + $this->mConns[$connKey][$i][0] = $conn; + } else { + $this->connLogger->warning( "Failed to connect to database $i at '$host'." ); + $this->errorConnection = $conn; + $conn = false; + } } } @@ -799,6 +820,10 @@ class LoadBalancer implements ILoadBalancer { $conn = false; } + if ( $autoCommit && $conn instanceof IDatabase ) { + $conn->clearFlag( $conn::DBO_TRX ); // auto-commit mode + } + return $conn; } @@ -820,27 +845,37 @@ class LoadBalancer implements ILoadBalancer { * * @param int $i Server index * @param string $domain Domain ID to open + * @param integer $flags Class CONN_* constant bitfield * @return Database */ - private function openForeignConnection( $i, $domain ) { + private function openForeignConnection( $i, $domain, $flags = 0 ) { $domainInstance = DatabaseDomain::newFromId( $domain ); $dbName = $domainInstance->getDatabase(); $prefix = $domainInstance->getTablePrefix(); + $autoCommit = ( ( $flags & self::CONN_TRX_AUTO ) == self::CONN_TRX_AUTO ); + + if ( $autoCommit ) { + $connFreeKey = self::KEY_FOREIGN_FREE_NOROUND; + $connInUseKey = self::KEY_FOREIGN_INUSE_NOROUND; + } else { + $connFreeKey = self::KEY_FOREIGN_FREE; + $connInUseKey = self::KEY_FOREIGN_INUSE; + } - if ( isset( $this->mConns[self::KEY_FOREIGN_INUSE][$i][$domain] ) ) { - // Reuse an in-use connection for the same domain that is not in-use - $conn = $this->mConns[self::KEY_FOREIGN_INUSE][$i][$domain]; + if ( isset( $this->mConns[$connInUseKey][$i][$domain] ) ) { + // Reuse an in-use connection for the same domain + $conn = $this->mConns[$connInUseKey][$i][$domain]; $this->connLogger->debug( __METHOD__ . ": reusing connection $i/$domain" ); - } elseif ( isset( $this->mConns[self::KEY_FOREIGN_FREE][$i][$domain] ) ) { - // Reuse a free connection for the same domain that is not in-use - $conn = $this->mConns[self::KEY_FOREIGN_FREE][$i][$domain]; - unset( $this->mConns[self::KEY_FOREIGN_FREE][$i][$domain] ); - $this->mConns[self::KEY_FOREIGN_INUSE][$i][$domain] = $conn; + } elseif ( isset( $this->mConns[$connFreeKey][$i][$domain] ) ) { + // Reuse a free connection for the same domain + $conn = $this->mConns[$connFreeKey][$i][$domain]; + unset( $this->mConns[$connFreeKey][$i][$domain] ); + $this->mConns[$connInUseKey][$i][$domain] = $conn; $this->connLogger->debug( __METHOD__ . ": reusing free connection $i/$domain" ); - } elseif ( !empty( $this->mConns[self::KEY_FOREIGN_FREE][$i] ) ) { - // Reuse a connection from another domain - $conn = reset( $this->mConns[self::KEY_FOREIGN_FREE][$i] ); - $oldDomain = key( $this->mConns[self::KEY_FOREIGN_FREE][$i] ); + } elseif ( !empty( $this->mConns[$connFreeKey][$i] ) ) { + // Reuse a free connection from another domain + $conn = reset( $this->mConns[$connFreeKey][$i] ); + $oldDomain = key( $this->mConns[$connFreeKey][$i] ); // The empty string as a DB name means "don't care". // DatabaseMysqlBase::open() already handle this on connection. if ( strlen( $dbName ) && !$conn->selectDB( $dbName ) ) { @@ -850,8 +885,8 @@ class LoadBalancer implements ILoadBalancer { $conn = false; } else { $conn->tablePrefix( $prefix ); - unset( $this->mConns[self::KEY_FOREIGN_FREE][$i][$oldDomain] ); - $this->mConns[self::KEY_FOREIGN_INUSE][$i][$domain] = $conn; + unset( $this->mConns[$connFreeKey][$i][$oldDomain] ); + $this->mConns[$connInUseKey][$i][$domain] = $conn; $this->connLogger->debug( __METHOD__ . ": reusing free connection from $oldDomain for $domain" ); } @@ -864,6 +899,7 @@ class LoadBalancer implements ILoadBalancer { $server['serverIndex'] = $i; $server['foreignPoolRefCount'] = 0; $server['foreign'] = true; + $server['autoCommitOnly'] = $autoCommit; $conn = $this->reallyOpenConnection( $server, $dbName ); if ( !$conn->isOpen() ) { $this->connLogger->warning( __METHOD__ . ": connection error for $i/$domain" ); @@ -871,7 +907,7 @@ class LoadBalancer implements ILoadBalancer { $conn = false; } else { $conn->tablePrefix( $prefix ); - $this->mConns[self::KEY_FOREIGN_INUSE][$i][$domain] = $conn; + $this->mConns[$connInUseKey][$i][$domain] = $conn; $this->connLogger->debug( __METHOD__ . ": opened new connection for $i/$domain" ); } } @@ -1030,6 +1066,10 @@ class LoadBalancer implements ILoadBalancer { return ( $name != '' ) ? $name : 'localhost'; } + public function getServerType( $i ) { + return isset( $this->mServers[$i]['type'] ) ? $this->mServers[$i]['type'] : 'unknown'; + } + /** * @deprecated Since 1.30, no alternative */ @@ -1083,8 +1123,11 @@ class LoadBalancer implements ILoadBalancer { $this->mConns = [ self::KEY_LOCAL => [], - self::KEY_FOREIGN_FREE => [], self::KEY_FOREIGN_INUSE => [], + self::KEY_FOREIGN_FREE => [], + self::KEY_LOCAL_NOROUND => [], + self::KEY_FOREIGN_INUSE_NOROUND => [], + self::KEY_FOREIGN_FREE_NOROUND => [] ]; $this->connsOpened = 0; } @@ -1304,6 +1347,10 @@ class LoadBalancer implements ILoadBalancer { * @param IDatabase $conn */ private function applyTransactionRoundFlags( IDatabase $conn ) { + if ( $conn->getLBInfo( 'autoCommitOnly' ) ) { + return; // transaction rounds do not apply to these connections + } + if ( $conn->getFlag( $conn::DBO_DEFAULT ) ) { // DBO_TRX is controlled entirely by CLI mode presence with DBO_DEFAULT. // Force DBO_TRX even in CLI mode since a commit round is expected soon. @@ -1318,6 +1365,10 @@ class LoadBalancer implements ILoadBalancer { * @param IDatabase $conn */ private function undoTransactionRoundFlags( IDatabase $conn ) { + if ( $conn->getLBInfo( 'autoCommitOnly' ) ) { + return; // transaction rounds do not apply to these connections + } + if ( $conn->getFlag( $conn::DBO_DEFAULT ) ) { $conn->restoreFlags( $conn::RESTORE_PRIOR ); } diff --git a/includes/objectcache/SqlBagOStuff.php b/includes/objectcache/SqlBagOStuff.php index 3cce530c3c..2cfd2a1d76 100644 --- a/includes/objectcache/SqlBagOStuff.php +++ b/includes/objectcache/SqlBagOStuff.php @@ -145,27 +145,11 @@ class SqlBagOStuff extends BagOStuff { $this->replicaOnly = !empty( $params['slaveOnly'] ); } - protected function getSeparateMainLB() { - global $wgDBtype; - - if ( $this->usesMainDB() && $wgDBtype !== 'sqlite' ) { - if ( !$this->separateMainLB ) { - // We must keep a separate connection to MySQL in order to avoid deadlocks - $lbFactory = MediaWikiServices::getInstance()->getDBLoadBalancerFactory(); - $this->separateMainLB = $lbFactory->newMainLB(); - } - return $this->separateMainLB; - } else { - // However, SQLite has an opposite behavior due to DB-level locking - return null; - } - } - /** * Get a connection to the specified database * * @param int $serverIndex - * @return IDatabase + * @return Database * @throws MWException */ protected function getDB( $serverIndex ) { @@ -181,8 +165,8 @@ class SqlBagOStuff extends BagOStuff { throw $this->connFailureErrors[$serverIndex]; } - # If server connection info was given, use that if ( $this->serverInfos ) { + // Use custom database defined by server connection info $info = $this->serverInfos[$serverIndex]; $type = isset( $info['type'] ) ? $info['type'] : 'mysql'; $host = isset( $info['host'] ) ? $info['host'] : '[unknown]'; @@ -190,17 +174,22 @@ class SqlBagOStuff extends BagOStuff { // Use a blank trx profiler to ignore expections as this is a cache $info['trxProfiler'] = new TransactionProfiler(); $db = Database::factory( $type, $info ); - $db->clearFlag( DBO_TRX ); + $db->clearFlag( DBO_TRX ); // auto-commit mode } else { + // Use the main LB database + $lb = MediaWikiServices::getInstance()->getDBLoadBalancer(); $index = $this->replicaOnly ? DB_REPLICA : DB_MASTER; - if ( $this->getSeparateMainLB() ) { - $db = $this->getSeparateMainLB()->getConnection( $index ); - $db->clearFlag( DBO_TRX ); // auto-commit mode + if ( $lb->getServerType( $lb->getWriterIndex() ) !== 'sqlite' ) { + // Keep a separate connection to avoid contention and deadlocks + $db = $lb->getConnection( $index, [], false, $lb::CONN_TRX_AUTO ); + // @TODO: Use a blank trx profiler to ignore expections as this is a cache } else { - $db = wfGetDB( $index ); - // Can't mess with transaction rounds (DBO_TRX) :( + // However, SQLite has the opposite behavior due to DB-level locking. + // Stock sqlite MediaWiki installs use a separate sqlite cache DB instead. + $db = $lb->getConnection( $index ); } } + $this->logger->debug( sprintf( "Connection %s will be used for SqlBagOStuff", $db ) ); $this->conns[$serverIndex] = $db; } @@ -812,9 +801,7 @@ class SqlBagOStuff extends BagOStuff { return true; } - $lb = $this->getSeparateMainLB() - ?: MediaWikiServices::getInstance()->getDBLoadBalancer(); - + $lb = MediaWikiServices::getInstance()->getDBLoadBalancer(); if ( $lb->getServerCount() <= 1 ) { return true; // no replica DBs } diff --git a/tests/phpunit/includes/db/LoadBalancerTest.php b/tests/phpunit/includes/db/LoadBalancerTest.php new file mode 100644 index 0000000000..f8ab7f481b --- /dev/null +++ b/tests/phpunit/includes/db/LoadBalancerTest.php @@ -0,0 +1,135 @@ + $wgDBserver, + 'dbname' => $wgDBname, + 'user' => $wgDBuser, + 'password' => $wgDBpassword, + 'type' => $wgDBtype, + 'dbDirectory' => $wgSQLiteDataDir, + 'load' => 0, + 'flags' => DBO_TRX // REPEATABLE-READ for consistency + ], + ]; + + $lb = new LoadBalancer( [ + 'servers' => $servers, + 'localDomain' => wfWikiID() + ] ); + + $dbw = $lb->getConnection( DB_MASTER ); + $this->assertTrue( $dbw->getLBInfo( 'master' ), 'master shows as master' ); + $this->assertTrue( $dbw->getFlag( $dbw::DBO_TRX ), "DBO_TRX set on master" ); + + $dbr = $lb->getConnection( DB_REPLICA ); + $this->assertTrue( $dbr->getLBInfo( 'master' ), 'DB_REPLICA also gets the master' ); + $this->assertTrue( $dbw->getFlag( $dbw::DBO_TRX ), "DBO_TRX set on replica" ); + + $dbwAuto = $lb->getConnection( DB_MASTER, [], false, $lb::CONN_TRX_AUTO ); + $this->assertFalse( $dbwAuto->getFlag( $dbw::DBO_TRX ), "No DBO_TRX with CONN_TRX_AUTO" ); + $this->assertTrue( $dbw->getFlag( $dbw::DBO_TRX ), "DBO_TRX still set on master" ); + $this->assertNotEquals( $dbw, $dbwAuto, "CONN_TRX_AUTO uses separate connection" ); + + $dbrAuto = $lb->getConnection( DB_REPLICA, [], false, $lb::CONN_TRX_AUTO ); + $this->assertFalse( $dbrAuto->getFlag( $dbw::DBO_TRX ), "No DBO_TRX with CONN_TRX_AUTO" ); + $this->assertTrue( $dbr->getFlag( $dbw::DBO_TRX ), "DBO_TRX still set on replica" ); + $this->assertNotEquals( $dbr, $dbrAuto, "CONN_TRX_AUTO uses separate connection" ); + + $dbwAuto2 = $lb->getConnection( DB_MASTER, [], false, $lb::CONN_TRX_AUTO ); + $this->assertEquals( $dbwAuto2, $dbwAuto, "CONN_TRX_AUTO reuses connections" ); + + $lb->closeAll(); + } + + public function testLBSimpleServers() { + global $wgDBserver, $wgDBname, $wgDBuser, $wgDBpassword, $wgDBtype, $wgSQLiteDataDir; + + $servers = [ + [ // master + 'host' => $wgDBserver, + 'dbname' => $wgDBname, + 'user' => $wgDBuser, + 'password' => $wgDBpassword, + 'type' => $wgDBtype, + 'dbDirectory' => $wgSQLiteDataDir, + 'load' => 0, + 'flags' => DBO_TRX // REPEATABLE-READ for consistency + ], + [ // emulated slave + 'host' => $wgDBserver, + 'dbname' => $wgDBname, + 'user' => $wgDBuser, + 'password' => $wgDBpassword, + 'type' => $wgDBtype, + 'dbDirectory' => $wgSQLiteDataDir, + 'load' => 100, + 'flags' => DBO_TRX // REPEATABLE-READ for consistency + ] + ]; + + $lb = new LoadBalancer( [ + 'servers' => $servers, + 'localDomain' => wfWikiID(), + 'loadMonitorClass' => 'LoadMonitorNull' + ] ); + + $dbw = $lb->getConnection( DB_MASTER ); + $this->assertTrue( $dbw->getLBInfo( 'master' ), 'master shows as master' ); + $this->assertEquals( + ( $wgDBserver != '' ) ? $wgDBserver : 'localhost', + $dbw->getLBInfo( 'clusterMasterHost' ), + 'cluster master set' ); + $this->assertTrue( $dbw->getFlag( $dbw::DBO_TRX ), "DBO_TRX set on master" ); + + $dbr = $lb->getConnection( DB_REPLICA ); + $this->assertTrue( $dbr->getLBInfo( 'replica' ), 'slave shows as slave' ); + $this->assertEquals( + ( $wgDBserver != '' ) ? $wgDBserver : 'localhost', + $dbr->getLBInfo( 'clusterMasterHost' ), + 'cluster master set' ); + $this->assertTrue( $dbw->getFlag( $dbw::DBO_TRX ), "DBO_TRX set on replica" ); + + $dbwAuto = $lb->getConnection( DB_MASTER, [], false, $lb::CONN_TRX_AUTO ); + $this->assertFalse( $dbwAuto->getFlag( $dbw::DBO_TRX ), "No DBO_TRX with CONN_TRX_AUTO" ); + $this->assertTrue( $dbw->getFlag( $dbw::DBO_TRX ), "DBO_TRX still set on master" ); + $this->assertNotEquals( $dbw, $dbwAuto, "CONN_TRX_AUTO uses separate connection" ); + + $dbrAuto = $lb->getConnection( DB_REPLICA, [], false, $lb::CONN_TRX_AUTO ); + $this->assertFalse( $dbrAuto->getFlag( $dbw::DBO_TRX ), "No DBO_TRX with CONN_TRX_AUTO" ); + $this->assertTrue( $dbr->getFlag( $dbw::DBO_TRX ), "DBO_TRX still set on replica" ); + $this->assertNotEquals( $dbr, $dbrAuto, "CONN_TRX_AUTO uses separate connection" ); + + $dbwAuto2 = $lb->getConnection( DB_MASTER, [], false, $lb::CONN_TRX_AUTO ); + $this->assertEquals( $dbwAuto2, $dbwAuto, "CONN_TRX_AUTO reuses connections" ); + + $lb->closeAll(); + } +}