r106605 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r106604‎ | r106605 | r106606 >
Date:21:50, 18 December 2011
Author:aaron
Status:deferred
Tags:
Comment:
* Added support for process caching and made it used for SHA-1. Non-FS backends will be more inclined to use this (PHP alreay caches FS stats).
* Refactored LockServerDaemon a bit to avoid "connect on construct" antipattern.
Modified paths:
  • /branches/FileBackend/phase3/includes/filerepo/backend/FSFileBackend.php (modified) (history)
  • /branches/FileBackend/phase3/includes/filerepo/backend/FileBackend.php (modified) (history)
  • /branches/FileBackend/phase3/includes/filerepo/backend/FileBackendMultiWrite.php (modified) (history)
  • /branches/FileBackend/phase3/maintenance/locking/LockServerDaemon.php (modified) (history)

Diff [purge]

Index: branches/FileBackend/phase3/maintenance/locking/LockServerDaemon.php
——@@ -35,11 +35,14 @@
3636 /** @var Array */
3737 protected $sessionIndexEx = array(); // (session => key => 1)
3838
 39+ protected $address; // string (IP/hostname)
 40+ protected $port; // integer
3941 protected $authKey; // string key
4042 protected $connTimeout; // array ( 'sec' => integer, 'usec' => integer )
4143 protected $lockTimeout; // integer number of seconds
4244 protected $maxLocks; // integer
4345 protected $maxClients; // integer
 46+ protected $maxBacklog; // integer
4447
4548 protected $startTime; // integer UNIX timestamp
4649 protected $lockCount = 0; // integer
——@@ -70,7 +73,10 @@
7174 }
7275 }
7376
 77+ $this->address = $config['address'];
 78+ $this->port = $config['port'];
7479 $this->authKey = $config['authKey'];
 80+
7581 $connTimeout = isset( $config['connTimeout'] )
7682 ? $config['connTimeout']
7783 : 1.5;
——@@ -87,10 +93,15 @@
8894 $this->maxClients = isset( $config['maxClients'] )
8995 ? $config['maxClients']
9096 : 100;
91 - $backlog = isset( $config['maxBacklog'] )
 97+ $this->maxBacklog = isset( $config['maxBacklog'] )
9298 ? $config['maxBacklog']
9399 : 10;
 100+ }
