From 5b67e492ffd309cbcb5c972791a5e630aa51be02 Mon Sep 17 00:00:00 2001 From: Aaron Schulz Date: Fri, 25 Jan 2013 12:19:57 -0800 Subject: [PATCH] [LockManager] Refactoring to allow proper cross-wiki support. Change-Id: I331d011a6fa4edd434f0fb547d325ad60901119d --- includes/DefaultSettings.php | 1 + includes/ForkController.php | 2 +- .../filebackend/lockmanager/DBLockManager.php | 4 +- .../filebackend/lockmanager/FSLockManager.php | 3 +- .../filebackend/lockmanager/LSLockManager.php | 4 +- .../filebackend/lockmanager/LockManager.php | 17 +++++++-- .../lockmanager/LockManagerGroup.php | 37 ++++++++++++------- .../lockmanager/MemcLockManager.php | 8 +--- tests/parser/parserTest.inc | 2 +- 9 files changed, 45 insertions(+), 33 deletions(-) diff --git a/includes/DefaultSettings.php b/includes/DefaultSettings.php index 6a4f6a60a3..2808996636 100644 --- a/includes/DefaultSettings.php +++ b/includes/DefaultSettings.php @@ -455,6 +455,7 @@ $wgFileBackends = array(); * - 'name' : A unique name for the lock manager * - 'class' : The lock manger class to use * Additional parameters are specific to the class used. + * These settings should be global to all wikis. */ $wgLockManagers = array(); diff --git a/includes/ForkController.php b/includes/ForkController.php index 448bc03be7..2a05411ef2 100644 --- a/includes/ForkController.php +++ b/includes/ForkController.php @@ -140,7 +140,7 @@ class ForkController { // Don't share DB, storage, or memcached connections wfGetLBFactory()->destroyInstance(); FileBackendGroup::destroySingleton(); - LockManagerGroup::destroySingleton(); + LockManagerGroup::destroySingletons(); ObjectCache::clear(); $wgMemc = null; } diff --git a/includes/filebackend/lockmanager/DBLockManager.php b/includes/filebackend/lockmanager/DBLockManager.php index a8fe258b27..196a32ac71 100644 --- a/includes/filebackend/lockmanager/DBLockManager.php +++ b/includes/filebackend/lockmanager/DBLockManager.php @@ -122,7 +122,7 @@ class DBLockManager extends QuorumLockManager { if ( $type == self::LOCK_EX ) { // writer locks try { - $keys = array_unique( array_map( 'LockManager::sha1Base36', $paths ) ); + $keys = array_unique( array_map( array( $this, 'sha1Base36Absolute' ), $paths ) ); # Build up values for INSERT clause $data = array(); foreach ( $keys as $key ) { @@ -321,7 +321,7 @@ class MySqlLockManager extends DBLockManager { $status = Status::newGood(); $db = $this->getConnection( $lockSrv ); // checked in isServerUp() - $keys = array_unique( array_map( 'LockManager::sha1Base36', $paths ) ); + $keys = array_unique( array_map( array( $this, 'sha1Base36Absolute' ), $paths ) ); # Build up values for INSERT clause $data = array(); foreach ( $keys as $key ) { diff --git a/includes/filebackend/lockmanager/FSLockManager.php b/includes/filebackend/lockmanager/FSLockManager.php index 9a6206fd5d..f374fdd193 100644 --- a/includes/filebackend/lockmanager/FSLockManager.php +++ b/includes/filebackend/lockmanager/FSLockManager.php @@ -237,8 +237,7 @@ class FSLockManager extends LockManager { * @return string */ protected function getLockPath( $path ) { - $hash = self::sha1Base36( $path ); - return "{$this->lockDir}/{$hash}.lock"; + return "{$this->lockDir}/{$this->sha1Base36Absolute( $path )}.lock"; } /** diff --git a/includes/filebackend/lockmanager/LSLockManager.php b/includes/filebackend/lockmanager/LSLockManager.php index 3de6183768..96d37abaac 100644 --- a/includes/filebackend/lockmanager/LSLockManager.php +++ b/includes/filebackend/lockmanager/LSLockManager.php @@ -94,7 +94,7 @@ class LSLockManager extends QuorumLockManager { // Send out the command and get the response... $type = ( $type == self::LOCK_SH ) ? 'SH' : 'EX'; - $keys = array_unique( array_map( 'LockManager::sha1Base36', $paths ) ); + $keys = array_unique( array_map( array( $this, 'sha1Base36Absolute' ), $paths ) ); $response = $this->sendCommand( $lockSrv, 'ACQUIRE', $type, $keys ); if ( $response !== 'ACQUIRED' ) { @@ -115,7 +115,7 @@ class LSLockManager extends QuorumLockManager { // Send out the command and get the response... $type = ( $type == self::LOCK_SH ) ? 'SH' : 'EX'; - $keys = array_unique( array_map( 'LockManager::sha1Base36', $paths ) ); + $keys = array_unique( array_map( array( $this, 'sha1Base36Absolute' ), $paths ) ); $response = $this->sendCommand( $lockSrv, 'RELEASE', $type, $keys ); if ( $response !== 'RELEASED' ) { diff --git a/includes/filebackend/lockmanager/LockManager.php b/includes/filebackend/lockmanager/LockManager.php index 51454a4f85..fd6f0f853a 100644 --- a/includes/filebackend/lockmanager/LockManager.php +++ b/includes/filebackend/lockmanager/LockManager.php @@ -53,6 +53,8 @@ abstract class LockManager { /** @var Array Map of (resource path => lock type => count) */ protected $locksHeld = array(); + protected $wiki; // string; wiki ID + /* Lock types; stronger locks have higher values */ const LOCK_SH = 1; // shared lock (for reads) const LOCK_UW = 2; // shared lock (for reads used to write elsewhere) @@ -61,9 +63,14 @@ abstract class LockManager { /** * Construct a new instance from configuration * + * $config paramaters include: + * - wiki : Wiki ID string that all resources are relative to [optional] + * * @param $config Array */ - public function __construct( array $config ) {} + public function __construct( array $config ) { + $this->wiki = isset( $config['wiki'] ) ? $config['wiki'] : wfWikiID(); + } /** * Lock the resources at the given abstract paths @@ -94,13 +101,15 @@ abstract class LockManager { } /** - * Get the base 36 SHA-1 of a string, padded to 31 digits + * Get the base 36 SHA-1 of a string, padded to 31 digits. + * Before hashing, the path will be prefixed with the wiki ID. + * This should be used interally for lock key or file names. * * @param $path string * @return string */ - final protected static function sha1Base36( $path ) { - return wfBaseConvert( sha1( $path ), 16, 36, 31 ); + final protected function sha1Base36Absolute( $path ) { + return wfBaseConvert( sha1( "{$this->wiki}:{$path}" ), 16, 36, 31 ); } /** diff --git a/includes/filebackend/lockmanager/LockManagerGroup.php b/includes/filebackend/lockmanager/LockManagerGroup.php index 8c8c940a0a..267d391e40 100644 --- a/includes/filebackend/lockmanager/LockManagerGroup.php +++ b/includes/filebackend/lockmanager/LockManagerGroup.php @@ -29,33 +29,41 @@ * @since 1.19 */ class LockManagerGroup { - /** - * @var LockManagerGroup - */ - protected static $instance = null; + /** @var Array (wiki => LockManager) */ + protected static $instances = array(); + + protected $wiki; // string; wiki ID /** @var Array of (name => ('class' =>, 'config' =>, 'instance' =>)) */ protected $managers = array(); - protected function __construct() {} + /** + * @param $wiki string Wiki ID + */ + protected function __construct( $wiki ) { + $this->wiki = $wiki; + } /** + * @param $wiki string Wiki ID * @return LockManagerGroup */ - public static function singleton() { - if ( self::$instance == null ) { - self::$instance = new self(); - self::$instance->initFromGlobals(); + public static function singleton( $wiki = false ) { + $wiki = ( $wiki === false ) ? wfWikiID() : $wiki; + if ( !isset( self::$instances[$wiki] ) ) { + self::$instances[$wiki] = new self( $wiki ); + self::$instances[$wiki]->initFromGlobals(); } - return self::$instance; + return self::$instances[$wiki]; } /** - * Destroy the singleton instance, so that a new one will be created next - * time singleton() is called. + * Destroy the singleton instances + * + * @return void */ - public static function destroySingleton() { - self::$instance = null; + public static function destroySingletons() { + self::$instances = array(); } /** @@ -78,6 +86,7 @@ class LockManagerGroup { */ protected function register( array $configs ) { foreach ( $configs as $config ) { + $config['wiki'] = $this->wiki; if ( !isset( $config['name'] ) ) { throw new MWException( "Cannot register a lock manager with no name." ); } diff --git a/includes/filebackend/lockmanager/MemcLockManager.php b/includes/filebackend/lockmanager/MemcLockManager.php index 610fc4773a..099f11da6b 100644 --- a/includes/filebackend/lockmanager/MemcLockManager.php +++ b/includes/filebackend/lockmanager/MemcLockManager.php @@ -50,7 +50,6 @@ class MemcLockManager extends QuorumLockManager { protected $lockExpiry; // integer; maximum time locks can be held protected $session = ''; // string; random SHA-1 UUID - protected $wikiId = ''; // string /** * Construct a new instance from configuration. @@ -61,7 +60,6 @@ class MemcLockManager extends QuorumLockManager { * each having an odd-numbered list of server names (peers) as values. * - memcConfig : Configuration array for ObjectCache::newFromParams. [optional] * If set, this must use one of the memcached classes. - * - wikiId : Wiki ID string that all resources are relative to. [optional] * * @param Array $config * @throws MWException @@ -88,8 +86,6 @@ class MemcLockManager extends QuorumLockManager { } } - $this->wikiId = isset( $config['wikiId'] ) ? $config['wikiId'] : wfWikiID(); - $met = ini_get( 'max_execution_time' ); // this is 0 in CLI mode $this->lockExpiry = $met ? 2*(int)$met : 2*3600; @@ -252,9 +248,7 @@ class MemcLockManager extends QuorumLockManager { * @return string */ protected function recordKeyForPath( $path ) { - $hash = LockManager::sha1Base36( $path ); - list( $db, $prefix ) = wfSplitWikiID( $this->wikiId ); - return wfForeignMemcKey( $db, $prefix, __CLASS__, 'locks', $hash ); + return implode( ':', array( __CLASS__, 'locks', $this->sha1Base36Absolute( $path ) ) ); } /** diff --git a/tests/parser/parserTest.inc b/tests/parser/parserTest.inc index ea1b29016d..db3c84acf8 100644 --- a/tests/parser/parserTest.inc +++ b/tests/parser/parserTest.inc @@ -966,7 +966,7 @@ class ParserTest { private function teardownGlobals() { RepoGroup::destroySingleton(); FileBackendGroup::destroySingleton(); - LockManagerGroup::destroySingleton(); + LockManagerGroup::destroySingletons(); LinkCache::singleton()->clear(); foreach ( $this->savedGlobals as $var => $val ) { -- 2.20.1