r94777 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r94776‎ | r94777 | r94778 >
Date:17:11, 17 August 2011
Author:reedy
Status:ok
Tags:
Comment:
Documentation

Trim trailing whitespace
Modified paths:
  • /trunk/extensions/PoolCounter/PoolCounterClient_body.php (modified) (history)
  • /trunk/phase3/includes/PoolCounter.php (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/PoolCounter.php
@@ -1,11 +1,11 @@
22 <?php
33
44 /**
5 - * When you have many workers (threads/servers) giving service, and a
 5+ * When you have many workers (threads/servers) giving service, and a
66 * cached item expensive to produce expires, you may get several workers
77 * doing the job at the same time.
88 *
9 - * Given enough requests and the item expiring fast (non-cacheable,
 9+ * Given enough requests and the item expiring fast (non-cacheable,
1010 * lots of edits...) that single work can end up unfairly using most (all)
1111 * of the cpu of the pool. This is also known as 'Michael Jackson effect'
1212 * since this effect triggered on the english wikipedia on the day Michael
@@ -15,16 +15,16 @@
1616 * The PoolCounter provides semaphore semantics for restricting the number
1717 * of workers that may be concurrently performing such single task.
1818 *
19 - * By default PoolCounter_Stub is used, which provides no locking. You
 19+ * By default PoolCounter_Stub is used, which provides no locking. You
2020 * can get a useful one in the PoolCounter extension.
2121 */
2222 abstract class PoolCounter {
23 -
 23+
2424 /* Return codes */
2525 const LOCKED = 1; /* Lock acquired */
2626 const RELEASED = 2; /* Lock released */
2727 const DONE = 3; /* Another worker did the work for you */
28 -
 28+
2929 const ERROR = -1; /* Indeterminate error */
3030 const NOT_LOCKED = -2; /* Called release() with no lock held */
3131 const QUEUE_FULL = -3; /* There are already maxqueue workers on this lock */
@@ -33,38 +33,38 @@
3434
3535 /**
3636 * I want to do this task and I need to do it myself.
37 - *
 37+ *
3838 * @return Locked/Error
3939 */
4040 abstract function acquireForMe();
4141
4242 /**
43 - * I want to do this task, but if anyone else does it
 43+ * I want to do this task, but if anyone else does it
4444 * instead, it's also fine for me. I will read its cached data.
45 - *
 45+ *
4646 * @return Locked/Done/Error
4747 */
4848 abstract function acquireForAnyone();
4949
5050 /**
5151 * I have successfully finished my task.
52 - * Lets another one grab the lock, and returns the workers
 52+ * Lets another one grab the lock, and returns the workers
5353 * waiting on acquireForAnyone()
54 - *
 54+ *
5555 * @return Released/NotLocked/Error
5656 */
5757 abstract function release();
58 -
 58+
5959 /**
6060 * $key: All workers with the same key share the lock.
61 - * $workers: It wouldn't be a good idea to have more than this number of
 61+ * $workers: It wouldn't be a good idea to have more than this number of
6262 * workers doing the task simultaneously.
63 - * $maxqueue: If this number of workers are already working/waiting,
 63+ * $maxqueue: If this number of workers are already working/waiting,
6464 * fail instead of wait.
6565 * $timeout: Maximum time in seconds to wait for the lock.
6666 */
6767 protected $key, $workers, $maxqueue, $timeout;
68 -
 68+
6969 /**
7070 * Create a Pool counter. This should only be called from the PoolWorks.
7171 *
@@ -80,10 +80,10 @@
8181 }
8282 $conf = $wgPoolCounterConf[$type];
8383 $class = $conf['class'];
84 -
 84+
8585 return new $class( $conf, $type, $key );
8686 }
87 -
 87+
8888 protected function __construct( $conf, $type, $key ) {
8989 $this->key = $key;
9090 $this->workers = $conf['workers'];
@@ -125,7 +125,7 @@
126126 */
127127 abstract class PoolCounterWork {
128128 protected $cacheable = false; //Does this override getCachedWork() ?
129 -
 129+
130130 /**
131131 * Actually perform the work, caching it if needed.
132132 */
@@ -140,30 +140,34 @@
141141 }
142142
143143 /**
144 - * A work not so good (eg. expired one) but better than an error
 144+ * A work not so good (eg. expired one) but better than an error
145145 * message.
146146 * @return mixed work result or false
147147 */
148148 function fallback() {
149149 return false;
150150 }
151 -
 151+
152152 /**
153153 * Do something with the error, like showing it to the user.
154154 */
155 - function error( $status ) {
 155+ function error( $status ) {
156156 return false;
157157 }
158158
159159 /**
160160 * Log an error
 161+ *
 162+ * @param $status Status
161163 */
162164 function logError( $status ) {
163165 wfDebugLog( 'poolcounter', $status->getWikiText() );
164166 }
165 -
 167+
166168 /**
167169 * Get the result of the work (whatever it is), or false.
 170+ * @param $skipcache bool
 171+ * @return bool|mixed
168172 */
169173 function execute( $skipcache = false ) {
170174 if ( $this->cacheable && !$skipcache ) {
@@ -183,7 +187,7 @@
184188 $result = $this->doWork();
185189 $this->poolCounter->release();
186190 return $result;
187 -
 191+
188192 case PoolCounter::DONE:
189193 $result = $this->getCachedWork();
190194 if ( $result === false ) {
@@ -193,27 +197,27 @@
194198 return $this->execute( true );
195199 }
196200 return $result;
197 -
 201+
198202 case PoolCounter::QUEUE_FULL:
199203 case PoolCounter::TIMEOUT:
200204 $result = $this->fallback();
201 -
 205+
202206 if ( $result !== false ) {
203207 return $result;
204208 }
205209 /* no break */
206 -
 210+
207211 /* These two cases should never be hit... */
208212 case PoolCounter::ERROR:
209213 default:
210214 $errors = array( PoolCounter::QUEUE_FULL => 'pool-queuefull', PoolCounter::TIMEOUT => 'pool-timeout' );
211 -
212 - $status = Status::newFatal( isset($errors[$status->value]) ? $errors[$status->value] : 'pool-errorunknown' );
 215+
 216+ $status = Status::newFatal( isset( $errors[$status->value] ) ? $errors[$status->value] : 'pool-errorunknown' );
213217 $this->logError( $status );
214218 return $this->error( $status );
215219 }
216220 }
217 -
 221+
218222 function __construct( $type, $key ) {
219223 $this->poolCounter = PoolCounter::factory( $type, $key );
220224 }
Index: trunk/extensions/PoolCounter/PoolCounterClient_body.php
@@ -13,6 +13,10 @@
1414 }
1515 }
1616
 17+ /**
 18+ * @param $key
 19+ * @return Status
 20+ */
1721 function get( $key ) {
1822 $hashes = array();
1923 foreach ( $this->hostNames as $hostName ) {
@@ -47,6 +51,9 @@
4852 return Status::newGood( $conn );
4953 }
5054
 55+ /**
 56+ * @param $conn
 57+ */
5158 function close( $conn ) {
5259 foreach ( $this->conns as $hostName => $otherConn ) {
5360 if ( $conn === $otherConn ) {
@@ -65,6 +72,9 @@
6673 class PoolCounter_Client extends PoolCounter {
6774 private $conn;
6875
 76+ /**
 77+ * @var PoolCounter_ConnectionManager
 78+ */
6979 static private $manager;
7080
7181 function __construct( $conf, $type, $key ) {
@@ -75,6 +85,9 @@
7686 }
7787 }
7888
 89+ /**
 90+ * @return Status
 91+ */
7992 function getConn() {
8093 if ( !isset( $this->conn ) ) {
8194 $status = self::$manager->get( $this->key );
@@ -83,13 +96,16 @@
8497 }
8598 $this->conn = $status->value;
8699
87 - // Set the read timeout to be 1.5 times the pool timeout.
 100+ // Set the read timeout to be 1.5 times the pool timeout.
88101 // This allows the server to time out gracefully before we give up on it.
89102 stream_set_timeout( $this->conn, 0, $this->timeout * 1e6 * 1.5 );
90103 }
91104 return Status::newGood( $this->conn );
92105 }
93106
 107+ /**
 108+ * @return Status
 109+ */
94110 function sendCommand( /*, ...*/ ) {
95111 $args = func_get_args();
96112 $args = str_replace( ' ', '%20', $args );
@@ -127,6 +143,9 @@
128144 }
129145 }
130146
 147+ /**
 148+ * @return Status
 149+ */
131150 function acquireForMe() {
132151 wfProfileIn( __METHOD__ );
133152 $status = $this->sendCommand( 'ACQ4ME', $this->key, $this->workers, $this->maxqueue, $this->timeout );
@@ -134,6 +153,9 @@
135154 return $status;
136155 }
137156
 157+ /**
 158+ * @return Status
 159+ */
138160 function acquireForAnyone() {
139161 wfProfileIn( __METHOD__ );
140162 $status = $this->sendCommand( 'ACQ4ANY', $this->key, $this->workers, $this->maxqueue, $this->timeout );
@@ -141,6 +163,9 @@
142164 return $status;
143165 }
144166
 167+ /**
 168+ * @return Status
 169+ */
145170 function release() {
146171 wfProfileIn( __METHOD__ );
147172 $status = $this->sendCommand( 'RELEASE', $this->key );

Status & tagging log