Added pool counter support for all thumb.php requests

* This can limit how many workers work on a given file at once,
  regardless of which specific thumbnail parameters are requested.
* Also added a BagOStuff::incrWithInit() method per DRY.
* Tweaked the register_shutdown_function() callback to avoid
  File/MediaTransformOutput references, since they may involve
  TempFSFile objects.

Change-Id: Ic01461cb974ab23b179ac3b60cf6de12e36360f6

diff --git a/includes/objectcache/BagOStuff.php b/includes/objectcache/BagOStuff.php
index 99f73aa..74af7a4 100644 (file)
--- a/includes/objectcache/BagOStuff.php
+++ b/includes/objectcache/BagOStuff.php
@@ -307,6 +307,23 @@ abstract class BagOStuff {
                return $this->incr( $key, - $value );
        }
 
+       /**
+        * Increase stored value of $key by $value while preserving its TTL
+        *
+        * This will create the key with value $init and TTL $ttl if not present
+        *
+        * @param string $key
+        * @param integer $ttl
+        * @param integer $value
+        * @param integer $init
+        * @return bool
+        * @since 1.24
+        */
+       public function incrWithInit( $key, $ttl, $value = 1, $init = 1 ) {
+               return $this->incr( $key, $value ) ||
+                       $this->add( $key, $init, $ttl ) || $this->incr( $key, $value );
+       }
+
        /**
         * Get the "last error" registered; clearLastError() should be called manually
         * @return int ERR_* constant for the "last error" registry

diff --git a/languages/i18n/en.json b/languages/i18n/en.json
index 4c844c3..86c78ce 100644 (file)
--- a/languages/i18n/en.json
+++ b/languages/i18n/en.json
@@ -227,6 +227,7 @@
     "jumptonavigation": "navigation",
     "jumptosearch": "search",
     "view-pool-error": "Sorry, the servers are overloaded at the moment.\nToo many users are trying to view this page.\nPlease wait a while before you try to access this page again.\n\n$1",
+    "generic-pool-error": "Sorry, the servers are overloaded at the moment.\nToo many users are trying to view this resource.\nPlease wait a while before you try to access this resource again.",
     "pool-timeout": "Timeout waiting for the lock",
     "pool-queuefull": "Pool queue is full",
     "pool-errorunknown": "Unknown error",

diff --git a/languages/i18n/qqq.json b/languages/i18n/qqq.json
index 2e3bd52..26322bb 100644 (file)
--- a/languages/i18n/qqq.json
+++ b/languages/i18n/qqq.json
@@ -389,6 +389,7 @@
        "jumptonavigation": "Part of the \"jump to\" navigation links. Hidden by default in monobook skin. The format is: [[MediaWiki:Jumpto/{{SUBPAGENAME}}|{{int:jumpto}}]] {{int:jumptonavigation}}, [[MediaWiki:Jumptosearch/{{SUBPAGENAME}}|{{int:jumptosearch}}]].\n\n{{Identical|Navigation}}",
        "jumptosearch": "Part of the \"jump to\" navigation links. Hidden by default in monobook skin. The format is: [[MediaWiki:Jumpto/{{SUBPAGENAME}}|{{int:jumpto}}]] [[MediaWiki:Jumptonavigation/{{SUBPAGENAME}}|{{int:jumptonavigation}}]], {{int:jumptosearch}}.\n\n{{Identical|Search}}",
        "view-pool-error": "Error message. Parameters:\n* $1 - probably unused",
+       "generic-pool-error": "Error message",
        "pool-timeout": "Part of {{msg-mw|view-pool-error}}.\n\nFor explanation of 'lock' see [[w:Lock_(computer_science)|wikipedia]].",
        "pool-queuefull": "Part of {{msg-mw|view-pool-error}}\n\n\"Pool\" refers to a pool of processes.",
        "pool-errorunknown": "Part of {{msg-mw|view-pool-error}}.\n{{Identical|Unknown error}}",

diff --git a/maintenance/language/messages.inc b/maintenance/language/messages.inc
index a88d1ba..500d7de 100644 (file)
--- a/maintenance/language/messages.inc
+++ b/maintenance/language/messages.inc
@@ -264,6 +264,7 @@ $wgMessageStructure = array(
                'jumptonavigation',
                'jumptosearch',
                'view-pool-error',
+               'generic-pool-error',
                'pool-timeout',
                'pool-queuefull',
                'pool-errorunknown',

diff --git a/thumb.php b/thumb.php
index bfaf9a0..707f1e2 100644 (file)
--- a/thumb.php
+++ b/thumb.php
@@ -324,31 +324,15 @@ function wfStreamThumb( array $params ) {
        } elseif ( $user->pingLimiter( 'renderfile' ) ) {
                wfThumbError( 500, wfMessage( 'actionthrottledtext' ) );
                return;
-       } elseif ( wfThumbIsAttemptThrottled( $img, $thumbName, 4 ) ) {
-               wfThumbError( 500, wfMessage( 'thumbnail_image-failure-limit', 4 ) );
-               return;
        }
 
-       // Thumbnail isn't already there, so create the new thumbnail...
-       $thumb = null;
-       try {
-               // Record failures on PHP fatals too
-               register_shutdown_function( function() use ( &$thumb, $img, $thumbName ) {
-                       if ( $thumb === null ) { // transform() gave a fatal
-                               wfThumbIncrAttemptFailures( $img, $thumbName );
-                       }
-               } );
-               $thumb = $img->transform( $params, File::RENDER_NOW );
-       } catch ( Exception $ex ) {
-               // Tried to select a page on a non-paged file?
-               $thumb = false;
-       }
+       // Actually generate a new thumbnail
+       list( $thumb, $errorMsg ) = wfGenerateThumbnail( $img, $params, $thumbName );
 
        // Check for thumbnail generation errors...
-       $errorMsg = false;
        $msg = wfMessage( 'thumbnail_error' );
        if ( !$thumb ) {
-               $errorMsg = $msg->rawParams( 'File::transform() returned false' )->escaped();
+               $errorMsg = $errorMsg ?: $msg->rawParams( 'File::transform() returned false' )->escaped();
        } elseif ( $thumb->isError() ) {
                $errorMsg = $thumb->getHtmlMsg();
        } elseif ( !$thumb->hasFile() ) {
@@ -359,7 +343,6 @@ function wfStreamThumb( array $params ) {
        }
 
        if ( $errorMsg !== false ) {
-               wfThumbIncrAttemptFailures( $img, $thumbName );
                wfThumbError( 500, $errorMsg );
        } else {
                // Stream the file if there were no errors
@@ -367,6 +350,74 @@ function wfStreamThumb( array $params ) {
        }
 }
 
+/**
+ * Actually try to generate a new thumbnail
+ *
+ * @param File $file
+ * @param array $params
+ * @param string $thumbName
+ * @return array (MediaTransformOutput|bool, string|bool error message HTML)
+ */
+function wfGenerateThumbnail( File $file, array $params, $thumbName ) {
+       global $wgMemc, $wgAttemptFailureEpoch;
+
+       $key = wfMemcKey( 'attempt-failures', $wgAttemptFailureEpoch,
+               $file->getRepo()->getName(), md5( $file->getName() ), md5( $thumbName ) );
+
+       // Check if this file keeps failing to render
+       if ( $wgMemc->get( $key ) >= 4 ) {
+               return array( false, wfMessage( 'thumbnail_image-failure-limit', 4 ) );
+       }
+
+       $done = false;
+       // Record failures on PHP fatals in addition to caching exceptions
+       register_shutdown_function( function() use ( &$done, $key ) {
+               if ( !$done ) { // transform() gave a fatal
+                       global $wgMemc;
+                       $wgMemc->incrWithInit( $key, 3600 );
+               }
+       } );
+
+       $thumb = false;
+       $errorHtml = false;
+
+       // Thumbnail isn't already there, so create the new thumbnail...
+       try {
+               $work = new PoolCounterWorkViaCallback( 'FileRender', sha1( $file->getName() ),
+                       array(
+                               'doWork' => function() use ( $file, $params ) {
+                                       return $file->transform( $params, File::RENDER_NOW );
+                               },
+                               'getCachedWork' => function() use ( $file, $params ) {
+                                       return $file->transform( $params );
+                               },
+                               'fallback' => function() {
+                                       return wfMessage( 'generic-pool-error' )->parse();
+                               },
+                               'error' => function ( $status ) {
+                                       return $status->getHTML();
+                               }
+                       )
+               );
+               $result = $work->execute();
+               if ( $result instanceof MediaTransformOutput ) {
+                       $thumb = $result;
+               } elseif ( is_string( $result ) ) { // error
+                       $errorHtml = $result;
+               }
+       } catch ( Exception $e ) {
+               // Tried to select a page on a non-paged file?
+       }
+
+       $done = true; // no PHP fatal occured
+
+       if ( !$thumb || $thumb->isError() ) {
+               $wgMemc->incrWithInit( $key, 3600 );
+       }
+
+       return array( $thumb, $errorHtml );
+}
+
 /**
  * Returns true if this thumbnail is one that MediaWiki generates
  * links to on file description pages and possibly parser output.
@@ -398,45 +449,6 @@ function wfThumbIsStandard( File $img, array $params ) {
        return true;
 }
 
-/**
- * @param File $img
- * @param string $thumbName
- * @param int $limit
- * @return int|bool
- */
-function wfThumbIsAttemptThrottled( File $img, $thumbName, $limit ) {
-       global $wgMemc;
-
-       return ( $wgMemc->get( wfThumbAttemptKey( $img, $thumbName ) ) >= $limit );
-}
-
-/**
- * @param File $img
- * @param string $thumbName
- */
-function wfThumbIncrAttemptFailures( File $img, $thumbName ) {
-       global $wgMemc;
-
-       $key = wfThumbAttemptKey( $img, $thumbName );
-       if ( !$wgMemc->incr( $key, 1 ) ) {
-               if ( !$wgMemc->add( $key, 1, 3600 ) ) {
-                       $wgMemc->incr( $key, 1 );
-               }
-       }
-}
-
-/**
- * @param File $img
- * @param string $thumbName
- * @return string
- */
-function wfThumbAttemptKey( File $img, $thumbName ) {
-       global $wgAttemptFailureEpoch;
-
-       return wfMemcKey( 'attempt-failures', $wgAttemptFailureEpoch,
-               $img->getRepo()->getName(), md5( $img->getName() ), md5( $thumbName ) );
-}
-
 /**
  * Convert pathinfo type parameter, into normal request parameters
  *