r108188 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r108187‎ | r108188 | r108189 >
Date:01:30, 6 January 2012
Author:aaron
Status:deferred
Tags:
Comment:
* Follow-up r107980: updated Swift backend.
* Redid getFileListInternal() using SwiftFileIterator to handle listing limitations.
* Removed swiftcopy() code merged functionality into doCopyInternal().
* Added wfDebugLog() calls to unexpected exceptions caught.
Modified paths:
  • /branches/FileBackend/phase3/includes/filerepo/backend/SwiftFileBackend.php (modified) (history)

Diff [purge]

Index: branches/FileBackend/phase3/includes/filerepo/backend/SwiftFileBackend.php
——@@ -16,6 +16,7 @@
1717 * All of the library classes must be registed in $wgAutoloadClasses.
1818 *
1919 * @TODO: update MessagesEn for status errors.
 20+ * @TODO: handle 'latest' param as "X-Newest: true".
2021 *
2122 * @ingroup FileBackend
2223 */
——@@ -67,7 +68,7 @@
6869 function doStoreInternal( array $params ) {
6970 $status = Status::newGood();
7071
71 - list( $dstCont, $destRel ) = $this->resolveStoragePath( $params['dst'] );
 72+ list( $dstCont, $destRel ) = $this->resolveStoragePathReal( $params['dst'] );
7273 if ( $destRel === null ) {
7374 $status->fatal( 'backend-fail-invalidpath', $params['dst'] );
7475 return $status;
——@@ -91,6 +92,7 @@
9293 return $status;
9394 } catch ( Exception $e ) { // some other exception?
9495 $status->fatal( 'backend-fail-internal' );
 96+ $this->logException( $e, __METHOD__, $params );
9597 return $status;
9698 }
9799
——@@ -109,6 +111,7 @@
110112 return $status;
111113 } catch ( Exception $e ) { // some other exception?
112114 $status->fatal( 'backend-fail-internal' );
 115+ $this->logException( $e, __METHOD__, $params );
113116 return $status;
114117 }
115118
——@@ -124,6 +127,7 @@
125128 $status->fatal( 'backend-fail-connect' );
126129 } catch ( Exception $e ) { // some other exception?
127130 $status->fatal( 'backend-fail-internal' );
 131+ $this->logException( $e, __METHOD__, $params );
128132 }
129133
130134 return $status;
——@@ -135,13 +139,13 @@
136140 function doCopyInternal( array $params ) {
137141 $status = Status::newGood();
138142
139 - list( $srcCont, $srcRel ) = $this->resolveStoragePath( $params['src'] );
 143+ list( $srcCont, $srcRel ) = $this->resolveStoragePathReal( $params['src'] );
140144 if ( $srcRel === null ) {
141145 $status->fatal( 'backend-fail-invalidpath', $params['src'] );
142146 return $status;
143147 }
144148
145 - list( $dstCont, $destRel ) = $this->resolveStoragePath( $params['dst'] );
 149+ list( $dstCont, $destRel ) = $this->resolveStoragePathReal( $params['dst'] );
146150 if ( $destRel === null ) {
147151 $status->fatal( 'backend-fail-invalidpath', $params['dst'] );
148152 return $status;
——@@ -166,6 +170,7 @@
167171 return $status;
168172 } catch ( Exception $e ) { // some other exception?
169173 $status->fatal( 'backend-fail-internal' );
 174+ $this->logException( $e, __METHOD__, $params );
170175 return $status;
171176 }
172177
——@@ -184,16 +189,20 @@
185190 return $status;
186191 } catch ( Exception $e ) { // some other exception?
187192 $status->fatal( 'backend-fail-internal' );
 193+ $this->logException( $e, __METHOD__, $params );
188194 return $status;
189195 }
190196
191197 // (d) Actually copy the file to the destination
192198 try {
193 - $this->swiftcopy( $sContObj, $srcRel, $dContObj, $destRel );
 199+ $sContObj->copy_object_to( $srcRel, $dContObj, $destRel );
 200+ } catch ( NoSuchObjectException $e ) { // source object does not exist
 201+ $status->fatal( 'backend-fail-copy', $params['src'], $params['dst'] );
194202 } catch ( InvalidResponseException $e ) {
195203 $status->fatal( 'backend-fail-connect' );
196204 } catch ( Exception $e ) { // some other exception?
197205 $status->fatal( 'backend-fail-internal' );
 206+ $this->logException( $e, __METHOD__, $params );
198207 }
199208
200209 return $status;
——@@ -205,7 +214,7 @@
206215 function doDeleteInternal( array $params ) {
207216 $status = Status::newGood();
208217
209 - list( $srcCont, $srcRel ) = $this->resolveStoragePath( $params['src'] );
 218+ list( $srcCont, $srcRel ) = $this->resolveStoragePathReal( $params['src'] );
210219 if ( $srcRel === null ) {
211220 $status->fatal( 'backend-fail-invalidpath', $params['src'] );
212221 return $status;
——@@ -229,6 +238,7 @@
230239 return $status;
231240 } catch ( Exception $e ) { // some other exception?
232241 $status->fatal( 'backend-fail-internal' );
 242+ $this->logException( $e, __METHOD__, $params );
233243 return $status;
234244 }
235245
——@@ -243,6 +253,7 @@
244254 $status->fatal( 'backend-fail-connect' );
245255 } catch ( Exception $e ) { // some other exception?
246256 $status->fatal( 'backend-fail-internal' );
 257+ $this->logException( $e, __METHOD__, $params );
247258 }
248259
249260 return $status;
——@@ -254,7 +265,7 @@
255266 function doCreateInternal( array $params ) {
256267 $status = Status::newGood();
257268
258 - list( $dstCont, $destRel ) = $this->resolveStoragePath( $params['dst'] );
 269+ list( $dstCont, $destRel ) = $this->resolveStoragePathReal( $params['dst'] );
259270 if ( $destRel === null ) {
260271 $status->fatal( 'backend-fail-invalidpath', $params['dst'] );
261272 return $status;
——@@ -278,6 +289,7 @@
279290 return $status;
280291 } catch ( Exception $e ) { // some other exception?
281292 $status->fatal( 'backend-fail-internal' );
 293+ $this->logException( $e, __METHOD__, $params );
282294 return $status;
283295 }
284296
——@@ -296,6 +308,7 @@
297309 return $status;
298310 } catch ( Exception $e ) { // some other exception?
299311 $status->fatal( 'backend-fail-internal' );
 312+ $this->logException( $e, __METHOD__, $params );
300313 return $status;
301314 }
302315
——@@ -309,6 +322,7 @@
310323 $status->fatal( 'backend-fail-connect' );
311324 } catch ( Exception $e ) { // some other exception?
312325 $status->fatal( 'backend-fail-internal' );
 326+ $this->logException( $e, __METHOD__, $params );
313327 }
314328
315329 return $status;
——@@ -317,15 +331,9 @@
318332 /**
319333 * @see FileBackend::prepate()
320334 */
321 - function prepare( array $params ) {
 335+ function doPrepare( $fullCont, $dir, array $params ) {
322336 $status = Status::newGood();
323337
324 - list( $dstCont, $destRel ) = $this->resolveStoragePath( $params['dir'] );
325 - if ( $destRel === null ) {
326 - $status->fatal( 'backend-fail-invalidpath', $params['dir'] );
327 - return $status;
328 - }
329 -
330338 // (a) Get a swift proxy connection
331339 $conn = $this->getConnection();
332340 if ( !$conn ) {
——@@ -335,9 +343,10 @@
336344
337345 // (b) Create the destination container
338346 try {
339 - $conn->create_container( $dstCont );
 347+ $conn->create_container( $fullCont );
340348 } catch ( Exception $e ) { // some other exception?
341349 $status->fatal( 'backend-fail-internal' );
 350+ $this->logException( $e, __METHOD__, $params );
342351 }
343352
344353 return $status;
——@@ -346,17 +355,17 @@
347356 /**
348357 * @see FileBackend::secure()
349358 */
350 - function secure( array $params ) {
 359+ function doSecure( $fullCont, $dir, array $params ) {
351360 $status = Status::newGood();
352361 // @TODO: restrict container from $this->swiftProxyUser
353362 return $status; // badgers? We don't need no steenking badgers!
354363 }
355364
356365 /**
357 - * @see FileBackend::fileExists()
 366+ * @see FileBackend::doFileExists()
358367 */
359 - function fileExists( array $params ) {
360 - list( $srcCont, $srcRel ) = $this->resolveStoragePath( $params['src'] );
 368+ function doFileExists( array $params ) {
 369+ list( $srcCont, $srcRel ) = $this->resolveStoragePathReal( $params['src'] );
361370 if ( $srcRel === null ) {
362371 return false; // invalid storage path
363372 }
——@@ -376,16 +385,17 @@
377386 $exists = false;
378387 } catch ( Exception $e ) { // some other exception?
379388 $exists = false; // fail vs not exists?
 389+ $this->logException( $e, __METHOD__, $params );
380390 }
381391
382392 return $exists;
383393 }
384394
385395 /**
386 - * @see FileBackend::getFileTimestamp()
 396+ * @see FileBackend::doGetFileTimestamp()
387397 */
388 - function getFileTimestamp( array $params ) {
389 - list( $srcCont, $srcRel ) = $this->resolveStoragePath( $params['src'] );
 398+ function doGetFileTimestamp( array $params ) {
 399+ list( $srcCont, $srcRel ) = $this->resolveStoragePathReal( $params['src'] );
390400 if ( $srcRel === null ) {
391401 return false; // invalid storage path
392402 }
——@@ -405,6 +415,7 @@
406416 $obj = NULL;
407417 } catch ( Exception $e ) { // some other exception?
408418 $obj = NULL; // fail vs not exists?
 419+ $this->logException( $e, __METHOD__, $params );
409420 }
410421
411422 if ( $obj ) {
——@@ -417,32 +428,70 @@
418429 }
419430
420431 /**
421 - * @see FileBackend::getFileList()
 432+ * @see FileBackendBase::getFileContents()
422433 */
423 - function getFileList( array $params ) {
424 - list( $dirc, $dir ) = $this->resolveStoragePath( $params['dir'] );
425 - if ( $dir === null ) { // invalid storage path
426 - return array(); // empty result
 434+ public function getFileContents( array $params ) {
 435+ list( $srcCont, $srcRel ) = $this->resolveStoragePathReal( $params['src'] );
 436+ if ( $srcRel === null ) {
 437+ return false; // invalid storage path
427438 }
428439
429440 $conn = $this->getConnection();
430441 if ( !$conn ) {
 442+ $status->fatal( 'backend-fail-connect' );
 443+ return $status;
 444+ }
 445+
 446+ try {
 447+ $container = $conn->get_container( $srcCont);
 448+ $obj = $container->get_object( $srcRel );
 449+ $data = $obj->read();
 450+ } catch ( NoSuchContainerException $e ) {
 451+ $data = false;
 452+ } catch ( NoSuchObjectException $e ) {
 453+ $data = false;
 454+ } catch ( Exception $e ) { // some other exception?
 455+ $data = false; // fail vs not exists?
 456+ $this->logException( $e, __METHOD__, $params );
 457+ }
 458+
 459+ return $data;
 460+ }
 461+
 462+ /**
 463+ * @see FileBackend::getFileListInternal()
 464+ */
 465+ function getFileListInternal( $fullCont, $dir, array $params ) {
 466+ return new SwiftFileIterator( $this, $fullCont, $dir );
 467+ }
 468+
 469+ /**
 470+ * Do not call this function outside of SwiftFileIterator
 471+ *
 472+ * @param $fullCont string Resolved container name
 473+ * @param $dir string Resolved storage directory
 474+ * @param $after string Storage path of file to list items after
 475+ * @param $limit integer Max number of items to list
 476+ * @return Array
 477+ */
 478+ public function getFileListPageInternal( $fullCont, $dir, $after, $limit ) {
 479+ $conn = $this->getConnection();
 480+ if ( !$conn ) {
431481 return null;
432482 }
433483
434 - // @TODO: return an Iterator class that pages via list_objects()
435484 try {
436 - $container = $conn->get_container( $dirc );
437 - $files = $container->list_objects( 0, NULL, $dir );
 485+ $container = $conn->get_container( $fullCont );
 486+ $files = $container->list_objects( $limit, $after, $dir );
438487 } catch ( NoSuchContainerException $e ) {
439488 $files = array();
440489 } catch ( NoSuchObjectException $e ) {
441490 $files = array();
442491 } catch ( Exception $e ) { // some other exception?
443 - $files = null;
 492+ $files = array();
 493+ $this->logException( $e, __METHOD__, $params );
444494 }
445495
446 - // if there are no files matching the prefix, return empty array
447496 return $files;
448497 }
449498
——@@ -450,7 +499,7 @@
451500 * @see FileBackend::getLocalCopy()
452501 */
453502 function getLocalCopy( array $params ) {
454 - list( $srcCont, $srcRel ) = $this->resolveStoragePath( $params['src'] );
 503+ list( $srcCont, $srcRel ) = $this->resolveStoragePathReal( $params['src'] );
455504 if ( $srcRel === null ) {
456505 return null;
457506 }
——@@ -480,6 +529,7 @@
481530 $tmpFile = null;
482531 } catch ( Exception $e ) { // some other exception?
483532 $tmpFile = null;
 533+ $this->logException( $e, __METHOD__, $params );
484534 }
485535
486536 return $tmpFile;
——@@ -511,49 +561,83 @@
512562 }
513563
514564 /**
515 - * Copy a file from one place to another place
516 - *
517 - * @param $srcContainer CF_Container
518 - * @param $srcRel String: relative path to the source file.
519 - * @param $dstContainer CF_Container
520 - * @param $dstRel String: relative path to the destination.
 565+ * Log an unexpected exception for this backend
 566+ *
 567+ * @param $e Exception
 568+ * @param $func string
 569+ * @param $params Array
 570+ * @return void
521571 */
522 - protected function swiftcopy( $srcContainer, $srcRel, $dstContainer, $dstRel ) {
523 - // The destination must exist already.
524 - $obj = $dstContainer->create_object( $dstRel );
525 - $obj->content_type = 'text/plain'; // overwritten by source object.
 572+ protected function logException( Exception $e, $func, array $params ) {
 573+ wfDebugLog( 'SwiftBackend',
 574+ get_class( $e ) . " in '{$this->name}': '{$func}' with " . serialize( $params )
 575+ );
 576+ }
 577+}
526578
527 - try {
528 - $obj->write( '.' );
529 - } catch ( SyntaxException $e ) {
530 - throw new MWException( "Write failed: $e" );
531 - } catch ( BadContentTypeException $e ) {
532 - throw new MWException( "Missing Content-Type: $e" );
533 - } catch ( MisMatchedChecksumException $e ) {
534 - throw new MWException( __METHOD__ . "should not happen: '$e'" );
535 - }
 579+/**
 580+ * SwiftFileBackend helper class to page through object listings.
 581+ * Swift also has a listing limit of 10,000 objects for sanity.
 582+ *
 583+ * @ingroup FileBackend
 584+ */
 585+class SwiftFileIterator implements Iterator {
 586+ /** @var Array */
 587+ protected $bufferIter = array();
 588+ protected $bufferAfter = ''; // string; list items *after* this path
 589+ protected $pos = 0; // integer
536590
537 - try {
538 - $obj = $dstContainer->get_object( $dstRel );
539 - } catch ( NoSuchObjectException $e ) {
540 - throw new MWException( 'The object we just created does not exist: ' .
541 - $dstContainer->name . "/$dstRel: $e" );
542 - }
 591+ /** @var SwiftFileBackend */
 592+ protected $backend;
 593+ protected $container; //
 594+ protected $dir; // string storage directory
543595
544 - try {
545 - $srcObj = $srcContainer->get_object( $srcRel );
546 - } catch ( NoSuchObjectException $e ) {
547 - throw new MWException( 'Source file does not exist: ' .
548 - $srcContainer->name . "/$srcRel: $e" );
549 - }
 596+ const PAGE_SIZE = 5000; // file listing buffer size
550597
551 - try {
552 - $dstContainer->copy_object_from($srcObj,$srcContainer,$dstRel);
553 - } catch ( SyntaxException $e ) {
554 - throw new MWException( 'Source file does not exist: ' .
555 - $srcContainer->name . "/$srcRel: $e" );
556 - } catch ( MisMatchedChecksumException $e ) {
557 - throw new MWException( "Checksums do not match: $e" );
 598+ /**
 599+ * Get an FSFileIterator from a file system directory
 600+ *
 601+ * @param $backend SwiftFileBackend
 602+ * @param $fullCont string Resolved container name
 603+ * @param $dir string Resolved relateive path
 604+ */
 605+ public function __construct( SwiftFileBackend $backend, $fullCont, $dir ) {
 606+ $this->container = $fullCont;
 607+ $this->dir = $dir;
 608+ $this->backend = $backend;
 609+ }
 610+
 611+ public function current() {
 612+ return current( $this->bufferIter );
 613+ }
 614+
 615+ public function key() {
 616+ return $this->pos;
 617+ }
 618+
 619+ public function next() {
 620+ // Advance to the next file in the page
 621+ next( $this->bufferIter );
 622+ ++$this->pos;
 623+ // Check if there are no files left in this page and
 624+ // advance to the next page if this page was not empty.
 625+ if ( !$this->valid() && count( $this->bufferIter ) ) {
 626+ $this->bufferAfter = end( $this->bufferIter );
 627+ $this->bufferIter = $this->backend->getFileListPageInternal(
 628+ $this->container, $this->dir, $this->bufferAfter, self::PAGE_SIZE
 629+ );
558630 }
559631 }
 632+
 633+ public function rewind() {
 634+ $this->pos = 0;
 635+ $this->bufferAfter = '';
 636+ $this->bufferIter = $this->backend->getFileListPageInternal(
 637+ $this->container, $this->dir, $this->bufferAfter, self::PAGE_SIZE
 638+ );
 639+ }
 640+
 641+ public function valid() {
 642+ return ( current( $this->bufferIter ) !== false ); // no paths can have this value
 643+ }
560644 }

Past revisions this follows-up on

RevisionCommit summaryAuthorDate
r107980* Added supported for container sharding via existing FileRepo-style hash pat...aaron01:08, 4 January 2012

Status & tagging log