From 89d85a4be417f41c375df8c3d57ab17ff11a3f63 Mon Sep 17 00:00:00 2001 From: Aaron Schulz Date: Wed, 23 Apr 2014 13:42:12 -0700 Subject: [PATCH] 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 --- includes/objectcache/BagOStuff.php | 17 ++++ languages/i18n/en.json | 1 + languages/i18n/qqq.json | 1 + maintenance/language/messages.inc | 1 + thumb.php | 130 ++++++++++++++++------------- 5 files changed, 91 insertions(+), 59 deletions(-) diff --git a/includes/objectcache/BagOStuff.php b/includes/objectcache/BagOStuff.php index 99f73aabe2..74af7a4f26 100644 --- 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 4c844c38c6..86c78ce723 100644 --- 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 2e3bd52215..26322bbfe7 100644 --- 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 a88d1ba197..500d7de314 100644 --- 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 bfaf9a02d0..707f1e26cb 100644 --- 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 * -- 2.20.1