Why:
- The JobQueue docs currently do not describe the
semantics of handling job execution failures.
- Some other features, such as job deduplication, enqueue semantics and
the overall execution flow, are also missing from the architecture
document.
What:
- Expand the JobQueue architecture document to describe how jobs that
return false from Job::run() or complete exceptionally are treated.
- Rework some preexisting sections of the same document for brevity.
- Describe JobSpecification and JobQueueGroup in the same document.
- Describe the different between push() and lazyPush() semantics.
- Describe the execution flow of a Job.
- Briefly summarize how job deduplication is expected to function.
Change-Id: Ib0c1f165feabefe862710b28977b598faf637ec5
Callers should not catch an unchecked exception, so it doesn't belong
in a function signature. Unchecked exceptions indicate a coding error,
which by definition the code will not be able to handle correctly.
If any of these exceptions were supposed to be in response to an edge
case, user input, or initial conditions, then they should be changed
to a runtime error. If the exception class cannot be changed, then
the annotation should include a comment explaining its purpose and
prognosis.
Bug: T240672
Change-Id: I2e640b9737cb68090a8e1cb70067d1b74037d647
get_debug_type() does the same thing but better (spelling type names
in the same way as in type declarations, and including names of
object classes and resource types). It was added in PHP 8, but the
symfony/polyfill-php80 package provides it while we still support 7.4.
Also remove uses of get_class() and get_resource_type() where the new
method already provides the same information.
For reference:
https://www.php.net/manual/en/function.get-debug-type.phphttps://www.php.net/manual/en/function.gettype.php
In this commit I'm only changing code where it looks like the result
is used only for some king of debug, log, or test output. This
probably won't break anything important, but I'm not sure whether
anything might depend on the exact values.
Change-Id: I7c1f0a8f669228643e86f8e511c0e26a2edb2948
This patch introduces a namespace declaration for the
MediaWiki\Json to FormatJson and establishes a class
alias marked as deprecated since version 1.43.
Bug: T353458
Change-Id: I5e1311e4eb7a878a7db319b725ae262f40671c32
* combine `JobQueueFederated` and `JobQueueRedis` into a single
`JobQueue` channel.
* Remove duplicate descriptions from file blocks in favour of class
doc blocks. This reduces needless duplication and was often
incorrect or outdated, and helps (ironically) to make the file header
more consistently visually ignorable. Various files in this patch
contained bogus copy-pasta descriptions from unrelated classes,
and re-defined `defgroup JobQueue` many times, showing exactly
how this is defacto ignored and counter-productive to maintain
in two places.
Remove `ingroup` from file blocks in class files as otherwise
the file is indexed twice (e.g. in Doxygen) which makes navigation
on doc.wikimedia.org rather messy for classes in this group.
Ref <https://gerrit.wikimedia.org/r/q/message:ingroup+is:merged>
Change-Id: I926a3aec2bc98fefa1075c4a794c46108579ae3f
Make JobSpecification inherit the request ID of the queuing
request just like Job does. This makes it easier to find log
events for actions that are triggered by a web request but happen
via jobs. Especially useful for enqueueable updates which might
or might not happen via jobs depending on the load of the system
at the moment.
Bug: T351729
Change-Id: I965ee211b00b2b33970651368930b5c59704a827
Depends-On: Iccf0f3bf666b9a77fbbd5874fe2f56bfffc9bd4c
Depends-On: I9613c8c293ac3fb4b1ac25c229bb4dc83e2f34fa
* Inappropriate @inheritDoc usage. Arguably all @inheritDoc is
inappropriate but these are the ones PHPStorm flags as misleading
due to the method not being inherited.
* Doc comment type does not match actual argument/return type.
* I replaced "@return void|never" with "@return void" since never means
never, it doesn't make sense for it to be conditional. If a method
can return (even if that is unlikely) then @return contains the type
that it returns. "@return never" means that there is no such type
because the method never returns.
* Incomplete/partial/broken doc tags
Change-Id: Ide86bd6d2b44387f37d234c2b059d6fbc42ec962
Removing newline characters from job parameter values since newlines
indicate a new log line in larger logfiles.
Change-Id: Ia99a9e580ff9e61f16718b79b37b7b0bf1cf639c
This is really hard to read. What is code, what is string? These
places are so simple, they really don't need the "{$var}" syntax.
Change-Id: I589dedb8c0193eec4eef500bbb896b5b790b727b
Part 1, proof of concept. Hundreds of files left to go. These changes
brought to you in large part by vim macros.
Bug: T305805
Change-Id: I44789091e9f6394c800a11b29f22528c8dcacf71
For compliance with the new version of the table interface policy
(T255803).
This patch was created by an automated search & replace operation
on the includes/ directory.
Bug: T257789
Change-Id: Ie32c1b11b3d16ddfc0c83a757327d449ff80b2e4
For compliance with the new version of the table interface policy
(T255803).
This patch was created by an automated search & replace operation
on the includes/ directory.
Bug: T257789
Change-Id: I5ffbb91882ecce2019ab644839eab5e8fb8a1c5f
For compliance with the new version of the table interface policy
(T255803).
This patch was created by an automated search & replace operation
on the includes/ directory.
Bug: T257789
Change-Id: If560596f5e1e0a3da91afc36e656e7c27f040968
PHP doesn't care much but I think we humans do because we should
call methods by the name we give them. Method fixed are;
- isOk() -> isOK()
- setOk() -> setOK()
- teardown() -> tearDown()
Change-Id: I6b3f0cf3902887058efa426968da380803869e0b
The Job::factory method throws an InvalidArgumentException if a given
job specification is invalid. The error message for this exception was
worded in correctly. This patch improves the message to avoid confusion.
Change-Id: I6cab739263c1d3530c0650823db022dd5a4b60bc
* Remove duplicate $params check from Job::factory done in Job::__construct.
* In Job::factory(), restore use of a valid title as default for passing as
constructor arg to old job classes. Their constructor may expect it to
be valid.
Keep the invalid dummy in Job::__construct, and document why.
* tests: Update test case for failure mode when using Job::factory
with a class that requires a title. It asserted getting an invalid
title. This now restores the behaviour prior to fc5d51f129,
which is that job classes that require a title, get a valid one.
* tests: Remove test case for testToString that used
an explicitly passed but invalid params value. I've converted
that to expect the exception we now throw instead.
* tests: Update getMockJob(), also used by testToString, which was
relying on undocumented behaviour that 'new Title' is public
and gets namespace=0 and title=''. Before fc5d51f129,
title params weren't in toString() and it asserted outputting
three spaces (delimiter, empty string from formatted title,
delimiter).
In fc5d51f129, this changed to asserting "Special:" which
seems unintentional as we didn't pass it the internally reserved
NS_SPECIAL/'' value, and yet was caught by the dbkey=='' check.
Given this test case doesn't deal with titles, omit it for now.
A job can either have a $title and title/namespace in params,
or neither. This test was asserting an in-memory scenario
where $title can be an object, but title/namespace absent from
params.
Bug: T221368
Depends-On: I89f6ad6967d6f82d87a62c15c0dded901c51b714
Change-Id: I2ec99a12ecc627359a2aae5153d5d7c54156ff46
Simplify the code of jobs that do not care about titles and removes
the direct Title dependency from JobQueue. Remove getTitle() from
IJobSpecification itself. Move all the Job::factory calls into a
single JobQueue::factoryJob() method.
Depends-on: Iee78f4baeca0c0b4d6db073f2fbcc56855114ab0
Change-Id: I9c9d0726d4066bb0aa937665847ad6042ade13ec
Follows-up 9b4938c40d, 4bff6f1558, and 766549999c.
It seems despite being optional and documented as 'array|Title',
Phan still thinks it is wrong to pass Title.
Try removing the explicit default of empty array,
the lower code already does this anyway.
This should fix build failures that are preventing merges
in repos where Phan is finding Job::__construct(string, Title)
where it currently fails as follows:
CompileArticleMetadataJob.php:11
PhanTypeMismatchArgument Argument 2 (params) is …\Title|string but
\Job::__construct() takes array
Change-Id: I1c3aee273e0917e30b9c18b49b24fc7f966ff57c
Follows-up 9b4938c40d, and 4bff6f1558. The latter of which
updated ::factory() instead of __construct(). Oops.
This should fix build failures that are preventing merges
in repos where Phan is finding Job::__construct(string, Title)
where it currently fails as follows:
CompileArticleMetadataJob.php:11
PhanTypeMismatchArgument Argument 2 (params) is …\Title|string but
\Job::__construct() takes array
As Phan is reading the php doc I guess.
Change-Id: I128d57ead6bcb9dbae99d41a1f23192c3f6fbdba
Follows-up 9b4938c40d.
This should fix build failures that are preventing merges
in repos where Phan is finding Job::__construct(string, Title)
where it currently fails as follows:
CompileArticleMetadataJob.php:11
PhanTypeMismatchArgument Argument 2 (params) is …\Title|string but
\Job::__construct() takes array
As Phan is reading the php doc I guess.
Change-Id: I94b442f06b6858d136546bf22f2465cf8f071ab0
Remove the $title argument from these methods to simplify subclasses that
do not have a meaningful title to use. The Job::getTitle() method can be
overriden by subclasses to return something meaningful.
The old call signature is still supported for backwards compatibility.
This will automatically determine what getTitle() returns as before.
Use "Blankpage" as the "not applicable" title for jobs instead of one
that looks like some error occured.
Change-Id: I3d5bd012d9cef1e7daaccfb0d5d319552eb89fb6
Find: /isset\(\s*([^()]+?)\s*\)\s*\?\s*\1\s*:\s*/
Replace with: '\1 ?? '
(Everywhere except includes/PHPVersionCheck.php)
(Then, manually fix some line length and indentation issues)
Then manually reviewed the replacements for cases where confusing
operator precedence would result in incorrect results
(fixing those in I478db046a1cc162c6767003ce45c9b56270f3372).
Change-Id: I33b421c8cb11cdd4ce896488c9ff5313f03a38cf
The replaces the hacky use of onTransactionIdle(), which no longer runs
immediately in explicit transaction rounds since d4c31cf841.
Also clarified TransactionRoundDefiningUpdate comment about rounds.
Change-Id: Ie17eacdcaea4e47019cc94e1c7beed9d7fec5cf2
$wgJobClasses can now specify a factory function for creating a job,
instead of a class to be instantiated directly. This makes it possible
to inject services in a job constructor, and register a factory function
that calls the constructor with default services.
This follows Ieb85493a7765 and Ia2107dc5af78, which introduced factory
functions for API modules and special pages.
Change-Id: I0461e59da2a8fa6681e3b1fcdfc38bfed7f3ac32
We currently push a request id into structured logging (monolog/
logstash) to allow seeing all logs that were triggered by the same
request. This extends that to pass the id through jobs so jobs triggered
by a web request also share the same id and can be tracked together.
This web request id will follow jobs both directly created by a request,
and jobs created by those jobs.
This should give us some more visibility when debugging into what
started a particular job, and if a large number of jobs blowing up the
job queue are somehow related.
Change-Id: Iedbd031e6e9bb18fd6f7b923c8c305102255ab4b
