r108944 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r108943‎ | r108944 | r108945 >
Date:23:11, 14 January 2012
Author:aaron
Status:ok
Tags:filebackend 
Comment:
In SwiftFileBackend:
* Added getContainer(), createContainer(), and deleteContainer() helper functions, which cache container objects to avoid RTTs
* Made doCleanInternal() delete empty containers
* Bumped authentication TTL to 120 seconds
* Some comment tweaks
Modified paths:
  • /trunk/phase3/includes/filerepo/backend/SwiftFileBackend.php (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/filerepo/backend/SwiftFileBackend.php
——@@ -7,26 +7,29 @@
88 */
99
1010 /**
11 - * Class for a Swift based file backend.
12 - * Status messages should avoid mentioning the Swift account name
13 - * Likewise, error suppression should be used to avoid path disclosure.
 11+ * Class for an OpenStack Swift based file backend.
1412 *
1513 * This requires that the php-cloudfiles library is present,
1614 * which is available at https://github.com/rackspace/php-cloudfiles.
1715 * All of the library classes must be registed in $wgAutoloadClasses.
1816 *
 17+ * Status messages should avoid mentioning the Swift account name
 18+ * Likewise, error suppression should be used to avoid path disclosure.
 19+ *
1920 * @ingroup FileBackend
2021 * @since 1.19
2122 */
2223 class SwiftFileBackend extends FileBackend {
2324 /** @var CF_Authentication */
24 - protected $auth; // swift authentication handler
 25+ protected $auth; // Swift authentication handler
 26+
2527 /** @var CF_Connection */
26 - protected $conn; // swift connection handle
 28+ protected $conn; // Swift connection handle
2729 protected $connStarted = 0; // integer UNIX timestamp
 30+ protected $connContainers = array(); // container object cache
 31+ protected $connTTL = 120; // integer seconds
2832
2933 protected $swiftProxyUser; // string
30 - protected $connTTL = 60; // integer seconds
3134
3235 /**
3336 * @see FileBackend::__construct()
——@@ -59,7 +62,7 @@
6063 */
6164 protected function resolveContainerPath( $container, $relStoragePath ) {
6265 if ( strlen( urlencode( $relStoragePath ) ) > 1024 ) {
63 - return null; // too long for swift
 66+ return null; // too long for Swift
6467 }
6568 return $relStoragePath;
6669 }
——@@ -76,16 +79,9 @@
7780 return $status;
7881 }
7982
80 - // (a) Get a swift proxy connection
81 - $conn = $this->getConnection();
82 - if ( !$conn ) {
83 - $status->fatal( 'backend-fail-connect', $this->name );
84 - return $status;
85 - }
86 -
87 - // (b) Check the destination container
 83+ // (a) Check the destination container
8884 try {
89 - $dContObj = $conn->get_container( $dstCont );
 85+ $dContObj = $this->getContainer( $dstCont );
9086 } catch ( NoSuchContainerException $e ) {
9187 $status->fatal( 'backend-fail-create', $params['dst'] );
9288 return $status;
——@@ -98,7 +94,7 @@
9995 return $status;
10096 }
10197
102 - // (c) Check if the destination object already exists
 98+ // (b) Check if the destination object already exists
10399 try {
104100 $dContObj->get_object( $destRel ); // throws NoSuchObjectException
105101 // NoSuchObjectException not thrown: file must exist
——@@ -117,10 +113,10 @@
118114 return $status;
119115 }
120116
121 - // (d) Get a SHA-1 hash of the object
 117+ // (c) Get a SHA-1 hash of the object
122118 $sha1Hash = wfBaseConvert( sha1( $params['content'] ), 16, 36, 31 );
123119
124 - // (e) Actually create the object
 120+ // (d) Actually create the object
125121 try {
126122 $obj = $dContObj->create_object( $destRel );
127123 // Note: metadata keys stored as [Upper case char][[Lower case char]...]
——@@ -150,16 +146,9 @@
151147 return $status;
152148 }
153149
154 - // (a) Get a swift proxy connection
155 - $conn = $this->getConnection();
156 - if ( !$conn ) {
157 - $status->fatal( 'backend-fail-connect', $this->name );
158 - return $status;
159 - }
160 -
161 - // (b) Check the destination container
 150+ // (a) Check the destination container
162151 try {
163 - $dContObj = $conn->get_container( $dstCont );
 152+ $dContObj = $this->getContainer( $dstCont );
164153 } catch ( NoSuchContainerException $e ) {
165154 $status->fatal( 'backend-fail-copy', $params['src'], $params['dst'] );
166155 return $status;
——@@ -172,7 +161,7 @@
173162 return $status;
174163 }
175164
176 - // (c) Check if the destination object already exists
 165+ // (b) Check if the destination object already exists
177166 try {
178167 $dContObj->get_object( $destRel ); // throws NoSuchObjectException
179168 // NoSuchObjectException not thrown: file must exist
——@@ -191,7 +180,7 @@
192181 return $status;
193182 }
194183
195 - // (d) Get a SHA-1 hash of the object
 184+ // (c) Get a SHA-1 hash of the object
196185 $sha1Hash = sha1_file( $params['src'] );
197186 if ( $sha1Hash === false ) { // source doesn't exist?
198187 $status->fatal( 'backend-fail-copy', $params['src'], $params['dst'] );
——@@ -199,7 +188,7 @@
200189 }
201190 $sha1Hash = wfBaseConvert( $sha1Hash, 16, 36, 31 );
202191
203 - // (e) Actually store the object
 192+ // (d) Actually store the object
204193 try {
205194 $obj = $dContObj->create_object( $destRel );
206195 // Note: metadata keys stored as [Upper case char][[Lower case char]...]
——@@ -237,17 +226,10 @@
238227 return $status;
239228 }
240229
241 - // (a) Get a swift proxy connection
242 - $conn = $this->getConnection();
243 - if ( !$conn ) {
244 - $status->fatal( 'backend-fail-connect', $this->name );
245 - return $status;
246 - }
247 -
248 - // (b) Check the source and destination containers
 230+ // (a) Check the source and destination containers
249231 try {
250 - $sContObj = $conn->get_container( $srcCont );
251 - $dContObj = $conn->get_container( $dstCont );
 232+ $sContObj = $this->getContainer( $srcCont );
 233+ $dContObj = $this->getContainer( $dstCont );
252234 } catch ( NoSuchContainerException $e ) {
253235 $status->fatal( 'backend-fail-copy', $params['src'], $params['dst'] );
254236 return $status;
——@@ -260,7 +242,7 @@
261243 return $status;
262244 }
263245
264 - // (c) Check if the destination object already exists
 246+ // (b) Check if the destination object already exists
265247 try {
266248 $dContObj->get_object( $destRel ); // throws NoSuchObjectException
267249 // NoSuchObjectException not thrown: file must exist
——@@ -279,7 +261,7 @@
280262 return $status;
281263 }
282264
283 - // (d) Actually copy the file to the destination
 265+ // (c) Actually copy the file to the destination
284266 try {
285267 $sContObj->copy_object_to( $srcRel, $dContObj, $destRel );
286268 } catch ( NoSuchObjectException $e ) { // source object does not exist
——@@ -306,16 +288,9 @@
307289 return $status;
308290 }
309291
310 - // (a) Get a swift proxy connection
311 - $conn = $this->getConnection();
312 - if ( !$conn ) {
313 - $status->fatal( 'backend-fail-connect', $this->name );
314 - return $status;
315 - }
316 -
317 - // (b) Check the source container
 292+ // (a) Check the source container
318293 try {
319 - $sContObj = $conn->get_container( $srcCont );
 294+ $sContObj = $this->getContainer( $srcCont );
320295 } catch ( NoSuchContainerException $e ) {
321296 $status->fatal( 'backend-fail-delete', $params['src'] );
322297 return $status;
——@@ -328,7 +303,7 @@
329304 return $status;
330305 }
331306
332 - // (c) Actually delete the object
 307+ // (b) Actually delete the object
333308 try {
334309 $sContObj->delete_object( $srcRel );
335310 } catch ( NoSuchObjectException $e ) {
——@@ -351,16 +326,8 @@
352327 protected function doPrepareInternal( $fullCont, $dir, array $params ) {
353328 $status = Status::newGood();
354329
355 - // (a) Get a swift proxy connection
356 - $conn = $this->getConnection();
357 - if ( !$conn ) {
358 - $status->fatal( 'backend-fail-connect', $this->name );
359 - return $status;
360 - }
361 -
362 - // (b) Create the destination container
363330 try {
364 - $conn->create_container( $fullCont );
 331+ $this->createContainer( $fullCont );
365332 } catch ( InvalidResponseException $e ) {
366333 $status->fatal( 'backend-fail-connect', $this->name );
367334 } catch ( Exception $e ) { // some other exception?
——@@ -381,6 +348,45 @@
382349 }
383350
384351 /**
 352+ * @see FileBackend::doCleanInternal()
 353+ */
 354+ protected function doCleanInternal( $fullCont, $dir, array $params ) {
 355+ $status = Status::newGood();
 356+
 357+ // (a) Check the container
 358+ try {
 359+ $contObj = $this->getContainer( $fullCont, true );
 360+ } catch ( NoSuchContainerException $e ) {
 361+ return $status; // ok, nothing to do
 362+ } catch ( InvalidResponseException $e ) {
 363+ $status->fatal( 'backend-fail-connect', $this->name );
 364+ return $status;
 365+ } catch ( Exception $e ) { // some other exception?
 366+ $status->fatal( 'backend-fail-internal', $this->name );
 367+ $this->logException( $e, __METHOD__, $params );
 368+ return $status;
 369+ }
 370+
 371+ // (c) Delete the container if empty
 372+ if ( $contObj->count == 0 ) {
 373+ try {
 374+ $this->deleteContainer( $fullCont );
 375+ } catch ( NoSuchContainerException $e ) {
 376+ return $status; // race?
 377+ } catch ( InvalidResponseException $e ) {
 378+ $status->fatal( 'backend-fail-connect', $this->name );
 379+ return $status;
 380+ } catch ( Exception $e ) { // some other exception?
 381+ $status->fatal( 'backend-fail-internal', $this->name );
 382+ $this->logException( $e, __METHOD__, $params );
 383+ return $status;
 384+ }
 385+ }
 386+
 387+ return $status;
 388+ }
 389+
 390+ /**
385391 * @see FileBackend::doFileExists()
386392 */
387393 protected function doGetFileStat( array $params ) {
——@@ -389,17 +395,12 @@
390396 return false; // invalid storage path
391397 }
392398
393 - $conn = $this->getConnection();
394 - if ( !$conn ) {
395 - return null;
396 - }
397 -
398399 $stat = false;
399400 try {
400 - $container = $conn->get_container( $srcCont );
 401+ $container = $this->getContainer( $srcCont );
401402 // @TODO: handle 'latest' param as "X-Newest: true"
402403 $obj = $container->get_object( $srcRel );
403 - // Convert "Tue, 03 Jan 2012 22:01:04 GMT" to TS_MW
 404+ // Convert dates like "Tue, 03 Jan 2012 22:01:04 GMT" to TS_MW
404405 $date = DateTime::createFromFormat( 'D, d F Y G:i:s e', $obj->last_modified );
405406 if ( $date ) {
406407 $stat = array(
——@@ -431,14 +432,9 @@
432433 return false; // invalid storage path
433434 }
434435
435 - $conn = $this->getConnection();
436 - if ( !$conn ) {
437 - return false;
438 - }
439 -
440436 $data = false;
441437 try {
442 - $container = $conn->get_container( $srcCont );
 438+ $container = $this->getContainer( $srcCont );
443439 $obj = $container->get_object( $srcRel );
444440 $data = $obj->read( $this->headersFromParams( $params ) );
445441 } catch ( NoSuchContainerException $e ) {
——@@ -468,14 +464,9 @@
469465 * @return Array
470466 */
471467 public function getFileListPageInternal( $fullCont, $dir, $after, $limit ) {
472 - $conn = $this->getConnection();
473 - if ( !$conn ) {
474 - return null;
475 - }
476 -
477468 $files = array();
478469 try {
479 - $container = $conn->get_container( $fullCont );
 470+ $container = $this->getContainer( $fullCont );
480471 $files = $container->list_objects( $limit, $after, "{$dir}/" );
481472 } catch ( NoSuchContainerException $e ) {
482473 } catch ( NoSuchObjectException $e ) {
——@@ -510,13 +501,8 @@
511502 $status->fatal( 'backend-fail-invalidpath', $params['src'] );
512503 }
513504
514 - $conn = $this->getConnection();
515 - if ( !$conn ) {
516 - $status->fatal( 'backend-fail-connect', $this->name );
517 - }
518 -
519505 try {
520 - $cont = $conn->get_container( $srcCont );
 506+ $cont = $this->getContainer( $srcCont );
521507 $obj = $cont->get_object( $srcRel );
522508 } catch ( NoSuchContainerException $e ) {
523509 $status->fatal( 'backend-fail-stream', $params['src'] );
——@@ -534,7 +520,7 @@
535521 }
536522
537523 try {
538 - $output = fopen( "php://output", "w" );
 524+ $output = fopen( 'php://output', 'w' );
539525 $obj->stream( $output, $this->headersFromParams( $params ) );
540526 } catch ( InvalidResponseException $e ) {
541527 $status->fatal( 'backend-fail-connect', $this->name );
——@@ -555,11 +541,6 @@
556542 return null;
557543 }
558544
559 - $conn = $this->getConnection();
560 - if ( !$conn ) {
561 - return null;
562 - }
563 -
564545 // Get source file extension
565546 $ext = FileBackend::extensionFromPath( $srcRel );
566547 // Create a new temporary file...
——@@ -569,7 +550,7 @@
570551 }
571552
572553 try {
573 - $cont = $conn->get_container( $srcCont );
 554+ $cont = $this->getContainer( $srcCont );
574555 $obj = $cont->get_object( $srcRel );
575556 $handle = fopen( $tmpFile->getPath(), 'w' );
576557 if ( $handle ) {
——@@ -609,9 +590,10 @@
610591 }
611592
612593 /**
613 - * Get a connection to the swift proxy
 594+ * Get a connection to the Swift proxy
614595 *
615596 * @return CF_Connection|false
 597+ * @throws InvalidResponseException
616598 */
617599 protected function getConnection() {
618600 if ( $this->conn === false ) {
——@@ -620,6 +602,7 @@
621603 // Authenticate with proxy and get a session key.
622604 // Session keys expire after a while, so we renew them periodically.
623605 if ( $this->conn === null || ( time() - $this->connStarted ) > $this->connTTL ) {
 606+ $this->connContainers = array();
624607 try {
625608 $this->auth->authenticate();
626609 $this->conn = new CF_Connection( $this->auth );
——@@ -630,10 +613,59 @@
631614 $this->conn = false; // don't keep re-trying
632615 }
633616 }
 617+ if ( !$this->conn ) {
 618+ throw new InvalidResponseException; // auth/connection problem
 619+ }
634620 return $this->conn;
635621 }
636622
637623 /**
 624+ * Get a Swift container object, possibly from process cache.
 625+ * Use $reCache if the file count or byte count is needed.
 626+ *
 627+ * @param $container string Container name
 628+ * @param $reCache bool Refresh the process cache
 629+ * @return CF_Container
 630+ */
 631+ protected function getContainer( $container, $reCache = false ) {
 632+ $conn = $this->getConnection(); // Swift proxy connection
 633+ if ( $reCache ) {
 634+ unset( $this->connContainers[$container] ); // purge cache
 635+ }
 636+ if ( !isset( $this->connContainers[$container] ) ) {
 637+ $contObj = $conn->get_container( $container );
 638+ // Exception not thrown: container must exist
 639+ $this->connContainers[$container] = $contObj; // cache it
 640+ }
 641+ return $this->connContainers[$container];
 642+ }
 643+
 644+ /**
 645+ * Create a Swift container
 646+ *
 647+ * @param $container string Container name
 648+ * @return CF_Container
 649+ */
 650+ protected function createContainer( $container ) {
 651+ $conn = $this->getConnection(); // Swift proxy connection
 652+ $contObj = $conn->create_container( $container );
 653+ $this->connContainers[$container] = $contObj; // cache it
 654+ return $contObj;
 655+ }
 656+
 657+ /**
 658+ * Delete a Swift container
 659+ *
 660+ * @param $container string Container name
 661+ * @return void
 662+ */
 663+ protected function deleteContainer( $container ) {
 664+ $conn = $this->getConnection(); // Swift proxy connection
 665+ $conn->delete_container( $container );
 666+ unset( $this->connContainers[$container] ); // purge cache
 667+ }
 668+
 669+ /**
638670 * Log an unexpected exception for this backend
639671 *
640672 * @param $e Exception

Follow-up revisions

RevisionCommit summaryAuthorDate
r108945r108944: fixed bogus field...php-cloudfiles documentation misspelled this, it...aaron23:15, 14 January 2012
r109428In SwiftFileBackend:...aaron19:57, 18 January 2012

Status & tagging log