From 016fd0b7762df17529cecc4a190a8734f99adc2a Mon Sep 17 00:00:00 2001 From: Aaron Schulz Date: Wed, 20 May 2015 16:32:20 -0700 Subject: [PATCH] Refactored entry points to have uniform shutdown handling * Added doPreOutputCommit() and doPostOutputShutdown(), which most entry points just using the later * Also fixed problem where text profiling did not show up * Avoid calling triggerJobs() in the file streaming entry points Bug: T100127 Bug: T100085 Change-Id: Ibc7e768fd483389a01847f08cdeba4058c853d3f --- api.php | 17 +------ img_auth.php | 8 ++-- includes/MediaWiki.php | 76 ++++++++++++++++++++---------- includes/profiler/Profiler.php | 15 ++++++ includes/profiler/ProfilerStub.php | 3 ++ load.php | 8 +--- thumb.php | 7 +-- 7 files changed, 79 insertions(+), 55 deletions(-) diff --git a/api.php b/api.php index a9e56830f3..af7c452550 100644 --- a/api.php +++ b/api.php @@ -88,20 +88,9 @@ if ( $processor ) { $processor->execute(); } -if ( function_exists( 'fastcgi_finish_request' ) ) { - fastcgi_finish_request(); -} - -JobQueueGroup::pushLazyJobs(); - -// Execute any deferred updates -DeferredUpdates::doUpdates(); - // Log what the user did, for book-keeping purposes. $endtime = microtime( true ); -wfLogProfilingData(); - // Log the request if ( $wgAPIRequestLog ) { $items = array( @@ -130,7 +119,5 @@ if ( $wgAPIRequestLog ) { wfDebug( "Logged API request to $wgAPIRequestLog\n" ); } -// Shut down the database. foo()->bar() syntax is not supported in PHP4: we won't ever actually -// get here to worry about whether this should be = or =&, but the file has to parse properly. -$lb = wfGetLBFactory(); -$lb->shutdown(); +$mediawiki = new MediaWiki(); +$mediawiki->doPostOutputShutdown( 'fast' ); diff --git a/img_auth.php b/img_auth.php index 22fd401452..b26e6a5ec9 100644 --- a/img_auth.php +++ b/img_auth.php @@ -46,11 +46,9 @@ $wgArticlePath = false; # Don't let a "/*" article path clober our action path $wgActionPaths = array( "$wgUploadPath/" ); wfImageAuthMain(); -wfLogProfilingData(); -// Commit and close up! -$factory = wfGetLBFactory(); -$factory->commitMasterChanges(); -$factory->shutdown(); + +$mediawiki = new MediaWiki(); +$mediawiki->doPostOutputShutdown( 'fast' ); function wfImageAuthMain() { global $wgImgAuthUrlPathMap; diff --git a/includes/MediaWiki.php b/includes/MediaWiki.php index 58c49f4bac..d03b76af2e 100644 --- a/includes/MediaWiki.php +++ b/includes/MediaWiki.php @@ -433,37 +433,68 @@ class MediaWiki { // Bug 62091: while exceptions are convenient to bubble up GUI errors, // they are not internal application faults. As with normal requests, this // should commit, print the output, do deferred updates, jobs, and profiling. - wfGetLBFactory()->commitMasterChanges(); + $this->doPreOutputCommit(); $e->report(); // display the GUI error } } catch ( Exception $e ) { MWExceptionHandler::handleException( $e ); } - if ( function_exists( 'register_postsend_function' ) ) { - // https://github.com/facebook/hhvm/issues/1230 - register_postsend_function( array( $this, 'postSendUpdates' ) ); - } elseif ( function_exists( 'fastcgi_finish_request' ) ) { - fastcgi_finish_request(); - $this->postSendUpdates(); - } else { - $this->postSendUpdates(); - } + $this->doPostOutputShutdown( 'normal' ); + } + + /** + * This function commits all DB changes as needed before + * the user can receive a response (in case commit fails) + * + * @since 1.26 + */ + public function doPreOutputCommit() { + // Either all DBs should commit or none + ignore_user_abort( true ); + wfGetLBFactory()->commitMasterChanges(); } /** * This function does work that can be done *after* the * user gets the HTTP response so they don't block on it * + * @param string $mode Use 'fast' to always skip job running * @since 1.26 */ - public function postSendUpdates() { - try { - JobQueueGroup::pushLazyJobs(); - $this->triggerJobs(); - $this->restInPeace(); - } catch ( Exception $e ) { - MWExceptionHandler::handleException( $e ); + public function doPostOutputShutdown( $mode = 'normal' ) { + // Show profiling data if enabled + Profiler::instance()->logDataPageOutputOnly(); + + $that = $this; + $callback = function () use ( $that, $mode ) { + try { + // Assure deferred updates are not in the main transaction + wfGetLBFactory()->commitMasterChanges(); + // Run jobs occasionally, if enabled + if ( $mode === 'normal' ) { + $that->triggerJobs(); + } + // Do deferred updates and job insertion and final commit + $that->restInPeace(); + } catch ( Exception $e ) { + MWExceptionHandler::handleException( $e ); + } + }; + + if ( function_exists( 'register_postsend_function' ) ) { + // https://github.com/facebook/hhvm/issues/1230 + register_postsend_function( $callback ); + } else { + if ( function_exists( 'fastcgi_finish_request' ) ) { + fastcgi_finish_request(); + } else { + // Either all DB and deferred updates should happen or none. + // The later should not be cancelled due to client disconnect. + ignore_user_abort( true ); + } + + $callback(); } } @@ -602,16 +633,13 @@ class MediaWiki { // Actually do the work of the request and build up any output $this->performRequest(); - // Either all DB and deferred updates should happen or none. - // The later should not be cancelled due to client disconnect. - ignore_user_abort( true ); // Now commit any transactions, so that unreported errors after - // output() don't roll back the whole DB transaction - wfGetLBFactory()->commitMasterChanges(); + // output() don't roll back the whole DB transaction and so that + // we avoid having both success and error text in the response + $this->doPreOutputCommit(); // Output everything! $this->context->getOutput()->output(); - } /** @@ -644,7 +672,7 @@ class MediaWiki { * to run a specified number of jobs. This registers a callback to cleanup * the socket once it's done. */ - protected function triggerJobs() { + public function triggerJobs() { $jobRunRate = $this->config->get( 'JobRunRate' ); if ( $jobRunRate <= 0 || wfReadOnly() ) { return; diff --git a/includes/profiler/Profiler.php b/includes/profiler/Profiler.php index dbf80fa13b..9983fece38 100644 --- a/includes/profiler/Profiler.php +++ b/includes/profiler/Profiler.php @@ -230,6 +230,21 @@ abstract class Profiler { } } + /** + * Output current data to the page output if configured to do so + * + * @throws MWException + * @since 1.26 + */ + public function logDataPageOutputOnly() { + foreach ( $this->getOutputs() as $output ) { + if ( $output instanceof ProfilerOutputText ) { + $stats = $this->getFunctionStats(); + $output->log( $stats ); + } + } + } + /** * Get the content type sent out to the client. * Used for profilers that output instead of store data. diff --git a/includes/profiler/ProfilerStub.php b/includes/profiler/ProfilerStub.php index 244b4e4b39..3fe9cddb3f 100644 --- a/includes/profiler/ProfilerStub.php +++ b/includes/profiler/ProfilerStub.php @@ -46,4 +46,7 @@ class ProfilerStub extends Profiler { public function logData() { } + + public function logDataPageOutputOnly() { + } } diff --git a/load.php b/load.php index 0c7ea62e3f..e23180799b 100644 --- a/load.php +++ b/load.php @@ -41,11 +41,7 @@ $configFactory = ConfigFactory::getDefaultInstance(); $resourceLoader = new ResourceLoader( $configFactory->makeConfig( 'main' ) ); $resourceLoader->respond( new ResourceLoaderContext( $resourceLoader, $wgRequest ) ); -JobQueueGroup::pushLazyJobs(); - Profiler::instance()->setTemplated( true ); -wfLogProfilingData(); -// Shut down the database. -$lb = wfGetLBFactory(); -$lb->shutdown(); +$mediawiki = new MediaWiki(); +$mediawiki->doPostOutputShutdown( 'fast' ); diff --git a/thumb.php b/thumb.php index 2079a64766..051c39ea9d 100644 --- a/thumb.php +++ b/thumb.php @@ -35,11 +35,8 @@ if ( defined( 'THUMB_HANDLER' ) ) { wfStreamThumb( $_GET ); } -wfLogProfilingData(); -// Commit and close up! -$factory = wfGetLBFactory(); -$factory->commitMasterChanges(); -$factory->shutdown(); +$mediawiki = new MediaWiki(); +$mediawiki->doPostOutputShutdown( 'fast' ); //-------------------------------------------------------------------------- -- 2.20.1