From 0cf832a339422a3beb088b88e43f69322f2cbad0 Mon Sep 17 00:00:00 2001 From: Aaron Schulz Date: Sat, 17 Sep 2016 23:35:05 -0700 Subject: [PATCH] Inject "srvCache" and local DB connections into LockManagerDB * Also simplified the srvCache variable usage to be unconditional. * The wfRandomString() call has also been replaced. Change-Id: I17e83b17ec549906ee200bbe9eb2f0b151423e26 --- includes/DefaultSettings.php | 4 + .../filebackend/lockmanager/DBLockManager.php | 81 ++++++++----------- .../lockmanager/LockManagerGroup.php | 11 ++- .../lockmanager/MySqlLockManager.php | 5 -- 4 files changed, 49 insertions(+), 52 deletions(-) diff --git a/includes/DefaultSettings.php b/includes/DefaultSettings.php index 135c3e5231..be858c22c1 100644 --- a/includes/DefaultSettings.php +++ b/includes/DefaultSettings.php @@ -644,6 +644,10 @@ $wgFileBackends = []; * See LockManager::__construct() for more details. * Additional parameters are specific to the lock manager class used. * These settings should be global to all wikis. + * + * When using DBLockManager, the 'dbsByBucket' map can reference 'localDBMaster' as + * a peer database in each bucket. This will result in an extra connection to the domain + * that the LockManager services, which must also be a valid wiki ID. */ $wgLockManagers = []; diff --git a/includes/filebackend/lockmanager/DBLockManager.php b/includes/filebackend/lockmanager/DBLockManager.php index 4667dde450..c36ff483f8 100644 --- a/includes/filebackend/lockmanager/DBLockManager.php +++ b/includes/filebackend/lockmanager/DBLockManager.php @@ -36,7 +36,7 @@ * @since 1.19 */ abstract class DBLockManager extends QuorumLockManager { - /** @var array[] Map of DB names to server config */ + /** @var array[]|IDatabase[] Map of (DB names => server config or IDatabase) */ protected $dbServers; // (DB name => server config array) /** @var BagOStuff */ protected $statusCache; @@ -63,19 +63,15 @@ abstract class DBLockManager extends QuorumLockManager { * - flags : DB flags (see DatabaseBase) * - dbsByBucket : Array of 1-16 consecutive integer keys, starting from 0, * each having an odd-numbered list of DB names (peers) as values. - * Any DB named 'localDBMaster' will automatically use the DB master - * settings for this wiki (without the need for a dbServers entry). - * Only use 'localDBMaster' if the domain is a valid wiki ID. * - lockExpiry : Lock timeout (seconds) for dropped connections. [optional] * This tells the DB server how long to wait before assuming * connection failure and releasing all the locks for a session. + * - srvCache : A BagOStuff instance using APC or the like. */ public function __construct( array $config ) { parent::__construct( $config ); - $this->dbServers = isset( $config['dbServers'] ) - ? $config['dbServers'] - : []; // likely just using 'localDBMaster' + $this->dbServers = $config['dbServers']; // Sanitize srvsByBucket config to prevent PHP errors $this->srvsByBucket = array_filter( $config['dbsByBucket'], 'is_array' ); $this->srvsByBucket = array_values( $this->srvsByBucket ); // consecutive @@ -90,19 +86,25 @@ abstract class DBLockManager extends QuorumLockManager { ? 60 // pick a safe-ish number to match DB timeout default : $this->lockExpiry; // cover worst case - foreach ( $this->srvsByBucket as $bucket ) { - if ( count( $bucket ) > 1 ) { // multiple peers - // Tracks peers that couldn't be queried recently to avoid lengthy - // connection timeouts. This is useless if each bucket has one peer. - $this->statusCache = ObjectCache::getLocalServerInstance(); - break; - } - } + // Tracks peers that couldn't be queried recently to avoid lengthy + // connection timeouts. This is useless if each bucket has one peer. + $this->statusCache = isset( $config['srvCache'] ) + ? $config['srvCache'] + : new HashBagOStuff(); - $this->session = wfRandomString( 31 ); + $random = []; + for ( $i = 1; $i <= 5; ++$i ) { + $random[] = mt_rand( 0, 0xFFFFFFF ); + } + $this->session = substr( md5( implode( '-', $random ) ), 0, 31 ); } - // @todo change this code to work in one batch + /** + * @TODO change this code to work in one batch + * @param string $lockSrv + * @param array $pathsByType + * @return StatusValue + */ protected function getLocksOnServer( $lockSrv, array $pathsByType ) { $status = StatusValue::newGood(); foreach ( $pathsByType as $type => $paths ) { @@ -148,32 +150,27 @@ abstract class DBLockManager extends QuorumLockManager { */ protected function getConnection( $lockDb ) { if ( !isset( $this->conns[$lockDb] ) ) { - if ( $lockDb === 'localDBMaster' ) { - $lb = $this->getLocalLB(); - $db = $lb->getConnection( DB_MASTER, [], $this->domain ); - # Do not mess with settings if the LoadBalancer is the main singleton - # to avoid clobbering the settings of handles from wfGetDB( DB_MASTER ). - $init = ( wfGetLB() !== $lb ); - } elseif ( isset( $this->dbServers[$lockDb] ) ) { + if ( $this->dbServers[$lockDb] instanceof IDatabase ) { + // Direct injected connection hande for $lockDB + $db = $this->dbServers[$lockDb]; + } elseif ( is_array( $this->dbServers[$lockDb] ) ) { + // Parameters to construct a new database connection $config = $this->dbServers[$lockDb]; $db = DatabaseBase::factory( $config['type'], $config ); - $init = true; } else { throw new UnexpectedValueException( "No server called '$lockDb'." ); } - if ( $init ) { - $db->clearFlag( DBO_TRX ); - # If the connection drops, try to avoid letting the DB rollback - # and release the locks before the file operations are finished. - # This won't handle the case of DB server restarts however. - $options = []; - if ( $this->lockExpiry > 0 ) { - $options['connTimeout'] = $this->lockExpiry; - } - $db->setSessionOptions( $options ); - $this->initConnection( $lockDb, $db ); + $db->clearFlag( DBO_TRX ); + # If the connection drops, try to avoid letting the DB rollback + # and release the locks before the file operations are finished. + # This won't handle the case of DB server restarts however. + $options = []; + if ( $this->lockExpiry > 0 ) { + $options['connTimeout'] = $this->lockExpiry; } + $db->setSessionOptions( $options ); + $this->initConnection( $lockDb, $db ); $this->conns[$lockDb] = $db; } @@ -181,13 +178,6 @@ abstract class DBLockManager extends QuorumLockManager { return $this->conns[$lockDb]; } - /** - * @return LoadBalancer - */ - protected function getLocalLB() { - return wfGetLBFactory()->getMainLB( $this->domain ); - } - /** * Do additional initialization for new lock DB connection * @@ -206,7 +196,7 @@ abstract class DBLockManager extends QuorumLockManager { * @return bool */ protected function cacheCheckFailures( $lockDb ) { - return ( $this->statusCache && $this->safeDelay > 0 ) + return ( $this->safeDelay > 0 ) ? !$this->statusCache->get( $this->getMissKey( $lockDb ) ) : true; } @@ -218,7 +208,7 @@ abstract class DBLockManager extends QuorumLockManager { * @return bool Success */ protected function cacheRecordFailure( $lockDb ) { - return ( $this->statusCache && $this->safeDelay > 0 ) + return ( $this->safeDelay > 0 ) ? $this->statusCache->set( $this->getMissKey( $lockDb ), 1, $this->safeDelay ) : true; } @@ -230,7 +220,6 @@ abstract class DBLockManager extends QuorumLockManager { * @return string */ protected function getMissKey( $lockDb ) { - $lockDb = ( $lockDb === 'localDBMaster' ) ? wfWikiID() : $lockDb; // non-relative return 'dblockmanager:downservers:' . str_replace( ' ', '_', $lockDb ); } diff --git a/includes/filebackend/lockmanager/LockManagerGroup.php b/includes/filebackend/lockmanager/LockManagerGroup.php index 602b876b8a..9ad2faf251 100644 --- a/includes/filebackend/lockmanager/LockManagerGroup.php +++ b/includes/filebackend/lockmanager/LockManagerGroup.php @@ -20,6 +20,7 @@ * @file * @ingroup LockManager */ +use MediaWiki\MediaWikiServices; /** * Class to handle file lock manager registration @@ -29,7 +30,7 @@ * @since 1.19 */ class LockManagerGroup { - /** @var array (domain => LockManager) */ + /** @var LockManagerGroup[] (domain => LockManagerGroup) */ protected static $instances = []; protected $domain; // string; domain (usually wiki ID) @@ -115,6 +116,14 @@ class LockManagerGroup { if ( !isset( $this->managers[$name]['instance'] ) ) { $class = $this->managers[$name]['class']; $config = $this->managers[$name]['config']; + if ( $class === 'DBLockManager' ) { + $lbFactory = MediaWikiServices::getInstance()->getDBLoadBalancerFactory(); + $lb = $lbFactory->newMainLB( $config['domain'] ); + $dbw = $lb->getLazyConnectionRef( DB_MASTER, [], $config['domain'] ); + + $config['dbServers']['localDBMaster'] = $dbw; + $config['srvCache'] = ObjectCache::getLocalServerInstance( 'hash' ); + } $this->managers[$name]['instance'] = new $class( $config ); } diff --git a/includes/filebackend/lockmanager/MySqlLockManager.php b/includes/filebackend/lockmanager/MySqlLockManager.php index 124d41038a..441ffea2d5 100644 --- a/includes/filebackend/lockmanager/MySqlLockManager.php +++ b/includes/filebackend/lockmanager/MySqlLockManager.php @@ -15,11 +15,6 @@ class MySqlLockManager extends DBLockManager { self::LOCK_EX => self::LOCK_EX ]; - protected function getLocalLB() { - // Use a separate connection so releaseAllLocks() doesn't rollback the main trx - return wfGetLBFactory()->newMainLB( $this->domain ); - } - protected function initConnection( $lockDb, IDatabase $db ) { # Let this transaction see lock rows from other transactions $db->query( "SET SESSION TRANSACTION ISOLATION LEVEL READ UNCOMMITTED;" ); -- 2.20.1