Refactored entry points to have uniform shutdown handling
authorAaron Schulz <aschulz@wikimedia.org>
Wed, 20 May 2015 23:32:20 +0000 (16:32 -0700)
committerBryanDavis <bdavis@wikimedia.org>
Fri, 29 May 2015 20:40:46 +0000 (20:40 +0000)
* 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
img_auth.php
includes/MediaWiki.php
includes/profiler/Profiler.php
includes/profiler/ProfilerStub.php
load.php
thumb.php

diff --git a/api.php b/api.php
index a9e5683..af7c452 100644 (file)
--- 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' );
index 22fd401..b26e6a5 100644 (file)
@@ -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;
index 58c49f4..d03b76a 100644 (file)
@@ -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;
index dbf80fa..9983fec 100644 (file)
@@ -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.
index 244b4e4..3fe9cdd 100644 (file)
@@ -46,4 +46,7 @@ class ProfilerStub extends Profiler {
 
        public function logData() {
        }
+
+       public function logDataPageOutputOnly() {
+       }
 }
index 0c7ea62..e231807 100644 (file)
--- 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' );
index 2079a64..051c39e 100644 (file)
--- 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' );
 
 //--------------------------------------------------------------------------