94101
 102+ /**
 103+ * @return void
 104+ */
 105+ protected function setupSocket() {
95106 if ( !function_exists( 'socket_create' ) ) {
96107 throw new Exception( "PHP sockets extension missing from PHP CLI mode." );
97108 }
——@@ -99,10 +110,10 @@
100111 throw new Exception( "socket_create(): " . socket_strerror( socket_last_error() ) );
101112 }
102113 socket_set_option( $sock, SOL_SOCKET, SO_REUSEADDR, 1 ); // bypass 2MLS
103 - if ( socket_bind( $sock, $config['address'], $config['port'] ) === false ) {
 114+ if ( socket_bind( $sock, $this->address, $this->port ) === false ) {
104115 throw new Exception( "socket_bind(): " .
105116 socket_strerror( socket_last_error( $sock ) ) );
106 - } elseif ( socket_listen( $sock, $backlog ) === false ) {
 117+ } elseif ( socket_listen( $sock, $this->maxBacklog ) === false ) {
107118 throw new Exception( "socket_listen(): " .
108119 socket_strerror( socket_last_error( $sock ) ) );
109120 }
——@@ -115,6 +126,8 @@
116127 * @return void
117128 */
118129 public function main() {
 130+ // Setup socket and start listing
 131+ $this->setupSocket();
119132 // Create a list of all the clients that will be connected to us.
120133 $clients = array( $this->sock ); // start off with listening socket
121134 do {
Index: branches/FileBackend/phase3/includes/filerepo/backend/FileBackendMultiWrite.php
——@@ -93,6 +93,10 @@
9494 return $status; // abort
9595 }
9696
 97+ // Clear any cache entries (after locks acquired)
 98+ foreach ( $this->fileBackends as $backend ) {
 99+ $backend->clearCache();
 100+ }
97101 // Actually attempt the operation batch...
98102 $status->merge( FileOp::attemptBatch( $performOps, $opts ) );
99103
——@@ -116,7 +120,7 @@
117121 $newOp[$par] = preg_replace(
118122 '!^mwstore://' . preg_quote( $this->name ) . '/!',
119123 'mwstore://' . $backend->getName() . '/',
120 - $newOp[$par]
 124+ $newOp[$par] // string or array
121125 );
122126 }
123127 }
Index: branches/FileBackend/phase3/includes/filerepo/backend/FSFileBackend.php
——@@ -38,7 +38,7 @@
3939 return null;
4040 }
4141
42 - function store( array $params ) {
 42+ protected function doStore( array $params ) {
4343 $status = Status::newGood();
4444
4545 list( $c, $dest ) = $this->resolveStoragePath( $params['dst'] );
——@@ -79,7 +79,7 @@
8080 return $status;
8181 }
8282
83 - function copy( array $params ) {
 83+ protected function doCopy( array $params ) {
8484 $status = Status::newGood();
8585
8686 list( $c, $source ) = $this->resolveStoragePath( $params['src'] );
——@@ -127,7 +127,7 @@
128128 return $status;
129129 }
130130
131 - function move( array $params ) {
 131+ protected function doMove( array $params ) {
132132 $status = Status::newGood();
133133
134134 list( $c, $source ) = $this->resolveStoragePath( $params['src'] );
——@@ -176,7 +176,7 @@
177177 return $status;
178178 }
179179
180 - function delete( array $params ) {
 180+ protected function doDelete( array $params ) {
181181 $status = Status::newGood();
182182
183183 list( $c, $source ) = $this->resolveStoragePath( $params['src'] );
——@@ -203,7 +203,7 @@
204204 return $status;
205205 }
206206
207 - function concatenate( array $params ) {
 207+ protected function doConcatenate( array $params ) {
208208 $status = Status::newGood();
209209
210210 list( $c, $dest ) = $this->resolveStoragePath( $params['dst'] );
——@@ -301,7 +301,7 @@
302302 return $status;
303303 }
304304
305 - function create( array $params ) {
 305+ protected function doCreate( array $params ) {
306306 $status = Status::newGood();
307307
308308 list( $c, $dest ) = $this->resolveStoragePath( $params['dst'] );
Index: branches/FileBackend/phase3/includes/filerepo/backend/FileBackend.php
——@@ -356,6 +356,10 @@
357357 * @since 1.19
358358 */
359359 abstract class FileBackend extends FileBackendBase {
 360+ /** @var Array */
 361+ protected $cache = array(); // (storage path => key => value)
 362+ protected $maxCacheSize = 50; // integer; max paths with entries
 363+
360364 /**
361365 * Store a file into the backend from a file on disk.
362366 * Do not call this function from places outside FileBackend and FileOp.
——@@ -367,9 +371,18 @@
368372 * @param $params Array
369373 * @return Status
370374 */
371 - abstract public function store( array $params );
 375+ final public function store( array $params ) {
 376+ $status = $this->doStore( $params );
 377+ $this->clearCache( array( $params['dst'] ) );
 378+ return $status;
 379+ }
372380
373381 /**
 382+ * @see FileBackend::store()
 383+ */
 384+ abstract protected function doStore( array $params );
 385+
 386+ /**
374387 * Copy a file from one storage path to another in the backend.
375388 * Do not call this function from places outside FileBackend and FileOp.
376389 * $params include:
——@@ -380,9 +393,18 @@
381394 * @param $params Array
382395 * @return Status
383396 */
384 - abstract public function copy( array $params );
 397+ final public function copy( array $params ) {
 398+ $status = $this->doCopy( $params );
 399+ $this->clearCache( array( $params['dst'] ) );
 400+ return $status;
 401+ }
385402
386403 /**
 404+ * @see FileBackend::copy()
 405+ */
 406+ abstract protected function doCopy( array $params );
 407+
 408+ /**
387409 * Delete a file at the storage path.
388410 * Do not call this function from places outside FileBackend and FileOp.
389411 * $params include:
——@@ -391,9 +413,18 @@
392414 * @param $params Array
393415 * @return Status
394416 */
395 - abstract public function delete( array $params );
 417+ final public function delete( array $params ) {
 418+ $status = $this->doDelete( $params );
 419+ $this->clearCache( array( $params['src'] ) );
 420+ return $status;
 421+ }
396422
397423 /**
 424+ * @see FileBackend::delete()
 425+ */
 426+ abstract protected function doDelete( array $params );
 427+
 428+ /**
398429 * Move a file from one storage path to another in the backend.
399430 * Do not call this function from places outside FileBackend and FileOp.
400431 * $params include:
——@@ -404,7 +435,16 @@
405436 * @param $params Array
406437 * @return Status
407438 */
408 - public function move( array $params ) {
 439+ final public function move( array $params ) {
 440+ $status = $this->doMove( $params );
 441+ $this->clearCache( array( $params['src'], $params['dst'] ) );
 442+ return $status;
 443+ }
 444+
 445+ /**
 446+ * @see FileBackend::move()
 447+ */
 448+ protected function doMove( array $params ) {
409449 // Copy source to dest
410450 $status = $this->backend->copy( $params );
411451 if ( !$status->isOK() ) {
——@@ -426,9 +466,18 @@
427467 * @param $params Array
428468 * @return Status
429469 */
430 - abstract public function concatenate( array $params );
 470+ final public function concatenate( array $params ) {
 471+ $status = $this->doConcatenate( $params );
 472+ $this->clearCache( array( $params['dst'] ) );
 473+ return $status;
 474+ }
431475
432476 /**
 477+ * @see FileBackend::concatenate()
 478+ */
 479+ abstract protected function doConcatenate( array $params );
 480+
 481+ /**
433482 * Create a file in the backend with the given contents.
434483 * Do not call this function from places outside FileBackend and FileOp.
435484 * $params include:
——@@ -439,8 +488,17 @@
440489 * @param $params Array
441490 * @return Status
442491 */
443 - abstract public function create( array $params );
 492+ final public function create( array $params ) {
 493+ $status = $this->doCreate( $params );
 494+ $this->clearCache( array( $params['dst'] ) );
 495+ return $status;
 496+ }
444497
 498+ /**
 499+ * @see FileBackend::create()
 500+ */
 501+ abstract protected function doCreate( array $params );
 502+
445503 public function prepare( array $params ) {
446504 return Status::newGood();
447505 }
——@@ -454,11 +512,20 @@
455513 }
456514
457515 public function getFileSha1Base36( array $params ) {
458 - $fsFile = $this->getLocalReference( array( 'src' => $params['src'] ) );
 516+ $path = $params['src'];
 517+ if ( isset( $this->cache[$path]['sha1'] ) ) {
 518+ return $this->cache[$path]['sha1'];
 519+ }
 520+ $fsFile = $this->getLocalReference( array( 'src' => $path ) );
459521 if ( !$fsFile ) {
460522 return false;
461523 } else {
462 - return $fsFile->getSha1Base36();
 524+ $sha1 = $fsFile->getSha1Base36();
 525+ if ( $sha1 !== false ) { // don't cache negatives
 526+ $this->trimCache(); // limit memory
 527+ $this->cache[$path]['sha1'] = $sha1;
 528+ }
 529+ return $sha1;
463530 }
464531 }
465532
——@@ -565,6 +632,8 @@
566633 }
567634 }
568635
 636+ // Clear any cache entries (after locks acquired)
 637+ $this->clearCache();
569638 // Actually attempt the operation batch...
570639 $status->merge( FileOp::attemptBatch( $performOps, $opts ) );
571640
——@@ -572,6 +641,35 @@
573642 }
574643
575644 /**
 645+ * Invalidate the file existence and property cache
 646+ *
 647+ * @param $paths Array Clear cache for specific files
 648+ * @return void
 649+ */
 650+ final public function clearCache( array $paths = null ) {
 651+ if ( $paths === null ) {
 652+ $this->cache = array();
 653+ } else {
 654+ foreach ( $paths as $path ) {
 655+ unset( $this->cache[$path] );
 656+ }
 657+ }
 658+ }
 659+
 660+ /**
 661+ * Prune the cache if it is too big to add an item
 662+ *
 663+ * @return void
 664+ */
 665+ protected function trimCache() {
 666+ if ( count( $this->cache ) >= $this->maxCacheSize ) {
 667+ reset( $this->cache );
 668+ $key = key( $this->cache );
 669+ unset( $this->cache[$key] );
 670+ }
 671+ }
 672+
 673+ /**
576674 * Check if a given path is a mwstore:// path.
577675 * This does not do any actual validation or existence checks.
578676 *

Status & tagging log