jenkins-bot has submitted this change and it was merged.
Change subject: user-job-throttle
......................................................................
user-job-throttle
the number of gwtoolsetUploadMediafileJobs allowed to be added by a
gwtoolsetUploadMetadataJob was throttled by a value the user had no
control over. while testing the extension we realized that this value
is too generic and that the end user should have some control over
its setting.
Bug: 58417
Change-Id: I8fc0850bdf7b65b9e7ef2dd41c1b96901fd91828
---
M GWToolset.i18n.php
M includes/Config.php
M includes/Forms/MetadataDetectForm.php
M includes/Forms/MetadataMappingForm.php
M includes/Handlers/Forms/MetadataDetectHandler.php
M includes/Handlers/Forms/MetadataMappingHandler.php
M includes/Handlers/UploadHandler.php
M includes/Handlers/Xml/XmlMappingHandler.php
M includes/Jobs/UploadMetadataJob.php
M includes/Utils.php
10 files changed, 176 insertions(+), 16 deletions(-)
Approvals:
BryanDavis: Looks good to me, approved
CSteipp: Looks good to me, but someone else must approve
jenkins-bot: Verified
diff --git a/GWToolset.i18n.php b/GWToolset.i18n.php
index 82484bc..9d16b66 100644
--- a/GWToolset.i18n.php
+++ b/GWToolset.i18n.php
@@ -41,6 +41,7 @@
'gwtoolset-no-accepted-types' => 'No accepted types provided',
'gwtoolset-no-callback' => 'No callback passed to this method.',
'gwtoolset-no-comment' => "<code>user_options['comment']</code> not
set.",
+ 'gwtoolset-no-default' => 'No default value provided.',
'gwtoolset-no-field-size' => 'No field size specified for the field
"$1".',
'gwtoolset-no-file-backend-name' => 'No file backend name provided.',
'gwtoolset-no-file-backend-container' => 'No file backend container
name provided.',
@@ -48,7 +49,10 @@
'gwtoolset-no-form-handler' => 'No form handler created.',
'gwtoolset-no-mapping' => 'No <code>mapping_name</code> provided.',
'gwtoolset-no-mapping-json' => 'No <code>mapping_json</code> provided.',
+ 'gwtoolset-no-max' => 'No maximum value provided.',
+ 'gwtoolset-no-mediafile-throttle' => 'No mediafile job throttle
provided.',
'gwtoolset-no-mediawiki-template' => 'No
<code>mediawiki-template-name</code> provided.',
+ 'gwtoolset-no-min' => 'No minimum value provided.',
'gwtoolset-no-module' => 'No module name was specified.',
'gwtoolset-no-mwstore-complete-path' => 'No complete file path
provided.',
'gwtoolset-no-mwstore-relative-path' => 'No relative path provided.',
@@ -136,6 +140,8 @@
'gwtoolset-accepted-file-types' => 'Accepted file
{{PLURAL:$1|type|types}}:',
'gwtoolset-ensure-well-formed-xml' => 'Make sure the XML file is
well-formed with this $1.',
'gwtoolset-file-url-invalid' => 'The file URL was invalid; The file
does not yet exist in the wiki. You need to first upload the file from your
computer if you want to use the file URL reference in the form.',
+ 'gwtoolset-mediafile-throttle' => 'Mediafile throttle:',
+ 'gwtoolset-mediafile-throttle-description' => 'The throttle controls
the load Wikimedia Commons will put on your media server during the batch
upload. You can set the throttle between 1-20 where the number corresponds to
the number of media requests per minute.',
'gwtoolset-mediawiki-template-does-not-exist' => 'MediaWiki template
"<strong>$1</strong>" does not exist in the wiki.
Either import the template or select another MediaWiki template to use for
mapping.',
@@ -297,7 +303,9 @@
'gwtoolset-ignorewarnings' => 'Hint to the developer that appears when
ignorewarnings is not set.',
'gwtoolset-incorrect-form-handler' => 'A developer message that appears
when a module does not specify a form handler that extends
GWToolset\\Handlers\\Forms\\FormHandler.',
'gwtoolset-job-throttle-exceeded' => 'Developer message that appears
when the batch job throttle was exceeded.',
- 'gwtoolset-no-accepted-types' => 'Hint to the developer that appears
when no accepted types are provided.
+ 'gwtoolset-mediafile-throttle' => 'Mediafile throttle label for the
HTML form.',
+ 'gwtoolset-mediafile-throttle-description' => 'User description of the
mediafile throttle.',
+'gwtoolset-no-accepted-types' => 'Hint to the developer that appears when no
accepted types are provided.
Used if <code>$accepted_metadata_types</code> (metadata types the extension
accepts) is empty.
@@ -305,6 +313,7 @@
* $1 - (Unused) "gwtoolset-no-accepted-types-provided" (untranslatable)',
'gwtoolset-no-callback' => 'Hint to the developer that appears when no
callback is given.',
'gwtoolset-no-comment' => "Hint to the developer that appears when
user_options['comment'] is not set.",
+ 'gwtoolset-no-default' => 'Developer message that appears when no
default value was provided.',
'gwtoolset-no-field-size' => 'Developer message that appears when no
field size was specified for the field. Parameters:
* $1 is the name field.',
'gwtoolset-no-file-backend-name' => 'Message that appears when a web
admin does not provide a file backend name.',
@@ -313,7 +322,10 @@
'gwtoolset-no-form-handler' => 'Hint to the developer that appears when
no form handler was created.',
'gwtoolset-no-mapping' => 'Hint to the developer that appears when no
mapping_name is provided.',
'gwtoolset-no-mapping-json' => 'Hint to the developer that appears when
no mapping_json is provided.',
+ 'gwtoolset-no-max' => 'Developer message that appears when no maximum
value was provided.',
+ 'gwtoolset-no-mediafile-throttle' => 'Developer message that appears
when no mediafile job throttle was provided.',
'gwtoolset-no-mediawiki-template' => 'Hint to the developer that
appears when no mediawiki-template-name is provided.',
+ 'gwtoolset-no-min' => 'Developer message that appears when no minimum
value was provided.',
'gwtoolset-no-module' => 'Hint to the developer that appears when no
module name was specified.',
'gwtoolset-no-mwstore-complete-path' => 'Developer message that appears
when no mwstore complete file path provied.',
'gwtoolset-no-mwstore-relative-path' => 'Developer message that appears
when no mwstore relative path is provided.',
diff --git a/includes/Config.php b/includes/Config.php
index 5b4b512..7b7d47a 100644
--- a/includes/Config.php
+++ b/includes/Config.php
@@ -70,9 +70,28 @@
/**
* @var {int}
- * the maximum number of mediafile jobs to add to the job queue during
this run.
+ * the number of mediafile jobs to add to the job queue during this run.
*/
- public static $mediafile_job_throttle = 10;
+ public static $mediafile_job_throttle_default = 10;
+
+ /**
+ * @var {int}
+ * a min range for the mediafile job throttle.
+ */
+ public static $mediafile_job_throttle_min = 1;
+
+ /**
+ * @var {int}
+ * a max range for the mediafile job throttle.
+ *
+ * based on an irc chat with chris steipp. his main concern is about
someone setting up
+ * files that are near the max upload size, approx 1GB, which would
likely dos a server
+ * and use up all of the MediaWiki server’s bandwidth.
+ *
+ * @see https://gerrit.wikimedia.org/r/#/c/101008
+ * @see
https://www.assembla.com/spaces/glamwiki/tickets/195-user-controlled-job-throttle
+ */
+ public static $mediafile_job_throttle_max = 20;
/**
* @var {int}
@@ -116,10 +135,22 @@
/**
* @var {string}
+ * this delay is used specifically for each gwtoolsetUploadMetadataJob
+ *
* used in conjunction with the job param jobReleaseTimestamp when the
job queue
* delayedJobsEnabled() is true, e.g., JobQueueRedis
*/
- public static $metadata_job_delay = '3 minutes';
+ public static $metadata_job_delay = '1 minute';
+
+ /**
+ * @var {string}
+ * this delay is used specifically when the job queue for
gwtoolsetUploadMediafileJobs
+ * has reached the $mediafile_job_queue_max
+ *
+ * used in conjunction with the job param jobReleaseTimestamp when the
job queue
+ * delayedJobsEnabled() is true, e.g., JobQueueRedis
+ */
+ public static $metadata_job_attempt_delay = '5 minutes';
/**
* @var {int}
diff --git a/includes/Forms/MetadataDetectForm.php
b/includes/Forms/MetadataDetectForm.php
index a496cfa..e0900a1 100644
--- a/includes/Forms/MetadataDetectForm.php
+++ b/includes/Forms/MetadataDetectForm.php
@@ -226,6 +226,30 @@
Html::rawElement(
'li',
array(),
+ Html::rawElement(
+ 'label',
+ array(),
+ wfMessage(
'gwtoolset-mediafile-throttle' )->escaped() .
+ Html::rawElement(
+ 'input',
+ array(
+ 'type' => 'text',
+ 'name' =>
'gwtoolset-mediafile-throttle',
+ 'min' =>
Config::$mediafile_job_throttle_min,
+ 'max' =>
Config::$mediafile_job_throttle_max,
+ 'maxlength' => 2,
+ 'size' => 2,
+ 'placeholder' =>
Config::$mediafile_job_throttle_default
+ )
+ )
+ ) .
+ Html::rawElement( 'br' ) .
+ wfMessage(
'gwtoolset-mediafile-throttle-description' )->escaped()
+ ) .
+
+ Html::rawElement(
+ 'li',
+ array(),
wfMessage( 'gwtoolset-ensure-well-formed-xml'
)->params(
Html::rawElement(
'a',
diff --git a/includes/Forms/MetadataMappingForm.php
b/includes/Forms/MetadataMappingForm.php
index 4114139..77fe108 100644
--- a/includes/Forms/MetadataMappingForm.php
+++ b/includes/Forms/MetadataMappingForm.php
@@ -155,6 +155,15 @@
'input',
array(
'type' => 'hidden',
+ 'name' =>
'gwtoolset-mediafile-throttle',
+ 'value' =>
(int)$user_options['gwtoolset-mediafile-throttle']
+ )
+ ) .
+
+ Html::rawElement(
+ 'input',
+ array(
+ 'type' => 'hidden',
'id' =>
'gwtoolset-mediawiki-template-name',
'name' =>
'gwtoolset-mediawiki-template-name',
'value' => Utils::sanitizeString(
$user_options['gwtoolset-mediawiki-template-name'] )
diff --git a/includes/Handlers/Forms/MetadataDetectHandler.php
b/includes/Handlers/Forms/MetadataDetectHandler.php
index 5dd7fa1..fdcbac1 100644
--- a/includes/Handlers/Forms/MetadataDetectHandler.php
+++ b/includes/Handlers/Forms/MetadataDetectHandler.php
@@ -31,6 +31,7 @@
protected $_expected_post_fields = array(
'gwtoolset-form' => array( 'size' => 255 ),
'gwtoolset-mediawiki-template-name' => array( 'size' => 255 ),
+ 'gwtoolset-mediafile-throttle' => array( 'size' => 2 ),
'gwtoolset-metadata-file-upload' => array( 'size' => 255 ),
'gwtoolset-metadata-mapping-url' => array( 'size' => 255 ),
'gwtoolset-record-element-name' => array( 'size' => 255 ),
@@ -110,6 +111,18 @@
*/
protected function getUserOptions() {
return array(
+ 'gwtoolset-mediafile-throttle' =>
+ !empty(
$this->_whitelisted_post['gwtoolset-mediafile-throttle'] )
+ ? Utils::sanitizeNumericRange(
+
$this->_whitelisted_post['gwtoolset-mediafile-throttle'],
+ array(
+ 'min' =>
Config::$mediafile_job_throttle_min,
+ 'max' =>
Config::$mediafile_job_throttle_max,
+ 'default' =>
Config::$mediafile_job_throttle_default
+ )
+ )
+ : Config::$mediafile_job_throttle_default,
+
'gwtoolset-mediawiki-template-name' => !empty(
$this->_whitelisted_post['gwtoolset-mediawiki-template-name']
)
diff --git a/includes/Handlers/Forms/MetadataMappingHandler.php
b/includes/Handlers/Forms/MetadataMappingHandler.php
index 5f6afa3..d9122c0 100644
--- a/includes/Handlers/Forms/MetadataMappingHandler.php
+++ b/includes/Handlers/Forms/MetadataMappingHandler.php
@@ -44,6 +44,7 @@
'gwtoolset-category-metadata' => array( 'size' => 255 ),
'gwtoolset-form' => array( 'size' => 255 ),
'gwtoolset-preview' => array( 'size' => 255 ),
+ 'gwtoolset-mediafile-throttle' => array( 'size' => 2 ),
'gwtoolset-mediawiki-template-name' => array( 'size' => 255 ),
'gwtoolset-metadata-file-relative-path' => array( 'size' => 255
),
'gwtoolset-metadata-file-sha1' => array( 'size' => 255 ),
@@ -195,6 +196,18 @@
!empty( $this->_whitelisted_post['wpSummary'] )
? $this->_whitelisted_post['wpSummary']
: '',
+
+ 'gwtoolset-mediafile-throttle' =>
+ !empty(
$this->_whitelisted_post['gwtoolset-mediafile-throttle'] )
+ ? Utils::sanitizeNumericRange(
+
$this->_whitelisted_post['gwtoolset-mediafile-throttle'],
+ array(
+ 'min' =>
Config::$mediafile_job_throttle_min,
+ 'max' =>
Config::$mediafile_job_throttle_max,
+ 'default' =>
Config::$mediafile_job_throttle_default
+ )
+ )
+ : Config::$mediafile_job_throttle_default,
'gwtoolset-mediawiki-template-name' =>
!empty(
$this->_whitelisted_post['gwtoolset-mediawiki-template-name'] )
@@ -411,8 +424,8 @@
}
// at this point
- // * the UploadMetadataJob has created (
Config::$mediafile_job_throttle ) number of
- // UploadMediafileJobs
+ // * the UploadMetadataJob has created (
$user_options['gwtoolset-mediafile-throttle'] )
+ // number of UploadMediafileJobs
// * $user_options['gwtoolset-record-begin'] is the
value that the UploadMetadataJob
// began with
// * $user_options['gwtoolset-record-current'] is the
next record that needs to be
@@ -420,7 +433,7 @@
//
// example to illustrate the test
// * Config::$preview_throttle = 3
- // * Config::$mediafile_job_throttle = 10
+ // * $user_options['gwtoolset-mediafile-throttle'] =
10
// * $user_options['gwtoolset-record-count'] = 14
// * $user_options['gwtoolset-record-begin'] = 4 (
because the preview took care of 3 )
// * $user_options['gwtoolset-record-current'] = 14 (
13 mediafiles will have been
@@ -432,7 +445,8 @@
// * create another UploadMetadataJob that will take
care of the last record
if (
(int)$user_options['gwtoolset-record-count']
- >= (
(int)$user_options['gwtoolset-record-begin'] +
(int)Config::$mediafile_job_throttle )
+ >= (
(int)$user_options['gwtoolset-record-begin'] +
+
(int)$user_options['gwtoolset-mediafile-throttle'] )
) {
$this->_whitelisted_post['gwtoolset-record-begin'] =
(int)$user_options['gwtoolset-record-current'];
@@ -508,7 +522,7 @@
);
if ( $user_options['preview'] === true ) {
- Config::$mediafile_job_throttle =
(int)Config::$preview_throttle;
+ $user_options['gwtoolset-mediafile-throttle'] =
(int)Config::$preview_throttle;
$mediafile_titles = $this->processMetadata(
$user_options );
$result = PreviewForm::getForm(
diff --git a/includes/Handlers/UploadHandler.php
b/includes/Handlers/UploadHandler.php
index 1831fa2..f7fc2cc 100644
--- a/includes/Handlers/UploadHandler.php
+++ b/includes/Handlers/UploadHandler.php
@@ -500,11 +500,11 @@
* @return {bool}
*/
public function saveMediafileViaJob(
- array &$user_options, array $options, array $whitelisted_post
+ array $user_options, array $options, array $whitelisted_post
) {
$result = false;
- if ( count( $this->mediafile_jobs ) >
(int)Config::$mediafile_job_throttle ) {
+ if ( count( $this->mediafile_jobs ) >
(int)$user_options['gwtoolset-mediafile-throttle'] ) {
throw new MWException(
wfMessage( 'gwtoolset-developer-issue' )
->params(
@@ -646,7 +646,7 @@
* @param {array} $options
* @throws {MWException}
*/
- protected function validateUserOptions( array &$user_options ) {
+ protected function validateUserOptions( array $user_options ) {
if ( !isset( $user_options['comment'] ) ) {
throw new MWException(
wfMessage( 'gwtoolset-developer-issue' )
@@ -655,10 +655,10 @@
);
}
- if ( !isset( $user_options['save-as-batch-job'] ) ) {
+ if ( !isset( $user_options['gwtoolset-mediafile-throttle'] ) ) {
throw new MWException(
wfMessage( 'gwtoolset-developer-issue' )
- ->params( wfMessage(
'gwtoolset-no-save-as-batch' )->parse() )
+ ->params( wfMessage(
'gwtoolset-no-mediafile-throttle' )->parse() )
->parse()
);
}
@@ -670,5 +670,13 @@
->parse()
);
}
+
+ if ( !isset( $user_options['save-as-batch-job'] ) ) {
+ throw new MWException(
+ wfMessage( 'gwtoolset-developer-issue' )
+ ->params( wfMessage(
'gwtoolset-no-save-as-batch' )->parse() )
+ ->parse()
+ );
+ }
}
}
diff --git a/includes/Handlers/Xml/XmlMappingHandler.php
b/includes/Handlers/Xml/XmlMappingHandler.php
index 4f3f0c9..614b999 100644
--- a/includes/Handlers/Xml/XmlMappingHandler.php
+++ b/includes/Handlers/Xml/XmlMappingHandler.php
@@ -347,7 +347,8 @@
// the record nr we should begin
processing on plus the job throttle
if (
(int)$user_options['gwtoolset-record-current']
- >=
(int)$user_options['gwtoolset-record-begin'] +
(int)Config::$mediafile_job_throttle
+ >= (
(int)$user_options['gwtoolset-record-begin'] +
+
(int)$user_options['gwtoolset-mediafile-throttle'] )
) {
$result['stop-reading'] = true;
break;
diff --git a/includes/Jobs/UploadMetadataJob.php
b/includes/Jobs/UploadMetadataJob.php
index d8f5a0c..125f6f7 100644
--- a/includes/Jobs/UploadMetadataJob.php
+++ b/includes/Jobs/UploadMetadataJob.php
@@ -105,7 +105,7 @@
if ( $delayed_enabled ) {
$job->params['jobReleaseTimestamp'] = strtotime(
- '+' . Utils::sanitizeString(
Config::$metadata_job_delay )
+ '+' . Utils::sanitizeString(
Config::$metadata_job_attempt_delay )
);
}
diff --git a/includes/Utils.php b/includes/Utils.php
index 1d5db56..03d48f1 100644
--- a/includes/Utils.php
+++ b/includes/Utils.php
@@ -284,6 +284,54 @@
}
/**
+ * @param {int} $value
+ * @param {array} $params
+ * @param {int} $params['min']
+ * @param {int} $params['max']
+ * @param {int} $params['default']
+ * @return {int}
+ */
+ public static function sanitizeNumericRange( $value, $params ) {
+ if ( !isset( $params['min'] ) ) {
+ throw new MWException(
+ __METHOD__ . ': ' .
+ wfMessage( 'gwtoolset-developer-issue' )
+ ->params( wfMessage( 'gwtoolset-no-min' ) )
+ ->escaped()
+ );
+ }
+
+ if ( !isset( $params['max'] ) ) {
+ throw new MWException(
+ __METHOD__ . ': ' .
+ wfMessage( 'gwtoolset-developer-issue' )
+ ->params( wfMessage( 'gwtoolset-no-max' ) )
+ ->escaped()
+ );
+ }
+
+ if ( !isset( $params['min'] ) ) {
+ throw new MWException(
+ __METHOD__ . ': ' .
+ wfMessage( 'gwtoolset-developer-issue' )
+ ->params( wfMessage( 'gwtoolset-no-default' ) )
+ ->escaped()
+ );
+ }
+
+ $result = (int)$params['default'];
+ $value = (int)$value;
+
+ if ( $value >= (int)$params['min']
+ && $value <= (int)$params['max']
+ ) {
+ $result = $value;
+ }
+
+ return $result;
+ }
+
+ /**
* @param {string} $string
* @return {string|null}
*/
--
To view, visit https://gerrit.wikimedia.org/r/101008
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I8fc0850bdf7b65b9e7ef2dd41c1b96901fd91828
Gerrit-PatchSet: 7
Gerrit-Project: mediawiki/extensions/GWToolset
Gerrit-Branch: master
Gerrit-Owner: Dan-nl <d_ent...@yahoo.com>
Gerrit-Reviewer: BryanDavis <bda...@wikimedia.org>
Gerrit-Reviewer: CSteipp <cste...@wikimedia.org>
Gerrit-Reviewer: Dan-nl <d_ent...@yahoo.com>
Gerrit-Reviewer: Gergő Tisza <gti...@wikimedia.org>
Gerrit-Reviewer: Siebrand <siebr...@wikimedia.org>
Gerrit-Reviewer: jenkins-bot
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
