jenkins-bot has submitted this change and it was merged.
Change subject: Cleaned up PoolCounter docs, moved up some functions, and
declared public functions.
......................................................................
Cleaned up PoolCounter docs, moved up some functions, and declared public
functions.
Change-Id: I9b3a320a3ea12c3d7aea09236cd0ecbf37a19860
---
M includes/PoolCounter.php
1 file changed, 77 insertions(+), 74 deletions(-)
Approvals:
Demon: Looks good to me, approved
jenkins-bot: Verified
diff --git a/includes/PoolCounter.php b/includes/PoolCounter.php
index 2ebef04..c6a3077 100644
--- a/includes/PoolCounter.php
+++ b/includes/PoolCounter.php
@@ -22,25 +22,24 @@
*/
/**
- * When you have many workers (threads/servers) giving service, and a
+ * When you have many workers (threads/servers) giving service, and a
* cached item expensive to produce expires, you may get several workers
* doing the job at the same time.
*
- * Given enough requests and the item expiring fast (non-cacheable,
+ * Given enough requests and the item expiring fast (non-cacheable,
* lots of edits...) that single work can end up unfairly using most (all)
* of the cpu of the pool. This is also known as 'Michael Jackson effect'
* since this effect triggered on the english wikipedia on the day Michael
* Jackson died, the biographical article got hit with several edits per
* minutes and hundreds of read hits.
*
- * The PoolCounter provides semaphore semantics for restricting the number
+ * The PoolCounter provides semaphore semantics for restricting the number
* of workers that may be concurrently performing such single task.
*
- * By default PoolCounter_Stub is used, which provides no locking. You
+ * By default PoolCounter_Stub is used, which provides no locking. You
* can get a useful one in the PoolCounter extension.
*/
abstract class PoolCounter {
-
/* Return codes */
const LOCKED = 1; /* Lock acquired */
const RELEASED = 2; /* Lock released */
@@ -52,39 +51,26 @@
const TIMEOUT = -4; /* Timeout exceeded */
const LOCK_HELD = -5; /* Cannot acquire another lock while you have one
lock held */
- /**
- * I want to do this task and I need to do it myself.
- *
- * @return Locked/Error
- */
- abstract function acquireForMe();
+ /** @var string All workers with the same key share the lock */
+ protected $key;
+ /** @var integer Maximum number of workers doing the task
simultaneously */
+ protected $workers;
+ /** @var integer If this number of workers are already working/waiting,
fail instead of wait */
+ protected $maxqueue;
+ /** @var float Maximum time in seconds to wait for the lock */
+ protected $timeout;
/**
- * I want to do this task, but if anyone else does it
- * instead, it's also fine for me. I will read its cached data.
- *
- * @return Locked/Done/Error
+ * @param array $conf
+ * @param string $type
+ * @param string $key
*/
- abstract function acquireForAnyone();
-
- /**
- * I have successfully finished my task.
- * Lets another one grab the lock, and returns the workers
- * waiting on acquireForAnyone()
- *
- * @return Released/NotLocked/Error
- */
- abstract function release();
-
- /**
- * $key: All workers with the same key share the lock.
- * $workers: It wouldn't be a good idea to have more than this number
of
- * workers doing the task simultaneously.
- * $maxqueue: If this number of workers are already working/waiting,
- * fail instead of wait.
- * $timeout: Maximum time in seconds to wait for the lock.
- */
- protected $key, $workers, $maxqueue, $timeout;
+ protected function __construct( $conf, $type, $key ) {
+ $this->key = $key;
+ $this->workers = $conf['workers'];
+ $this->maxqueue = $conf['maxqueue'];
+ $this->timeout = $conf['timeout'];
+ }
/**
* Create a Pool counter. This should only be called from the PoolWorks.
@@ -105,39 +91,46 @@
return new $class( $conf, $type, $key );
}
- protected function __construct( $conf, $type, $key ) {
- $this->key = $key;
- $this->workers = $conf['workers'];
- $this->maxqueue = $conf['maxqueue'];
- $this->timeout = $conf['timeout'];
- }
+ /**
+ * I want to do this task and I need to do it myself.
+ *
+ * @return Status Value is one of Locked/Error
+ */
+ abstract public function acquireForMe();
+
+ /**
+ * I want to do this task, but if anyone else does it
+ * instead, it's also fine for me. I will read its cached data.
+ *
+ * @return Status Value is one of Locked/Done/Error
+ */
+ abstract public function acquireForAnyone();
+
+ /**
+ * I have successfully finished my task.
+ * Lets another one grab the lock, and returns the workers
+ * waiting on acquireForAnyone()
+ *
+ * @return Status value is one of Released/NotLocked/Error
+ */
+ abstract public function release();
}
class PoolCounter_Stub extends PoolCounter {
-
- /**
- * @return Status
- */
- function acquireForMe() {
- return Status::newGood( PoolCounter::LOCKED );
- }
-
- /**
- * @return Status
- */
- function acquireForAnyone() {
- return Status::newGood( PoolCounter::LOCKED );
- }
-
- /**
- * @return Status
- */
- function release() {
- return Status::newGood( PoolCounter::RELEASED );
- }
-
public function __construct() {
/* No parameters needed */
+ }
+
+ public function acquireForMe() {
+ return Status::newGood( PoolCounter::LOCKED );
+ }
+
+ public function acquireForAnyone() {
+ return Status::newGood( PoolCounter::LOCKED );
+ }
+
+ public function release() {
+ return Status::newGood( PoolCounter::RELEASED );
}
}
@@ -148,15 +141,24 @@
protected $cacheable = false; //Does this override getCachedWork() ?
/**
- * Actually perform the work, caching it if needed.
+ * @param string $type The type of PoolCounter to use
+ * @param string $key Key that identifies the queue this work is placed
on
*/
- abstract function doWork();
+ public function __construct( $type, $key ) {
+ $this->poolCounter = PoolCounter::factory( $type, $key );
+ }
+
+ /**
+ * Actually perform the work, caching it if needed
+ * @return mixed work result or false
+ */
+ abstract public function doWork();
/**
* Retrieve the work from cache
* @return mixed work result or false
*/
- function getCachedWork() {
+ public function getCachedWork() {
return false;
}
@@ -165,7 +167,7 @@
* message.
* @return mixed work result or false
*/
- function fallback() {
+ public function fallback() {
return false;
}
@@ -181,6 +183,7 @@
* Log an error
*
* @param $status Status
+ * @return void
*/
function logError( $status ) {
wfDebugLog( 'poolcounter', $status->getWikiText() );
@@ -191,7 +194,7 @@
* @param $skipcache bool
* @return bool|mixed
*/
- function execute( $skipcache = false ) {
+ public function execute( $skipcache = false ) {
if ( $this->cacheable && !$skipcache ) {
$status = $this->poolCounter->acquireForAnyone();
} else {
@@ -232,15 +235,15 @@
/* These two cases should never be hit... */
case PoolCounter::ERROR:
default:
- $errors = array( PoolCounter::QUEUE_FULL =>
'pool-queuefull', PoolCounter::TIMEOUT => 'pool-timeout' );
+ $errors = array(
+ PoolCounter::QUEUE_FULL =>
'pool-queuefull',
+ PoolCounter::TIMEOUT => 'pool-timeout'
);
- $status = Status::newFatal( isset(
$errors[$status->value] ) ? $errors[$status->value] : 'pool-errorunknown' );
+ $status = Status::newFatal( isset(
$errors[$status->value] )
+ ? $errors[$status->value]
+ : 'pool-errorunknown' );
$this->logError( $status );
return $this->error( $status );
}
- }
-
- function __construct( $type, $key ) {
- $this->poolCounter = PoolCounter::factory( $type, $key );
}
}
--
To view, visit https://gerrit.wikimedia.org/r/54600
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I9b3a320a3ea12c3d7aea09236cd0ecbf37a19860
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/core
Gerrit-Branch: master
Gerrit-Owner: Aaron Schulz <[email protected]>
Gerrit-Reviewer: Demon <[email protected]>
Gerrit-Reviewer: Platonides <[email protected]>
Gerrit-Reviewer: Tim Starling <[email protected]>
Gerrit-Reviewer: jenkins-bot
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
