* Document that Maintenance::shutdown is the CLI equivalent of
MediaWiki::restInPeace.
* Centrally document in the emitStats method why we flush stats
regularly, and clarify that these OOM concerns are specific to
CLI processes there. That isn't to say it could never happen on a
web request, but all our early flush handling (even DB trx hooks)
are explicitly limited to command-line mode today and always have
been.
* Ref T253547. It is now clear why --profiler=text doesn't work on
the CLI (it is missing the non-external profiler output handling),
which I'll fix in a follow-up.
* Ref T292269. The WebRequest-dependency in Profiler is now much more
clearly problematic. Previously this was masked by wfLogProfilingData
effectively silencing the warning on the CLI without it being so
obviously wrong. I'll fix that in a follow-up.
* Ref T292253. All this is already post-send, and flame graphs confirm
that we don't have any calls to emitBufferedStatsdData nor
StatsdClient::send apart from the post-send one via restInPeace.
Bug: T253547
Bug: T292269
Bug: T292253
Change-Id: If78c37046cf8651c7a8d6690e01d38c3ca29d8d8
This helps phan to detect unreachable code and also impossible types
after the functions.
It helps phan to avoid false positives for array keys
when the keys are checked before
Bug: T240141
Change-Id: I895f70e82b3053a46cd44135b15437e6f82a07b2
History:
- Added in I239572f75bebbc9c731a3e3860c4eff179dc15e4 with
its only use being in the PreMcrSchemaOverride trait in tests
- I16bea8927566a3c73c07e4f4afb3537e05aa04a5 moved the
PreMcrSchemaOverride trait from tests/phpunit/includes/Storage/
to under tests/phpunit/includes/Revision/
- I2ae4e82a12f730617563eb6f911fc406b9195391 removed the
PreMcrSchemaOverride trait, which I believe left PatchFileLocation
unused
Added to release notes as a breaking change since it predated
the stable interface policy, just in case anyone was using it.
Change-Id: If2020688cb6ee24ed4ee56a10b52bdf61e7d8455
MWLBFactory tries to determine the current user's name using
posix_getpwuid(). This will however fail if the current user has no
name, which is the case when running in our standard docker environment.
Bug: T283191
Change-Id: If814d791ee7061d8632ae04602cf489b06422eee
This means we'll consider a slightly more ChronologyProtector cookies
that are near their expiry. This is perhaps not so useful since that's
also near the time where max replication lag kicks in, and the storing
of the CP data itself is going to be evicted.
But, this may be a more predictable and stable reference point than
"current" time, considering that as of I3140922cec130cad, this
computation now happens lazily at the first call to LBFactory, instead
of during Setup.php, so in a way this change just brings it back to be
closer to the start of the request (except even earlier than it was
before I3140922cec130cad, since before that commit we did wait until
after the first few ~ milliseconds of multiversion, autoloader,
composer, etc.).
Bug: T228895
Change-Id: I76e2130431ab44344eea93ce8f4f8b2322840819
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
Avoid costly and unconditional set up of LBFactory early on in
Setup.php. Instead, inject this information lazily if and when the
service is contructed by something that actually needs a database
connection.
This removes the cost of LBFactory service + dependencies for requests
to load.php, api.php, and rest.php requests that involve no (other)
calls to the database service.
Bug: T228895
Change-Id: I3140922cec130cad84eb89b2b34520c0988027dc
Postgres:
- Rename the table from `mwuser` to `user`
- Transition user_id from int to serial
- Make user_token not nullable and add default
- Make user_real_name not nullable and add default
- Make user_email not nullable
- Make user_newpassword not nullable
- Make user_password not nullable
- Drop UNIQUE contraint on user_name and add default
MySQL/SQLite:
* No changes
Bug: T164898
Bug: T230428
Change-Id: I746714f7b3ae16f9625f97bcca84280fcd8b61a0
One less thing to inject and pass down, one less thing where MW-specific
wiring varies from the default.
Bug: T228895
Change-Id: I1bb181a0b9b081050e544eb4b2a6c4287ee89528
This checks that the Database state has not diverged from the driver DB
handle nor server-side connection state due to an exception being thrown
in an unexpected place within internal Database methods. DB handles with
possible state corruption will not accept queries.
For example, a PHP extension like Excimer might be used to throw request
timeout exceptions. Such exceptions can trigger after any PHP or Zend
function returns, e.g. within DatabaseMysqlBase::doSelectDomain() after
the "USE" query completes but before $this->currentDomain gets updated.
Also:
* Make getApproximateLagStatus() catch getLag() errors since begin()
expects it to simply use "false" for the lag value on failure. This
helps assure that $this->trxAutomatic gets properly set.
* Unsuppress exceptions in runOnTransactionPreCommitCallbacks()
as the transaction needs to get aborted anyway (as already happens).
* Unsuppress exceptions in runOnAtomicSectionCancelCallbacks() since
the safest thing to do is just roll back the transaction.
* Only suppress DBError exceptions in runOnTransactionIdleCallbacks().
and runTransactionListenerCallbacks(). Return the array of errors
rather than throw the first one. Most of the callers had to catch
the errors, so it's easier to avoid throwing them to begin with.
* Avoid blanket try/catch in sourceStream(), doReplace(), upsert(),
doInsertSelectGenericand().
* Clarify various code comments and add missing @internal tags.
Bug: T193565
Change-Id: I6b7b02c02b24c2ff01094af3df54c989fe504af7
Remove WRITE_SYNC flag from ChronologyProtector since the current
plan is to simply use a datacenter-local storage cluster.
Move the touched timestamps into the same stash key that holds the
replication positions. Update the ChronologyProtector::getTouched()
comments.
Also:
* Use $wgMainCacheType as a $wgChronologyProtectorStash fallback
since the main stash will be 'db-replicated' for most sites.
* Remove HashBagOStuff default for position store since that can
result in timeouts waiting on a write position index to appear
since the data does not actually persist accress requests.
* Rename ChronologyProtector::saveSessionReplicationPosition()
since it does not actually save replication positions to storage.
* Make ChronologyProtector::getTouched() check the "enabled" field.
* Allow mocking the current time in ChronologyProtector.
* Mark some internal methods with @internal.
* Migrate various comments from $wgMainStash to BagOStuff.
* Update some other ObjectCache related comments.
Bug: T254634
Change-Id: I0456f5d40a558122a1b50baf4ab400c5cf0b623d
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
It was suggested this header be renamed to be consistent with the
infrastructure header added in T91820. We have to explicitly
allow the REST API to use this header in the WebStart sanity check.
Bug: T91820
Bug: T259685
Followup-To: b75ac3953e
Change-Id: I0c4ec63bb26641b237c92dbd3bc5367811ca0675
This is a follow up to T93097, which worked around a limitation of
SQLite 3.8+ which prevents it from upgrading a read transaction to a write
transaction. Our heuristic is that HTTP POST requests are going to
eventually become DB writes, and so we take the SQLite lock early.
However, Parsoid POST requests don't have any side effects on the DB,
and taking the write lock causes deadlocks during VE saves: the POST
to action=visualeditoredit conflicts with the POST to the recursive
request to the Parsoid REST API. We can't use a GET request for
these requests without hitting query-length limits.
This patch allows REST API calls to set the `X-MediaWiki-Read` header
to opt-out of the SQLite obligatory lock. This avoids the deadlock,
while still allowing the API call to use a POST and avoid query length
limits.
Bug: T259685
Change-Id: If37dc890a24a45c3a914e310b5b5bf625965e9e6
For MySQL/Sqlite, drop the table options, they are for MySQL < 5.0 which
was released in 2003 and not any use today.
For Postgres, rename pagecontent table to text, approved as part of the
RFC. Swap order of creating auto-generated tables and manual tables.
Bug: T230428
Bug: T164898
Change-Id: Id7510b80beed7a7297353094a57d5e2d4f12fc64
For example, documenting the method getUser() with "get the User
object" does not add any information that's not already there.
But I have to read the text first to understand that it doesn't
document anything that's not already obvious from the code.
Some of this is from a time when we had a PHPCS sniff that was
complaining when a line like `@param User $user` doesn't end
with some descriptive text. Some users started adding text like
`@param User $user The User` back then. Let's please remove
this.
Change-Id: I0ea8d051bc732466c73940de9259f87ffb86ce7a
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
* Convert to a single map.
* Simplify source code.
* Remove deprecation warning for something we're not likely to
remove. This was embedded into LocalSettings.php files and
should generally just keep working and is easy and cheap to do.
Also note clear why one half warned and the other half didn't
(because that's the one we happen to use in prod.)
* Simplify the tests. A lot of the boilerplate was no longer needed.
* Reduce abstraction in the test as was was not more complex
than the source it tests.
Change-Id: If3e7e25dbf3bb408581fc16ac8e556b44b1855ad
Deprecating something means to say something nasty about it, or to draw
its character into question. For example, "this function is lazy and good
for nothing". Deprecatory remarks by a developer are generally taken as a
warning that violence will soon be done against the function in question.
Other developers are thus warned to avoid associating with the deprecated
function.
However, since wfDeprecated() was introduced, it has become obvious that
the targets of deprecation are not limited to functions. Developers can
deprecate literally anything: a parameter, a return value, a file
format, Mondays, the concept of being, etc. wfDeprecated() requires
every deprecatory statement to begin with "use of", leading to some
awkward sentences. For example, one might say: "Use of your mouth to
cough without it being covered by your arm is deprecated since 2020."
So, introduce wfDeprecatedMsg(), which allows deprecation messages to be
specified in plain text, with the caller description being optionally
appended. Migrate incorrect or gramatically awkward uses of wfDeprecated()
to wfDeprecatedMsg().
Change-Id: Ib3dd2fe37677d98425d0f3692db5c9e988943ae8
When the left side of ?? is wrapped with [] the array has always one
item and that means it is always true and then used
Move the use of the array key into the existing isset
The empty array for $serversCheck is the default
Change-Id: I2ec95026b3cd5a72dcaf540f369b140fd7b69a53
I was having a problem writing an integration test for
I3801b4d1f4a876a820c28f4541ec0802d886b7eb as part of T254283.
After much debugging I realized that 2 different db domains were being
used by the tests, resulting in inconsistent queries.
This is partly due to some recent changes in Wikbase code that no longer
always passes false around for the local domain, which was T252019
Ie8822ebd3661a21cbe475f75cae2615cc2b206c6
I35fa856b0b0591e814ddbfda2fed04322c6dd63f
When unit tests use a test prefix, the code in resolveDomainInstance no
longer resolves the db name string to the correct domain (with the
prefix) instead using the orig domain (with no test prefix).
This is due to the $domain === $this->localDomain->getId() check as for
a db name of "default" the equiv string id is now "default-unittest_"
This patch adds aliases to LB and LBF for the domain when a prefix is
used.
Aaron should probably confirm if this is the best place for this code to
live? Or if it should actually live in the test base class?
Bug: T254283
Bug: T252019
Change-Id: I4615c32cefc2e1af685137cb96c37e3813bdce75
A terminating line break has not been required in wfDebug() since 2014,
however no migration was done. Some of these line breaks found their way
into LoggerInterface::debug() calls, where they mess up the formatting
of the debug log.
So, remove terminating line breaks from wfDebug() and
LoggerInterface::debug() calls.
Also:
* Fix the stripping of leading line breaks from the log header emitted
by Setup.php. This feature, accidentally broken in 2014, allows
requests to be distinguished in the log file.
* Avoid using the global variable $self.
* Move the logging of the client IP back to Setup.php. It was moved to
WebRequest in the hopes that it would not always be needed, however
$wgRequest->getIP() is now called unconditionally a few lines up in
Setup.php. This means that it is put in its proper place after the
"start request" message.
* Wrap the log header code in a closure so that variables like $name do
not leak into global scope.
* In Linker.php, remove a few instances of an unnecessary second
parameter to wfDebug().
Change-Id: I96651d3044a95b9d210b51cb8368edc76bebbb9e
Clean up the recursive DB dependency mitigation logic by having
ServiceContainer detect recursion and throw an appropriate error.
Catch the error and use EmptyBagOStuff in such cases. This works
better than checking getQoS() since that begs the question by
requiring the cache instance to begin with.
Also add support for using different LoadBalancer instances for
local and global keys in SqlBagOStuff. This makes it easier to
share keys between projects.
Bug: T229062
Change-Id: Ib8ec1845bcf1b86cbb3bededa0ca7621a0ca293a
* Split MWDebug::sendRawDeprecated() from MWDebug::deprecated(). The new
function can be used to send arbitrary messages to the deprecation
log, rather than being constrained by the fixed format of
MWDebug::deprecated().
* Split formatCallerDescription() from sendMessage() to allow the caller
of sendRawDeprecated() to do its own caller formatting.
* Use the new function in MWLBFactory::logDeprecation()
* In tests, replace the ugly implementation of hideDeprecated() with one
that works by setting a list of regexes to filter. hideDeprecated()
now filters deprecation warnings that are a string match to the
supplied function. filterDeprecated() can be used to filter a regex,
and is intended to be used to filter warnings sent via
sendRawDeprecated(). The filter list is reset at the start of each
test, instead of leaking across tests as before.
Change-Id: I0d0df86db2e61cdd1769426bfa7bad4c2ae5e977
If $wgDebugLogFile was not set or empty and $wgDebugDumpSql
was set to false, the query tab would be empty.
Setting DBO_DEBUG whenever the toolbar is shown to enable query
logging.
Bug: T241516
Change-Id: I0c4f6c1a49341929b7ecad749a02c541daaa238a
Add new ILBFactory::setDomainAliases() method for injection database
domain aliases and call it in MWLBFactory::setDomainAliases().
Also:
* Remove overkill "last db/section" caching in LBFactoryMulti
* Clean up some LBFactoryMulti code comments
* Split out separate MWLBFactoryTest test file
Change-Id: If180a58c61178969ca7587c4a06b8786574c7254
This broke after e0cc49ce39, due to the field 'master'
being removed from the log context. The LegacyLogger logic
forwarding these messages to MWDebug (for the debug toolbar)
however, was dependant on.
Users of debug toolbar experienced a silent failure because the
logic in question is very tolerant of missing fields. This is
because it uses those fields to distinguish the 'sql' messages
from channel=DBQuery from other messages in the same channel.
Making that less fragile is outside the scope of this commit.
This commit:
* Restore the basic functionality by making sure MWDebug::query()
gets called again for DBQuery messages.
* Remove the code relating to the 'master' field as this no longer
exists in RDBMS. It also wasn't used anywhere (to be used,
it would need to be read by mediawiki.debug/debug.js).
* Remove unexpanded "{method}" and "{runtime}" noise in the debug
toolbar text. This was introduced by he conversion to PSR-3
logging.. These fields are already rendered separately by
the toolbar and should not be part of the "SQL" column.
To do this, we need to log the $sql bit as its own key, so
I've made this a context field as well.
* Reduce the condition logic in LegacyLogger to only looking for
'DBQuery' and 'sql'. This way, if it breaks again it will
still call the logic within and emit E_NOTICE instead, which
would help detect the issue (and still fallback to at least
showing the queries). Unlike before this commit where it took
quite some time to figure out why it wasn't working.
* The above fixes still weren't enough to get queries to show
up in the Debug toolbar for me. Turns out, this was because
my local setup (mediawiki-docker-dev) uses a master-replica
set up. The setup doesn't use any custom LBFactory config,
just plain $wgDBservers. The logic for turning these plain
settings into LBFactory (in MWLBFactory.php) does kick in,
and does run (unlike if I had custom wgLBFactoryConf).
But, the DBO_DEBUG flag didn't make it through because of
the += operator preferring any pre-existing value my setup
has, which is just `DBO_DEFAULT`.
Merging 'flags' keys seems unsafe in general, but adding
DBO_DEBUG based on $wgDebugDumpSql seems innocent and doesn't
affect other behaviour (it's a case of DWIM).
Bug: T231742
Change-Id: I122bb1a65620a7ae4e1943136c975b63524a5111
While these have not yet caused problems, we should probably deprecate
the whole confusing "index aliasing" feature. Cleaning this up needs to
be done first.
Method MWLBFactory::setSchemaAliases() is being considered private due
to the @internal notation, and so is being removed without deprecation.
Bug: T233240
Change-Id: Ib04b7bc4af53382fde01bd8550cc7e361124255f
See T233221 for the sordid history. The short version is that it might
exist as `ar_usertext_timestamp`, `usertext_timestamp`, or both on
MySQL, so dropping it for the actor migration needs to drop both names
if they exist.
We can also remove the now-obsolete aliasing from MWLBFactory.
Bug: T233221
Bug: T104756
Bug: T229712
Change-Id: I429e7ade2c86a2514aafe947012f096b03aa67ce
Add HashBagOStuff fallback for APC in MWLBFactory::injectObjectCaches.
Also fix APC-cache variable typo in MWLBFactory.
Bug: T227838
Change-Id: I71cb2ca58972ea09ab2f64f7e47bda7a5096c19b
After approval of RFC T191231, we are going to drop oracle and mssql
and it will be possible to bring back the support using the abstract schema
Adding to release notes will be done in a follow-up
Bug: T230418
Change-Id: I90bd5cfcc3e18011b193c965fdb1fa54675040b5
Mainly:
* Use oci_new_connect() for Oracle to avoid broken connection reuse
similar to the PGSQL_CONNECT_FORCE_NEW flag in DatabasePostgres
* Set 'client_min_messages' unconditionally for PostgreSQL
* Factor out Database::getConnectExceptionAndLog() helper method
* Use the same style of query() calls in DatabaseOracle::open() as
the other subclasses
* Make sure the Database driver handle field is null on failure
instead of false for sanity
Also:
* Disallow changing of Database handle DBO_* flags after construction
where it does not make sense to change them
* Do not mention DBO_* flags meant for non-config use in $wgDBservers
* Ignore DBO_PERSISTENT for SQLite if DBO_TRX is also set for sanity
* Remove $wgDBOracleDRCP variable to discourage careless automatic
setting of DBO_PERSISTENT that breaks LoadBalancer assumptions
Change-Id: Iea948f7f872294ea8fc5d897fc10c9d29b7141d5
