* 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
This config has been set to false in production since 2018 (Ie4ea1dc0d3927).
This doesn't provide much benefit and its impact yet to be proven and
it's blocking removal of ILoadBalancer::getAnyOpenConnection()
That removal makes $dbwSerial unconditionally false which turned the
whole method into a one-liner and since it was used only once,
we just replaced that.
Bug: T325389
Depends-On: I24f33a15d214d49bfb6c6013e7fd64c7d0eb0086
Change-Id: I19043e775fce9f7ddded128cb2c9395b46dd19a3
After deploying T320559 we found out that JobRunner sometimes calls
the WebRequest::overrideRequestId() with a null. This caused the
TelemetryService to fail due to $newId not being a string.
As a solution there is a new method called regenerateRequestId()
that will handle the regeneration as it's recommended to keep the
overrideRequestId() strict.
Bug: T343893
Change-Id: I3e6daa238fd8de0d9ea88660b8ba32b922b1323d
The db/ directory does not have an owner and it's a mess in general.
These classes don't depend on anything in core except the rdbms library.
Let's simply move it there. In other words, Krinkle made me do it.
Since the class was moved in I6202e52ba73 merged less than a week ago,
no need to alias anything.
Bug: T321882
Change-Id: I24ceeb8bf765a50f441270136acd612359d50aa2
They are not suitable to go to rdbms library as they depend on mediawiki
pieces, the second best place is the db/ directory.
Bug: T321882
Change-Id: I6202e52ba7306d74261206c2ba7930c5f1a0a18e
These options only support one value in the array: maxWriteDuration
Instead of turning this into a full array and pass it around, just pass
the integer to simplify the logic, avoid mistakes by typos and more.
Bug: T326274
Depends-On: Ib5c76346d0a61c3a6906365b3ced9fca2d43e4d2
Change-Id: Ib60f25ba4a7ca1d14d062d9121fe34e94ccc3b70
There is a service for this class. Use the service instead:
MediaWikiServices::getInstance()->getJobRunner()
Depends-On: Icefb296201046cf77e42270199cde76ee76668b5
Change-Id: Idce18bf1528c04bc1339c695f0a3a02c093451cd
This edition brought to you by:
grep -ERIn $(grep -o "'[A-Za-z0-9_]*'" includes/MainConfigNames.php | tr
"\n" '|' | sed 's/|$/\n/') includes/
I only corrected a fraction of the results provided by that command. I'm
submitting the partial patch now so it doesn't bitrot.
Bug: T305805
Change-Id: If1918c0b3d88cdf90403921e4310740e206d6962
* Some functions accept only string, cast ints and floats to string
* After preg_matches or explode() casts numbers to int to do maths
* Cast unix timestamps to int to do maths
* Cast return values from timestamp format function to int
* Cast bitwise operator to bool when needed as bool
* php internal functions like floor/round/ceil documented to return
float, most cases the result is used as int, added casts
Found by phan strict checks
Change-Id: Icb2de32107f43817acc45fe296fb77acf65c1786
Connection loss with a failure to reconnect led to a variety of failure
modes, such as an assertion from getBindingHandle() when addQuotes() is
next called. And Maintenance::shutdown() was calling
commitPrimaryChanges() three times, each call leading to an assertion
due to the round stage being broken.
So, try to exit promptly if a DBConnectionError is caught, by
introducing a new "reached" category for fatal exceptions. Guard the
calls to commitPrimaryChanges(), and skip the call prior to shutdown()
since shutdown() calls it already.
Change-Id: I81ee9017a58adac674a9495802f6bd4bcc22ee61
When the called function has a doc of int, it should be cast to be
explicit here.
Also cast for arithmetic operations to be explicit about the number
Change-Id: I905b78dfb66e66443e0e3203488bab5b548db543
This is micro-optimization of closure code to avoid binding the closure
to $this where it is not needed.
Created by I25a17fb22b6b669e817317a0f45051ae9c608208
Change-Id: I0ffc6200f6c6693d78a3151cb8cea7dce7c21653
This patch touches all uncontroversial (I hope) places where a chain
of isset(), array_key_exist() and the ternary ?: operator can be
replaced with the much shorter ?? feature from PHP 7.
?? does the same. It checks if the element before the ?? is set and
not null. When this check fails, the element after the ?? is used.
Change-Id: Id612e2782ae928164b26b6f0de676c6c7d8302f3
These were never meant to be part of the public interface and should not
ever have been marked with @since. They're only useful for constructing
the respective objects, which no outside users should be doing.
Change-Id: I86e01272d46fc72af32172d8a12b9180971d4613
Consolidate more logic into JobRunner::execute() and make
it public. Add "caught" field to the resulting map. The
intended use case for this method is JobExecutor. Calling
this method from there could cut down on code duplication.
Also:
* Use try/finally to restore state instead of ScopedCallback.
* Use more generic Throwable instead of Exception.
* Reorganize JobRunner::run() slightly for readability.
* Set class constant visibility and improve code comments.
Bug: T243492
Change-Id: I90566a49c603aa78f45b35c0d3fc1925d2cfe2f8
Repeating the variable name doesn't do anything. Documentation
generators don't need it. It's more stuff to read that doesn't add new
information. And it can become outdated.
Note there are two types of @var docs. When used inline (and not on a
class property) the variable name is needed.
Change-Id: If5a520405efacd8cefd90b878c999b842b91ac61
Use this in JobRunner to avoid overly sensitive lag timeouts and
log spam. The 3 second timeout is between the regular web default
and the CLI default.
Follow-up to e8df0fbab1.
Bug: T235244
Change-Id: I92f657a638031d913b0575d74bf48c3e3a63cd17
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
If JobRunner is called when replica transactions exists, the first job
would previously use the stale REPEATABLE-READ snapshot data.
Also clear any master connection snapshots via commitMasterChanges().
This makes the code more similar to DeferredUpdates::attemptUpdate().
Change-Id: I2157a91fb01ea8c233f964b1f3164e8c3b1a07ca
JobQueueGroup is giving RunnableJob on pop(), so it should take the same
type for ack() and deduplicateRootJob()
JobQueue::ack alsready accept the interface
Also change to RunnableJob in JobRunner to work with the type from the
job queue
Change-Id: I7b09586cff8affabe807ee16e80d04f5137dce45
This is slightly more robust and makes the intent much clearer
than random calling code checking getServerCount() all over the
place. In addition, this yields better separation of concern.
Also, cleanup the LoadBalancer constructer a bit and make the
validation a bit stricter.
Make some server index comparisons strict while at it.
Change-Id: Icc1a35bd65c6862ff81faa3ab9b2aa7cafe29443
This assures that MergeableUpdate tasks that lazy push job will actually
have those jobs run instead of being added after the lone callback update
to call JobQueueGroup::pushLazyJobs() already ran.
This also makes it more obvious that push will happen, since a mergeable
update is added each time lazyPush() is called and a job is buffered,
rather than rely on some magic callback enqueued into DeferredUpdates at
just the right point in multiple entry points.
Bug: T207809
Change-Id: I13382ef4a17a9ba0fd3f9964b8c62f564e47e42d
Use the given fname for all places.
The __METHOD__ inside the unlock closure would be shown as {closure} in
logs
Change-Id: I87ef26e893af858f58d1a77dcb2d8ee192456f5c
For maintenance scripts it is usually harmful to throw an exception.
For jobs the exception was already caught and handled appropriately,
so this can continue as before. For DeferredUpdates it was extremely
harmful to throw an exception. So in the web case, reduce the timeout to
1s and continue as normal if the 1s timeout is reached. This allows the
DeferredUpdate to be throttled without being killed.
In the updater, increase the replication wait timeout to 5 minutes.
ALTER TABLE could indeed cause replication lag, but exiting the update
script with an exception will probably ruin your day. Update actions are
not necessarily efficiently restartable.
Do not call JobQueue::waitForBackups() when jobs are popped. Maybe it
makes sense to call a queue-specific replication wait function for
bulk inserts, like copyJobQueue.php, but doing it when jobs are popped
just makes no sense. Surely the worst that could happen is that the
queue would become locally empty? Removing this waitForBackups() call
avoids waiting for replication twice when JobQueueDB is used.
Bug: T201482
Change-Id: Ia820196caccf9c95007aea12175faf809800f084
