Last modified: 2012-04-16 09:15:39 UTC

Wikimedia Bugzilla is closed!

Wikimedia migrated from Bugzilla to Phabricator. Bug reports are handled in Wikimedia Phabricator.
This static website is read-only and for historical purposes. It is not possible to log in and except for displaying bug reports and their history, links might be broken. See T29336, the corresponding Phabricator task for complete and up-to-date bug report information.
Bug 27336 - Job queue should support exclusion of some jobs from general pop() job
Job queue should support exclusion of some jobs from general pop() job
Status: RESOLVED FIXED
Product: MediaWiki
Classification: Unclassified
General/Unknown (Other open bugs)
unspecified
All All
: Normal enhancement (vote)
: ---
Assigned To: Nobody - You can work on this!
: patch, patch-reviewed
Depends on:
Blocks: 27699
  Show dependency treegraph
 
Reported: 2011-02-11 19:27 UTC by Michael Dale
Modified: 2012-04-16 09:15 UTC (History)
3 users (show)

See Also:
Web browser: ---
Mobile Platform: ---
Assignee Huggle Beta Tester: ---


Attachments
patch to add wgJobExplitRequestTypes (2.12 KB, patch)
2011-02-11 19:27 UTC, Michael Dale
Details
updated patch per Roans sugestions (2.21 KB, patch)
2011-03-04 18:18 UTC, Michael Dale
Details

Description Michael Dale 2011-02-11 19:27:17 UTC
Created attachment 8127 [details]
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.
Comment 1 Roan Kattouw 2011-03-04 10:38:02 UTC
(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.
Comment 2 Sam Reed (reedy) 2011-03-04 10:39:57 UTC
Few minor style issues also - missing spaces on the right hand side
Comment 3 p858snake 2011-03-04 11:38:36 UTC
-need-review +reviewed per roan.
Comment 4 Michael Dale 2011-03-04 18:18:17 UTC
Created attachment 8239 [details]
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
Comment 5 Roan Kattouw 2011-03-05 21:50:50 UTC
(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.
Comment 6 Roan Kattouw 2011-03-20 17:00:51 UTC
Modified patch applied in r84397.
Comment 7 Roan Kattouw 2011-03-20 17:20:14 UTC
(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

Note You need to log in before you can comment on or make changes to this bug.


Navigation
Links