From 9bba2d169ed968839b07c85be487e0185cb38ce0 Mon Sep 17 00:00:00 2001 From: Aaron Schulz Date: Fri, 7 Aug 2015 17:08:33 -0700 Subject: [PATCH] Added wfTransactionalTimeLimit() method and applied it * Potentially long running POST requests often use multiple transactions, talk to multiple services, or defer updates. Try to make sure they have a chance to complete all of the work. WMF already sets ignore_user_abort() across the board in config, but this applies it to key spots for all installs, in addition to bumping the time limit. * Eventually this can lower the need for high overall time limits. Bug: T102890 Change-Id: I893ddd773064dcd63b5b24c84c6391974f4b5aee --- RELEASE-NOTES-1.26 | 2 ++ includes/DefaultSettings.php | 8 +++++++ includes/GlobalFunctions.php | 24 +++++++++++++++++++-- includes/actions/Action.php | 10 +++++++++ includes/actions/DeleteAction.php | 2 ++ includes/actions/EditAction.php | 2 ++ includes/actions/RevertAction.php | 2 ++ includes/actions/RollbackAction.php | 3 +++ includes/api/ApiBase.php | 10 +++++++++ includes/api/ApiDelete.php | 2 ++ includes/api/ApiEditPage.php | 2 ++ includes/api/ApiFileRevert.php | 2 ++ includes/api/ApiImageRotate.php | 2 ++ includes/api/ApiImport.php | 2 ++ includes/api/ApiMove.php | 2 ++ includes/api/ApiRevisionDelete.php | 2 ++ includes/api/ApiRollback.php | 2 ++ includes/api/ApiUndelete.php | 2 ++ includes/specialpage/SpecialPage.php | 10 +++++++++ includes/specials/SpecialImport.php | 2 ++ includes/specials/SpecialMergeHistory.php | 2 ++ includes/specials/SpecialMovepage.php | 2 ++ includes/specials/SpecialRevisiondelete.php | 2 ++ includes/specials/SpecialUndelete.php | 2 ++ includes/specials/SpecialUpload.php | 2 ++ includes/specials/SpecialUploadStash.php | 2 ++ 26 files changed, 103 insertions(+), 2 deletions(-) diff --git a/RELEASE-NOTES-1.26 b/RELEASE-NOTES-1.26 index b68c1c9739..986ebc3f44 100644 --- a/RELEASE-NOTES-1.26 +++ b/RELEASE-NOTES-1.26 @@ -55,6 +55,8 @@ production. the store type from $wgObjectCaches. The default is the local database. * Interface message overrides in the MediaWiki namespace will now be cached in memcached and APC (if available), rather than memcached and local files. +* $wgTransactionalTimeLimit was added, which controls the request time limit + for potentially slow POST requests that need to be as atomic as possible. ==== External libraries ==== * Update es5-shim from v4.0.0 to v4.1.5. diff --git a/includes/DefaultSettings.php b/includes/DefaultSettings.php index 19d4b89dfb..9fcdf147f7 100644 --- a/includes/DefaultSettings.php +++ b/includes/DefaultSettings.php @@ -2078,6 +2078,14 @@ $wgMaxArticleSize = 2048; */ $wgMemoryLimit = "50M"; +/** + * The minimum amount of time that MediaWiki needs for "slow" write request, + * particularly ones with multiple non-atomic writes that *should* be as + * transactional as possible; MediaWiki will call set_time_limit() if needed. + * @since 1.26 + */ +$wgTransactionalTimeLimit = 120; + /** @} */ # end performance hacks } /************************************************************************//** diff --git a/includes/GlobalFunctions.php b/includes/GlobalFunctions.php index 2b9bc251d7..f2720df38f 100644 --- a/includes/GlobalFunctions.php +++ b/includes/GlobalFunctions.php @@ -3848,9 +3848,9 @@ function wfStripIllegalFilenameChars( $name ) { } /** - * Set PHP's memory limit to the larger of php.ini or $wgMemoryLimit; + * Set PHP's memory limit to the larger of php.ini or $wgMemoryLimit * - * @return int Value the memory limit was set to. + * @return int Prior memory limit */ function wfMemoryLimit() { global $wgMemoryLimit; @@ -3874,6 +3874,26 @@ function wfMemoryLimit() { return $memlimit; } +/** + * Set PHP's time limit to the larger of php.ini or $wgTransactionalTimeLimit + * + * @return int Prior time limit + * @since 1.26 + */ +function wfTransactionalTimeLimit() { + global $wgTransactionalTimeLimit; + + $timeLimit = ini_get( 'max_execution_time' ); + // Note that CLI scripts use 0 + if ( $timeLimit > 0 && $wgTransactionalTimeLimit > $timeLimit ) { + set_time_limit( $wgTransactionalTimeLimit ); + } + + ignore_user_abort( true ); // ignore client disconnects + + return $timeLimit; +} + /** * Converts shorthand byte notation to integer form * diff --git a/includes/actions/Action.php b/includes/actions/Action.php index bb6a4d5d1a..43f03c3a4f 100644 --- a/includes/actions/Action.php +++ b/includes/actions/Action.php @@ -407,4 +407,14 @@ abstract class Action { * @throws ErrorPageError */ abstract public function show(); + + /** + * Call wfTransactionalTimeLimit() if this request was POSTed + * @since 1.26 + */ + protected function useTransactionalTimeLimit() { + if ( $this->getRequest()->wasPosted() ) { + wfTransactionalTimeLimit(); + } + } } diff --git a/includes/actions/DeleteAction.php b/includes/actions/DeleteAction.php index be21a6f16a..841a94df03 100644 --- a/includes/actions/DeleteAction.php +++ b/includes/actions/DeleteAction.php @@ -41,6 +41,8 @@ class DeleteAction extends FormlessAction { } public function show() { + $this->useTransactionalTimeLimit(); + $out = $this->getOutput(); if ( $this->getContext()->getConfig()->get( 'UseMediaWikiUIEverywhere' ) ) { $out->addModuleStyles( array( diff --git a/includes/actions/EditAction.php b/includes/actions/EditAction.php index 6c8440ac1a..eb53f19ae3 100644 --- a/includes/actions/EditAction.php +++ b/includes/actions/EditAction.php @@ -41,6 +41,8 @@ class EditAction extends FormlessAction { } public function show() { + $this->useTransactionalTimeLimit(); + if ( $this->getContext()->getConfig()->get( 'UseMediaWikiUIEverywhere' ) ) { $out = $this->getOutput(); $out->addModuleStyles( array( diff --git a/includes/actions/RevertAction.php b/includes/actions/RevertAction.php index d025878491..c7f33463f5 100644 --- a/includes/actions/RevertAction.php +++ b/includes/actions/RevertAction.php @@ -107,6 +107,8 @@ class RevertAction extends FormAction { } public function onSubmit( $data ) { + $this->useTransactionalTimeLimit(); + $source = $this->page->getFile()->getArchiveVirtualUrl( $this->getRequest()->getText( 'oldimage' ) ); diff --git a/includes/actions/RollbackAction.php b/includes/actions/RollbackAction.php index 76d70d70af..93669cf4ab 100644 --- a/includes/actions/RollbackAction.php +++ b/includes/actions/RollbackAction.php @@ -36,6 +36,9 @@ class RollbackAction extends FormlessAction { } public function onView() { + // TODO: use $this->useTransactionalTimeLimit(); when POST only + wfTransactionalTimeLimit(); + $details = null; $request = $this->getRequest(); diff --git a/includes/api/ApiBase.php b/includes/api/ApiBase.php index 754c0ed2a6..7b71e6c5a2 100644 --- a/includes/api/ApiBase.php +++ b/includes/api/ApiBase.php @@ -2870,6 +2870,16 @@ abstract class ApiBase extends ContextSource { return $this->getResult()->getData(); } + /** + * Call wfTransactionalTimeLimit() if this request was POSTed + * @since 1.26 + */ + protected function useTransactionalTimeLimit() { + if ( $this->getRequest()->wasPosted() ) { + wfTransactionalTimeLimit(); + } + } + /**@}*/ } diff --git a/includes/api/ApiDelete.php b/includes/api/ApiDelete.php index 6279dfdef8..bdf02bf1cc 100644 --- a/includes/api/ApiDelete.php +++ b/includes/api/ApiDelete.php @@ -39,6 +39,8 @@ class ApiDelete extends ApiBase { * result object. */ public function execute() { + $this->useTransactionalTimeLimit(); + $params = $this->extractRequestParams(); $pageObj = $this->getTitleOrPageId( $params, 'fromdbmaster' ); diff --git a/includes/api/ApiEditPage.php b/includes/api/ApiEditPage.php index b623849246..2f1c01ce7d 100644 --- a/includes/api/ApiEditPage.php +++ b/includes/api/ApiEditPage.php @@ -35,6 +35,8 @@ */ class ApiEditPage extends ApiBase { public function execute() { + $this->useTransactionalTimeLimit(); + $user = $this->getUser(); $params = $this->extractRequestParams(); diff --git a/includes/api/ApiFileRevert.php b/includes/api/ApiFileRevert.php index 5517ee0884..a49397dc81 100644 --- a/includes/api/ApiFileRevert.php +++ b/includes/api/ApiFileRevert.php @@ -38,6 +38,8 @@ class ApiFileRevert extends ApiBase { protected $params; public function execute() { + $this->useTransactionalTimeLimit(); + $this->params = $this->extractRequestParams(); // Extract the file and archiveName from the request parameters $this->validateParameters(); diff --git a/includes/api/ApiImageRotate.php b/includes/api/ApiImageRotate.php index c8390b6fcd..7a544ec0e2 100644 --- a/includes/api/ApiImageRotate.php +++ b/includes/api/ApiImageRotate.php @@ -49,6 +49,8 @@ class ApiImageRotate extends ApiBase { } public function execute() { + $this->useTransactionalTimeLimit(); + $params = $this->extractRequestParams(); $rotation = $params['rotation']; diff --git a/includes/api/ApiImport.php b/includes/api/ApiImport.php index 41540836ef..735cc7fa11 100644 --- a/includes/api/ApiImport.php +++ b/includes/api/ApiImport.php @@ -32,6 +32,8 @@ class ApiImport extends ApiBase { public function execute() { + $this->useTransactionalTimeLimit(); + $user = $this->getUser(); $params = $this->extractRequestParams(); diff --git a/includes/api/ApiMove.php b/includes/api/ApiMove.php index e42958bf5a..aca4378437 100644 --- a/includes/api/ApiMove.php +++ b/includes/api/ApiMove.php @@ -31,6 +31,8 @@ class ApiMove extends ApiBase { public function execute() { + $this->useTransactionalTimeLimit(); + $user = $this->getUser(); $params = $this->extractRequestParams(); diff --git a/includes/api/ApiRevisionDelete.php b/includes/api/ApiRevisionDelete.php index 2896231662..bb5c145140 100644 --- a/includes/api/ApiRevisionDelete.php +++ b/includes/api/ApiRevisionDelete.php @@ -32,6 +32,8 @@ class ApiRevisionDelete extends ApiBase { public function execute() { + $this->useTransactionalTimeLimit(); + $params = $this->extractRequestParams(); $user = $this->getUser(); diff --git a/includes/api/ApiRollback.php b/includes/api/ApiRollback.php index 02e62a0351..6a3346f49e 100644 --- a/includes/api/ApiRollback.php +++ b/includes/api/ApiRollback.php @@ -40,6 +40,8 @@ class ApiRollback extends ApiBase { private $mUser = null; public function execute() { + $this->useTransactionalTimeLimit(); + $user = $this->getUser(); $params = $this->extractRequestParams(); diff --git a/includes/api/ApiUndelete.php b/includes/api/ApiUndelete.php index 28702b1926..cd50ee65c7 100644 --- a/includes/api/ApiUndelete.php +++ b/includes/api/ApiUndelete.php @@ -30,6 +30,8 @@ class ApiUndelete extends ApiBase { public function execute() { + $this->useTransactionalTimeLimit(); + $params = $this->extractRequestParams(); if ( !$this->getUser()->isAllowed( 'undelete' ) ) { diff --git a/includes/specialpage/SpecialPage.php b/includes/specialpage/SpecialPage.php index eb18b8f1af..65a4eb9abf 100644 --- a/includes/specialpage/SpecialPage.php +++ b/includes/specialpage/SpecialPage.php @@ -686,4 +686,14 @@ class SpecialPage { protected function getGroupName() { return 'other'; } + + /** + * Call wfTransactionalTimeLimit() if this request was POSTed + * @since 1.26 + */ + protected function useTransactionalTimeLimit() { + if ( $this->getRequest()->wasPosted() ) { + wfTransactionalTimeLimit(); + } + } } diff --git a/includes/specials/SpecialImport.php b/includes/specials/SpecialImport.php index f9b8ac3be4..4cdf6ddf51 100644 --- a/includes/specials/SpecialImport.php +++ b/includes/specials/SpecialImport.php @@ -57,6 +57,8 @@ class SpecialImport extends SpecialPage { * @throws ReadOnlyError */ function execute( $par ) { + $this->useTransactionalTimeLimit(); + $this->setHeaders(); $this->outputHeader(); diff --git a/includes/specials/SpecialMergeHistory.php b/includes/specials/SpecialMergeHistory.php index 1f0b6d45ce..7edf961acd 100644 --- a/includes/specials/SpecialMergeHistory.php +++ b/includes/specials/SpecialMergeHistory.php @@ -109,6 +109,8 @@ class SpecialMergeHistory extends SpecialPage { } public function execute( $par ) { + $this->useTransactionalTimeLimit(); + $this->checkPermissions(); $this->checkReadOnly(); diff --git a/includes/specials/SpecialMovepage.php b/includes/specials/SpecialMovepage.php index 5682657a45..e77479f32a 100644 --- a/includes/specials/SpecialMovepage.php +++ b/includes/specials/SpecialMovepage.php @@ -64,6 +64,8 @@ class MovePageForm extends UnlistedSpecialPage { } public function execute( $par ) { + $this->useTransactionalTimeLimit(); + $this->checkReadOnly(); $this->setHeaders(); diff --git a/includes/specials/SpecialRevisiondelete.php b/includes/specials/SpecialRevisiondelete.php index 77ebac3efe..7c27ac418c 100644 --- a/includes/specials/SpecialRevisiondelete.php +++ b/includes/specials/SpecialRevisiondelete.php @@ -110,6 +110,8 @@ class SpecialRevisionDelete extends UnlistedSpecialPage { } public function execute( $par ) { + $this->useTransactionalTimeLimit(); + $this->checkPermissions(); $this->checkReadOnly(); diff --git a/includes/specials/SpecialUndelete.php b/includes/specials/SpecialUndelete.php index d7e75bc668..14e295c7ac 100644 --- a/includes/specials/SpecialUndelete.php +++ b/includes/specials/SpecialUndelete.php @@ -771,6 +771,8 @@ class SpecialUndelete extends SpecialPage { } function execute( $par ) { + $this->useTransactionalTimeLimit(); + $user = $this->getUser(); $this->setHeaders(); diff --git a/includes/specials/SpecialUpload.php b/includes/specials/SpecialUpload.php index 6b0bf41379..44be81c928 100644 --- a/includes/specials/SpecialUpload.php +++ b/includes/specials/SpecialUpload.php @@ -152,6 +152,8 @@ class SpecialUpload extends SpecialPage { * @throws UserBlockedError */ public function execute( $par ) { + $this->useTransactionalTimeLimit(); + $this->setHeaders(); $this->outputHeader(); diff --git a/includes/specials/SpecialUploadStash.php b/includes/specials/SpecialUploadStash.php index ad329d3e80..dd90590097 100644 --- a/includes/specials/SpecialUploadStash.php +++ b/includes/specials/SpecialUploadStash.php @@ -58,6 +58,8 @@ class SpecialUploadStash extends UnlistedSpecialPage { * @return bool Success */ public function execute( $subPage ) { + $this->useTransactionalTimeLimit(); + $this->stash = RepoGroup::singleton()->getLocalRepo()->getUploadStash( $this->getUser() ); $this->checkPermissions(); -- 2.20.1