From e8fa89bd30832a636be5b1d24532cf3b8cb685a9 Mon Sep 17 00:00:00 2001 From: Aaron Schulz Date: Fri, 19 Dec 2014 15:27:59 -0800 Subject: [PATCH] Moved "nowait:" key code to PoolCounter in core Change-Id: I5286e6c6052289e1107314a04d72703b44a8fbc6 --- includes/poolcounter/PoolCounter.php | 57 ++++++++++++++++++++++- includes/poolcounter/PoolCounterRedis.php | 14 ++++++ languages/i18n/en.json | 1 + languages/i18n/qqq.json | 1 + 4 files changed, 72 insertions(+), 1 deletion(-) diff --git a/includes/poolcounter/PoolCounter.php b/includes/poolcounter/PoolCounter.php index e77ffd7cd6..5692d73199 100644 --- a/includes/poolcounter/PoolCounter.php +++ b/includes/poolcounter/PoolCounter.php @@ -34,7 +34,10 @@ * minutes and hundreds of read hits. * * The PoolCounter provides semaphore semantics for restricting the number - * of workers that may be concurrently performing such single task. + * of workers that may be concurrently performing such single task. Only one + * key can be locked by any PoolCounter instance of a process, except for keys + * that start with "nowait:". However, only 0 timeouts (non-blocking requests) + * can be used with "nowait:" keys. * * By default PoolCounter_Stub is used, which provides no locking. You * can get a useful one in the PoolCounter extension. @@ -67,6 +70,15 @@ abstract class PoolCounter { /** @var float Maximum time in seconds to wait for the lock */ protected $timeout; + /** + * @var boolean Whether the key is a "might wait" key + */ + private $isMightWaitKey; + /** + * @var boolean Whether this process holds a "might wait" lock key + */ + private static $acquiredMightWaitKey = 0; + /** * @param array $conf * @param string $type @@ -84,6 +96,7 @@ abstract class PoolCounter { $key = $this->hashKeyIntoSlots( $key, $this->slots ); } $this->key = $key; + $this->isMightWaitKey = !preg_match( '/^nowait:/', $this->key ); } /** @@ -136,6 +149,48 @@ abstract class PoolCounter { */ abstract public function release(); + /** + * Checks that the lock request is sane. + * @return Status - good for sane requests fatal for insane + * @since 1.25 + */ + final protected function precheckAcquire() { + if ( $this->isMightWaitKey ) { + if ( self::$acquiredMightWaitKey ) { + /* + * The poolcounter itself is quite happy to allow you to wait + * on another lock while you have a lock you waited on already + * but we think that it is unlikely to be a good idea. So we + * made it an error. If you are _really_ _really_ sure it is a + * good idea then feel free to implement an unsafe flag or + * something. + */ + return Status::newFatal( 'poolcounter-usage-error', + 'You may only aquire a single non-nowait lock.' ); + } + } elseif ( $this->timeout !== 0 ) { + return Status::newFatal( 'poolcounter-usage-error', + 'Locks starting in nowait: must have 0 timeout.' ); + } + return Status::newGood(); + } + + /** + * Update any lock tracking information when the lock is acquired + * @since 1.25 + */ + final protected function onAcquire() { + self::$acquiredMightWaitKey |= $this->isMightWaitKey; + } + + /** + * Update any lock tracking information when the lock is released + * @since 1.25 + */ + final protected function onRelease() { + self::$acquiredMightWaitKey &= !$this->isMightWaitKey; + } + /** * Given a key (any string) and the number of lots, returns a slot number (an integer from the [0..($slots-1)] range). * This is used for a global limit on the number of instances of a given type that can acquire a lock. diff --git a/includes/poolcounter/PoolCounterRedis.php b/includes/poolcounter/PoolCounterRedis.php index d609f61427..0f025f369b 100644 --- a/includes/poolcounter/PoolCounterRedis.php +++ b/includes/poolcounter/PoolCounterRedis.php @@ -123,12 +123,22 @@ class PoolCounterRedis extends PoolCounter { function acquireForMe() { $section = new ProfileSection( __METHOD__ ); + $status = $this->precheckAcquire(); + if ( !$status->isGood() ) { + return $status; + } + return $this->waitForSlotOrNotif( self::AWAKE_ONE ); } function acquireForAnyone() { $section = new ProfileSection( __METHOD__ ); + $status = $this->precheckAcquire(); + if ( !$status->isGood() ) { + return $status; + } + return $this->waitForSlotOrNotif( self::AWAKE_ALL ); } @@ -207,6 +217,8 @@ LUA; $this->onRelease = null; unset( self::$active[$this->session] ); + $this->onRelease(); + return Status::newGood( PoolCounter::RELEASED ); } @@ -266,6 +278,8 @@ LUA; self::$active[$this->session] = $this; } + $this->onAcquire(); + return Status::newGood( $slot === 'w' ? PoolCounter::DONE : PoolCounter::LOCKED ); } diff --git a/languages/i18n/en.json b/languages/i18n/en.json index 8fc9cf2e17..b0751e38bc 100644 --- a/languages/i18n/en.json +++ b/languages/i18n/en.json @@ -222,6 +222,7 @@ "pool-queuefull": "Pool queue is full", "pool-errorunknown": "Unknown error", "pool-servererror": "The pool counter service is not available ($1).", + "poolcounter-usage-error": "Usage error: $1", "aboutsite": "About {{SITENAME}}", "aboutpage": "Project:About", "copyright": "Content is available under $1 unless otherwise noted.", diff --git a/languages/i18n/qqq.json b/languages/i18n/qqq.json index 2c01b0241c..04a6f9dd14 100644 --- a/languages/i18n/qqq.json +++ b/languages/i18n/qqq.json @@ -386,6 +386,7 @@ "pool-queuefull": "Part of {{msg-mw|view-pool-error}}\n\n\"Pool\" refers to a pool of processes.", "pool-errorunknown": "Part of {{msg-mw|view-pool-error}}.\n{{Identical|Unknown error}}", "pool-servererror": "Error message. Parameters:\n* $1 - list of server addresses\n\nSee e.g. {{msg-mw|Poolcounter-desc}} (and the Pool Counter extension in general) for translation hints for “pool counter service”.", + "poolcounter-usage-error": "Used as error message. Parameters:\n* $1 - non-localized string describing usage mistake.", "aboutsite": "Used as the label of the link that appears at the footer of every page on the wiki (in most of the skins) and leads to the page that contains the site description. The link target is {{msg-mw|aboutpage}}.\n\n[[mw:Manual:Interface/Aboutsite|MediaWiki manual]].\n\n{{doc-important|Do not change {{SITENAME}}.}}\n\n{{Identical|About}}", "aboutpage": "Used as the target of the link that appears at the footer of every page on the wiki (in most of the skins) and leads to the page that contains the site description. Therefore the content should be the same with the page name of the site description page. Only the message in the [[mw:Manual:$wgLanguageCode|site language]] ([[MediaWiki:Aboutpage]]) is used. The link label is {{msg-mw|aboutsite}}.\n\n{{doc-important|Do not translate \"Project:\" part, for this is the namespace prefix.}}", "copyright": "Parameters:\n* $1 - license name\n'''See also'''\n* {{msg-mw|Mobile-frontend-copyright}}", -- 2.20.1