Doesn't cost anything to compute and include in logs, and would be
useful for understanding serial processing of DeferredUpdates
(basically, non-deferred deferred updates) in CI are slowing down
browser tests.
Bug: T225968
Change-Id: Id55bde5d8f4e465723d8db6df29b4c554fc1bc9c
This logic is not needed to run on every PHP process and was making
it difficult to run offline maintenance scripts without additional
complexity based on Maintenance::getDbType and DB_NONE.
Instead of skipping this only for DB_NONE, and establishing a pattern
that may spread to other ad-hoc places throughout the codebase, instead
remove this entirely from the eager set up code for all PHP processes
and move it to the service wiring and dependency injection.
That way, it naturally doesn't happen until and unless the DB service
is actually called upon. Scripts and entry point that need to disable
the DB service, can continue to use
MediaWikiServices::disableStorageBackend.
== Impact on SiteStatsUpdate ==
With wgCommandLineMode no longer being read at run-time from a global,
but in service wiring, this means SiteStatsUpdateTest can't change
the behaviour between CLI-like and Web-like, unless it e.g. resets
the 'DBLoadBalancerFactory' service first. Unfortunately, while most
any reset is supported, a reset of the 'DBLoadBalancerFactory' would
be unsupported as that would lose the temporary db clone context and
such, bringing us to either the developer's live db, or a broken set
up altogether. If there is a strong need for toggling oppertunistic
updates off and on at run-time, this could be supported in the
DeferredUpdates class perhaps, but we already have numerous methods
there (incl db begin/commit being a good proxy already), which this
test already used, so for now I've just removed the extra assertion
for this as it wasn't essential to that test.
Bug: T228895
Bug: T238436
Change-Id: Icf29bc484c155f52b6d8f61e5902233a15ba0c6d
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
Follow-up ba6490aa1e.
That patch worked around $executeContext iteration errors due to the
lack of recursion support in doUpdates()/handleUpdateQueue(). Errors
were triggered by MediaWiki::schedulePostSendJobs() enqueueing deferred
updates that invoke JobRunner::run(), which, in turn, invoke doUpdates()
for each job via JobRunner:: doExecuteJob(). The doUpdates() method was
changed to operate on the sub-queue for the in-progress DeferrableUpdate.
Further improve the recursion logic:
* Use classes for the scope stack and scope queues.
* Put *all* updates added during a DeferrableUpdate::doUpdate() call into
the subqueue, regardless of "defer until" stage or whether it implements
MergeableUpdate. Now, doUpdate() can run *everything* it enqueued instead
of a non-obvious subset. Note that the TransactionRoundDefiningUpdate
that invokes JobRunner was already a POSTSEND update before ba6490aa1eb;
the only effect of this change is that MergeableUpdate instances from jobs
will once again run in doExecuteJob().
* Make recursive DeferredUpdates::doUpdate() calls error out immediately
unless the DeferrableUpdate responsible is a TransactionRoundAwareUpdate
with the TRX_ROUND_ABSENT flag. This covers the schedulePostSendJobs()
scenario and only prohibits insane call patterns. Failing early avoids
the risk of handleUpdateQueue() dropping all the updates due to the same
DBTransactionError error in DeferredUpdates::attemptUpdate().
* Avoid recursion loop in tryOpportunisticExecute() when JobQueueDB is in
use and a large number of tasks are pending. This happened due to methods
like onTransactionPreCommitOrIdle() being used within JobQueueDB.
Mark DeferredUpdates::doUpdates()/tryOpportunisticExecute() as @internal
and create a Maintenance::shutdown() method to avoid a direct call in the
doMaintenance.php file.
Bug: T249069
Bug: T268840
Change-Id: Ib369f0e74243a48ababdb9cd83b155c9a0f5e741
PHP 7.0 makes many error conditions throw instances of the new Error class
which does not extend the known Exception.
The Throwable interface provides a concise and type-safe way of handling
either, e.g. for logging purposes, but HHVM did not support it, requiring
tedious fallback checks.
This commit replaces occurrences of Exception in code paths equally
covered by Throwable, like Exception|Throwable parameter and return types
(also nullable), instanceof guards, duplicated `catch` blocks, as well as
related comments and documentation blocks, with the exception of $previous
parameter descriptions consistent with the manual at
https://www.php.net/manual/en/exception.construct.php
Proper type declarations have been added or reinstated where possible.
Change-Id: I5d3920d3cc66936a350314e2f19c4f6faeffd7c0
This way most existing Logstash dashboards looking for traces
can pick this up without requiring the filter to be duplicated
or written as a custom Query DSL entry.
Also:
* Fix the broken logger from the jobify() method which did not
define the "{type}" or "{exception_message}" keys at all.
* Fix the over-normalized logging of all fatal errors from deferred
updates as "Error from {type}: {exception}" which isn't useful to
aggregate (we can already get an overall count for
channel:DeferredUpdates, level:error). The normalized aggregate
is meant to uniquely identify specific errors that have a common
cause. For other fatal exceptions we leave the keep the cause
and exception message in the raw message and only add the request
context and trace into placeholder fields (MWExceptionHandler.php).
Apply the same principal here as well.
* Field 'exception.class' will now be available. This was previously
missing (e.g. what exception is being thrown). It would log
"The given Title does not belong" instead of
"InvalidArgumentException: The given Title…".
Bug: T233342
Change-Id: I9b15658c97e3bfcc2ce8b234d1c0d47c9c294fb7
Bail out in attemptUpdate() if the transaction state is dirty rather
that failing at some later point. Also, flush implicit transaction
rounds before calling DeferrableUpdate::doUpdate() for fresher data.
Note that only instances of EnqueueableDataUpdate can become jobs.
Make handleUpdateQueue() defer throwing the exception until every task
was attempted for the special unit test logic.
Clean up some of the logging from 34427e7d7b.
Bug: T206283
Change-Id: I84ba1f2f8c4bf7c8ef21a907f73ad1065dd8f330
Some of the errors are suppressed because they're phan false positives.
The idea behind this is that they'll be fixed in a future version of
phan, and we'll just have to remove the suppressions.
Note: I'm disabling UnusedPluginSuppression so that we can start suppressing
issues even if they're still disabled. The sniff should be re-enabled
as soon as we upgrade phan.
Bug: T231636
Change-Id: I0f7fa06a9e03fbb86c7a5eb6e50a850bb258a7f7
Bail out in attemptUpdate() if the transaction state is dirty rather
that failing at some later point. Also, flush implicit transaction
rounds before calling DeferrableUpdate::doUpdate() for fresher data.
Bug: T225103
Change-Id: I4f5d2f9814a562069619f05e003663fcedbd3f64
Other cleanups and fixes:
* Split up handleUpdate() method into run() and jobify()
* Handle Throwable errors
* Use 'cli' in stats keys in CLI mode instead of "get"
* Tweak some code comments
Change-Id: I7749465df2d7b58e66ee5ebdd3c3d25aea52eeb3
Refactor the deferred update transaction round handling code to use
a new TransactionRoundAwareUpdate interface. Also, rename a few
DeferredUpdates methods so they do not give the impression that
doUpdates() is always called.
Change-Id: Idc4c6d81c4e2ca0ce41bca1e7800f797fa7e37f6
This is a stop-gap solution for the problem described in T190178.
A better solution would probably include changing the behavior
of LegacyLogger.
Bug: T190178
Change-Id: I433be04b8ee725becd174e567270aa674d2661df
This allows scheduling of updates that need to start their own
transaction round. Specifically for cases where the ability to
commit early is not enough (which is already possible via LBFactory
getEmptyTransactionTicket and commitAndWaitForReplication).
Change-Id: I0910587b61c8ddf825f91e92c2f93582cc7ebd80
This avoids putting updates in the PRESEND queue at a point where they
may never get run later in the request. The peculiarity lead to a
regression in 24842cfac.
Move "enqueue" logic to runUpdate() to simplify execute(). If job
insertion batching is strongly desired for a class, then it can use
MergeableUpdate.
Removed unused "update" field in $executeContext.
Bug: T168723
Change-Id: I40d16f6cd0adc8583797b99d859b76a836d362a8
Previously, tryOpportunisticExecute() tried to nest transaction rounds,
which would fail. Added LBFactory::hasTransactionRound() as needed.
Also cleaned up some unqualified class names in callbacks and set the
PRESEND flag for the JobQueueDB AutoCommitUpdate callback. Use the
proper getMasterDB() method while at it. These follow up 24842cfac.
Bug: T154425
Change-Id: Ib1d38f68bd217903d1a7d46fb15b7d7d9620daa6
In the postprocessing, some jobs can be executed but given the deferred
updates were already "closed", any new DeferredUpdate were directly called
(as explained by Krinkle on T165714), and the transactions opened by
classical jobs are badly mixed with transactions (directly) executed by
DeferredUpdates jobs, issuing a DBError, avoiding the job, which stays
in a 'claimed' status even if failed.
Quite similarly, some DeferredUpdates callables use JobQueueGroup::lazyPush
so it is needed to really push the generated jobs.
This change removes the run-immediately-deferred-updates behaviour even
in the post-connection shutdown, and given there is a call to
DeferredUpdates::doUpdates in JobRunner::execute it is not necessary to
add another one and hence execution of Web jobs is more similar to execution
of CLI jobs. In the same spirit to reconcile Web jobs and CLI jobs, the
call to JobQueueGroup::pushLazyJobs is done in JobRunner::execute.
Bug: T165714
Bug: T100085
Change-Id: I721e7167eca5b0b6227234fe516005243ab22388
Updates are stored in private variables which is not convenient when
testing deferred updates. Add a getPendingUpdates() accessor. The stage
can be specified to only retrieve PRE or POST ones.
Change-Id: I1af730ec5e48bc0be555a8db4611a76eb9332444
This should lower the rate of "does not have outer scope" log
warnings in DBPerformance.
Also add traces to such logs.
Change-Id: I7d21ea745cae07e0fbbe4cd8de82e93f1d10e0a5
