Job queue should support exclusion of some jobs from general pop() job
Closed, ResolvedPublic

Description

Author: mdale

Description:
patch to add wgJobExplitRequestTypes

I propose adding $wgJobExplitRequestTypes configuration variable.. any job you don't want running in the general job pool you can put it in there.

This patch is mostly needed for the transcode extension where you don't want transcode jobs being spawned off from the web request and you would probably want some machines dedicated to running transcoes using the runJobs.php --type option

Likewise other jobQueue boxes should not be running the transcode jobs.

patch attached.


Version: unspecified
Severity: enhancement

attachment jobQueue.php.patch ignored as obsolete

bzimport added a subscriber: Unknown Object (MLST).
bzimport set Reference to bz27336.
bzimport created this task.Via LegacyFeb 11 2011, 7:27 PM
Catrope added a comment.Via ConduitMar 4 2011, 10:38 AM

(In reply to comment #0)

patch attached.

  • Don't use empty()
  • Don't use + to merge arrays with numerical indices, it doesn't do what you think it does. In this case, it causes the job_id >= $offset condition to be overwritten with the first element of $conditions. Use array_merge() instead
  • $wgJobExplitRequestTypes is misspelled, should be $wgJobExplicitRequestTypes. I'm also not a big fan of that name, but then I can't think of anything better either

Looks good otherwise.

Reedy added a comment.Via ConduitMar 4 2011, 10:39 AM

Few minor style issues also - missing spaces on the right hand side

Peachey88 added a comment.Via ConduitMar 4 2011, 11:38 AM

-need-review +reviewed per roan.

bzimport added a comment.Via ConduitMar 4 2011, 6:18 PM

mdale wrote:

updated patch per Roans sugestions

Thanks for the review, updated per your suggestions. ( Yea I remember running into issues with + indexed array merges but must of forgot about that in this quick patch :( ... I can't think of a much better name either without getting more wordy... maybe we go a bit more wordy for a bit more clarity: $wgJobsTypesExcludedFromDefaultQueue

Attached: updated27336.patch.txt

Catrope added a comment.Via ConduitMar 5 2011, 9:50 PM

(In reply to comment #4)

Created attachment 8239 [details]
updated patch per Roans sugestions

The if and foreach statements don't follow whitespace conventions. Patch is good otherwise.

Thanks for the review, updated per your suggestions. ( Yea I remember running
into issues with + indexed array merges but must of forgot about that in this
quick patch :( ... I can't think of a much better name either without getting
more wordy... maybe we go a bit more wordy for a bit more clarity:
$wgJobsTypesExcludedFromDefaultQueue

I think the doc comment in DefaultSettings.php should more clearly say that it's about job types that are skipped by normal job runners. The var name you suggested already goes a long way towards clarifying that IMO.

Attached: updated27336.patch.txt

Catrope added a comment.Via ConduitMar 20 2011, 5:00 PM

Modified patch applied in r84397.

Catrope added a comment.Via ConduitMar 20 2011, 5:20 PM

(In reply to comment #6)

Modified patch applied in r84397.

Modified in the sense that I changed the name of the $wg var, tweaked its doc comment, and changed strencode to addQuotes

Add Comment