In FileBackend/FileOp:

* Added a sane default max file size to FileBackend. Operation batches need to check this before trying anything.
* Temporarily adjust the PHP execution time limit in attemptBatch() to reduce the chance of dying in the middle of it. Also added a maximum batch size limit.
* Added some code comments.
This commit is contained in:
Aaron Schulz 2012-01-19 02:07:48 +00:00
parent 905925716a
commit 95f31706ee
5 changed files with 91 additions and 5 deletions

View file

@ -511,6 +511,7 @@ $wgAutoloadLocalClasses = array(
'MySqlLockManager'=> 'includes/filerepo/backend/lockmanager/DBLockManager.php',
'NullLockManager' => 'includes/filerepo/backend/lockmanager/LockManager.php',
'FileOp' => 'includes/filerepo/backend/FileOp.php',
'FileOpScopedPHPTimeout' => 'includes/filerepo/backend/FileOp.php',
'StoreFileOp' => 'includes/filerepo/backend/FileOp.php',
'CopyFileOp' => 'includes/filerepo/backend/FileOp.php',
'MoveFileOp' => 'includes/filerepo/backend/FileOp.php',

View file

@ -592,6 +592,19 @@ abstract class FileBackend extends FileBackendBase {
/** @var Array */
protected $shardViaHashLevels = array(); // (container name => integer)
protected $maxFileSize = 1000000000; // integer bytes (1GB)
/**
* Get the maximum allowable file size given backend
* medium restrictions and basic performance constraints.
* Do not call this function from places outside FileBackend and FileOp.
*
* @return integer Bytes
*/
final public function maxFileSizeInternal() {
return $this->maxFileSize;
}
/**
* Create a file in the backend with the given contents.
* Do not call this function from places outside FileBackend and FileOp.
@ -605,8 +618,12 @@ abstract class FileBackend extends FileBackendBase {
* @return Status
*/
final public function createInternal( array $params ) {
$status = $this->doCreateInternal( $params );
$this->clearCache( array( $params['dst'] ) );
if ( strlen( $params['content'] ) > $this->maxFileSizeInternal() ) {
$status = Status::newFatal( 'backend-fail-create', $params['dst'] );
} else {
$status = $this->doCreateInternal( $params );
$this->clearCache( array( $params['dst'] ) );
}
return $status;
}
@ -628,8 +645,12 @@ abstract class FileBackend extends FileBackendBase {
* @return Status
*/
final public function storeInternal( array $params ) {
$status = $this->doStoreInternal( $params );
$this->clearCache( array( $params['dst'] ) );
if ( filesize( $params['src'] ) > $this->maxFileSizeInternal() ) {
$status = Status::newFatal( 'backend-fail-store', $params['dst'] );
} else {
$status = $this->doStoreInternal( $params );
$this->clearCache( array( $params['dst'] ) );
}
return $status;
}

View file

@ -34,6 +34,10 @@ abstract class FileOp {
const STATE_CHECKED = 2;
const STATE_ATTEMPTED = 3;
/* Timeout related parameters */
const MAX_BATCH_SIZE = 1000;
const TIME_LIMIT_SEC = 300; // 5 minutes
/**
* Build a new file operation transaction
*
@ -52,6 +56,10 @@ abstract class FileOp {
/**
* Allow stale data for file reads and existence checks.
*
* Note that we don't want to mix stale and non-stale reads
* because stat calls are cached: if we read X without 'latest'
* and then read it with 'latest', the data may still be stale.
*
* @return void
*/
@ -80,6 +88,12 @@ abstract class FileOp {
$allowStale = !empty( $opts['allowStale'] );
$ignoreErrors = !empty( $opts['force'] );
$n = count( $performOps );
if ( $n > self::MAX_BATCH_SIZE ) {
$status->fatal( 'backend-fail-batchsize', $n, self::MAX_BATCH_SIZE );
return $status;
}
$predicates = FileOp::newPredicates(); // account for previous op in prechecks
// Do pre-checks for each operation; abort on failure...
foreach ( $performOps as $index => $fileOp ) {
@ -97,6 +111,11 @@ abstract class FileOp {
}
}
// Restart PHP's execution timer and set the timeout to safe amount.
// This handles cases where the operations take a long time or where we are
// already running low on time left. The old timeout is restored afterwards.
$scopedTimeLimit = new FileOpScopedPHPTimeout( self::TIME_LIMIT_SEC );
// Attempt each operation...
foreach ( $performOps as $index => $fileOp ) {
if ( $fileOp->failed() ) {
@ -325,6 +344,39 @@ abstract class FileOp {
}
}
/**
* FileOp helper class to expand PHP execution time for a function.
* On construction, set_time_limit() is called and set to $seconds.
* When the object goes out of scope, the timer is restarted, with
* the original time limit minus the time the object existed.
*/
class FileOpScopedPHPTimeout {
protected $startTime; // integer seconds
protected $oldTimeout; // integer seconds
/**
* @param $seconds integer
*/
public function __construct( $seconds ) {
if ( ini_get( 'max_execution_time' ) > 0 ) { // CLI uses 0
$this->oldTimeout = ini_set( 'max_execution_time', $seconds );
}
$this->startTime = time();
}
/*
* Restore the original timeout.
* This does not account for the timer value on __construct().
*/
public function __destruct() {
if ( $this->oldTimeout ) {
$elapsed = time() - $this->startTime;
// Note: a limit of 0 is treated as "forever"
set_time_limit( max( 1, $this->oldTimeout - $elapsed ) );
}
}
}
/**
* Store a file into the backend from a file on the file system.
* Parameters similar to FileBackend::storeInternal(), which include:
@ -345,6 +397,11 @@ class StoreFileOp extends FileOp {
$status->fatal( 'backend-fail-notexists', $this->params['src'] );
return $status;
}
// Check if the source file is too big
if ( filesize( $this->params['src'] ) > $this->backend->maxFileSizeInternal() ) {
$status->fatal( 'backend-fail-store', $this->params['dst'] );
return $status;
}
// Check if destination file exists
$status->merge( $this->precheckDestExistence( $predicates ) );
if ( !$status->isOK() ) {
@ -395,6 +452,11 @@ class CreateFileOp extends FileOp {
protected function doPrecheck( array &$predicates ) {
$status = Status::newGood();
// Check if the source data is too big
if ( strlen( $this->params['content'] ) > $this->backend->maxFileSizeInternal() ) {
$status->fatal( 'backend-fail-create', $this->params['dst'] );
return $status;
}
// Check if destination file exists
$status->merge( $this->precheckDestExistence( $predicates ) );
if ( !$status->isOK() ) {

View file

@ -2258,6 +2258,7 @@ If the problem persists, contact an [[Special:ListUsers/sysop|administrator]].',
'backend-fail-connect' => 'Could not connect to file backend "$1".',
'backend-fail-internal' => 'An unknown error occurred in file backend "$1".',
'backend-fail-contenttype' => 'Could not determine the content type of file to store at "$1".',
'backend-fail-batchsize' => 'Backend given a batch of $1 file {{PLURAL:$1|operation|operations}}; the limit is $2.',
# Lock manager
'lockmanager-notlocked' => 'Could not unlock "$1"; it is not locked.',

View file

@ -1367,7 +1367,8 @@ $wgMessageStructure = array(
'backend-fail-synced',
'backend-fail-connect',
'backend-fail-internal',
'backend-fail-contenttype'
'backend-fail-contenttype',
'backend-fail-batchsize'
),
'lockmanager-errors' => array(