WinCache is an APCu equivalent for use with Microsoft IIS, but in recent
years has been unmaintained and lacks support for PHP 8 and newer.[1]
So, remove support for it as MediaWiki will be raising the minimum
supported PHP version to 8.1.
[1] https://www.php.net/manual/en/install.windows.recommended.php
Bug: T365691
Change-Id: I4d2dc01a9119bb1f858132f0146b894750c1e86d
Generally, configuration has 0 or DBO_DEFAULT, not DBO_TRX. Also,
clearing DBO_TRX does not stop it from getting set again due to
DBO_DEFAULT in DatabaseFactory::create().
Change-Id: Ie958475c9706199bd4f474f55565c8a9a32f1291
Most of the logic in ObjectCache.php has been refactored to a proper
factory in ObjectCacheFactory and exposed as a service. These tests
now are really for ObjectCacheFactory, hence the rename.
See: I3179a387486377c6a575d173 (d372626b97).
Change-Id: I7c2c20091490065ec93cccdb8cdb311b398d21ba
And deprecated aliases for the the no namespaced classes.
ReplicatedBagOStuff that already is deprecated isn't moved.
Bug: T353458
Change-Id: Ie01962517e5b53e59b9721e9996d4f1ea95abb51
Changes to the use statements done automatically via script
Addition of missing use statement done manually
Change-Id: Ic4d4dd61de5ab896fb6173eb579c81f164a1e4a3
Need a check for RawSQLValue in Database::upsert
Also check for all int keys,
as this must not be an real list starting as index 0
Change-Id: If525243154072ebe81b5ecce2da92e5ccf773ab9
Follows-up 7475063bfd. We don't usually keep back-compat between
a subject and its own tests. It looks like the tests don't have
an alternative way to set or test this, so perhaps this is not for
back-compat but rather for internal usage in tests.
Change-Id: I4d212433326592589a45d71a081c4d01377e689b
This patch completes the rest of the ObjectCache refactor and
migrates methods to the appropriate class while deprecating them
in `ObjectCache.php`.
It also moves the `_LocalClusterCache` internal service logic
into ObjectCacheFactory and calls that instead making sure that
wiring code stays wiring code and let the class do the heavy lifting.
`::makeLocalServerCache()` is retained as a static method in the
ObjectCacheFactory class because it's called early in Setup.php
before the services container is available (so it needs to be stand-
alone).
To add, we also converts all global variables that were used in the
`ObjectCache.php` class into the config schema approach and retrieves
them using ServiceOptions injected in service wiring.
NOTE: MediaWikiIntegrationTestCase::setMainCache() was slightly
rewritten to take care of service reset which throws away the cache
object preserved by setInstanceForTesting() after service reset.
Instead, we preserve the object via MainConfigNames::ObjectCaches
setting with a factory closure which returns the correct cache object.
As a nice side effect of the above, the setInstanceForTesting() method
was removed entirely.
As a follow-up to this patch, I would like to remove the internal
_LocalClusterCache service in a stand-alone patch.
Bug: T363770
Change-Id: Ia2b689243980dbac37ee3bcfcbdf0683f9e1779b
Looking at all implementations, it only returns null or getServerId(),
which we already rely on in the only caller SqlBagOStuff.
Bug: T363839
Change-Id: I680e82d6d36548cd6bc351ab1d1fba48a827cbf3
Some of these have been marked in-code as deprecated for a long while,
but haven't ever been announced in the RELEASE-NOTES (and later,
HISTORY) file, so let's mark them up now so we can get the ball rolling
at least.
Per Gergo, the AuthManager one was 'born deprecated' and should only
have been used by humans also reading the deprecation notice given in
the code, and indeed no uses are known to code search, so also emit
deprecation warnings there immediately; others will have to wait until
uses have been migrated.
Change-Id: I0c1c71d8f4293623039302da35d58d2a24367e97
This patch does the follow with a few gotchas:
* Properly inject LBFactory service into ObjectCacheFactory to be
used where needed.
* doc: Pull in relevant documentation from ObjectCache class into
the ObjectCacheFactory class and also update docs in the OCF
class to be more accurate with the code.
* This patch also resolves an issue that caused an infinite loop in
SqlBagOStuff making connections to the DB not to be reused within
a request there by crashing the application (when the index.php
entry-point) is accessed directly and every cache type is set to
CACHE_ANYTHING. Meaning in LocalSettings, only `$wgMainCacheType`
is set and its relatives (ParserCache, MessageCache, etc) aren't
set.
NOTES
=====
-> A circular dependency would occur in OCF when injecting LBFactory:
DBLoadBalancerFactory->DBLoadBalancerFactoryConfigBuilder
->LocalServerObjectCache->ObjectCacheFactory->DBLoadBalancerFactory
directly, so in order to resolve this, we'll inject a closure instead
and call that to retrieve the service when needed. The solution above
is already used in other services today in the code. As an example,
you can see SignatureValidatorFactory.
-> In MainConfigSchema.php, the CACHE_ANYTHING key got removed
in https://gerrit.wikimedia.org/r/c/mediawiki/core/+/955771
and this is a change in behavior. So we need to recompute the
value of CACHE_ANYTHING's ID via service wiring for DB operations.
Test plan
=========
This patch is fairly straight forward, all it does is do some DI of
a service into OCF (via a callable). No change in behavior should be
expected in your local wiki.
So if your local wiki is still running smoothly when you hit the
`/index.php` entry-point directly and other cases, then everything
should be working fine.
Bug: T362686
Change-Id: I305ef0c377a023236b8ed9a101762813f32e6cd0
* Fix main makeLocalServerCache() call in ObjectCacheFactory::newFromId
to include a default keyspace, since wgCachePrefix is false by default
(including at WMF).
* Idem for ExtensionRegistry.
* Dependency inject the domain ID so that service wiring does the
correct thing when doing cross-wiki operations.
This is a followup on: I3179a387486377c6a575d173f39f82870c49c321.
Bug: T358346
Bug: T361177
Change-Id: Ibbb250465529810b8593f90bbb8330af0a2c3dbd
ObjectCache is already doing a lot of factory pattern logic like
creating instances of the various BagOStuff, this should really be
the responsibility of the factory servicet.
This patch introduces a proper factory (ObjectCacheFactory) to handle
the responsibility of creating various instances of BagOStuff. Since
`newFromParams()` is a static function that gets passed in configuration
of $wgObjectCaches, that can stay that way (to keep supporting how we do
this in prod today).
Technical Breaking Change: `ObjectCache::makeLocalServerCache()` now has
a parameter and requires it but there are no callers of this method outside
MW core hence it is safe to change (and this patch update all callers) to
work correctly. Cache prefix is gotten from global state because sometimes
at this stage, the services container is not available.
Bug: T358346
Change-Id: I3179a387486377c6a575d173f39f82870c49c321
This patch migrates BagOStuff metrics capture from StatsdD to
StatsLib. `updateOpStats()` definition significantly changed with
this migration thereby affecting the changes in the test case.
What we tried to do in the test is to assert that the metrics
match the StatsLib equivalent ones which is the new format we're
using and that the values corresponding to the samples are also
in the range of values written to the cache.
Notes
=====
* And the cache key also uses underscore (_) instead of (.) because
of StatsLib as well.
* The most important thing which is tested is that the keys inserted
into the cache are all there and correspond to their values. The way
we do this is observe the metrics stream after `->flush()`.
Bug: T356062
Change-Id: Ia6c14746de5bddeaca7917c76f1c9d1e100ae2b2
This was introduced but never really used outside of core[1]. The only
place that used it in core was MainStash setting which under the hood
will use CACHE_DB (SqlBagOStuff).
This patch removes the "db-replicated" key in $wgObjectCaches without
deprecation because it was never really used in the first place and
had a replacement already when it got released, see: T352481.
[1] https://codesearch.wmcloud.org/search/?q=ReplicatedBagOStuff&files=&excludeFiles=&repos=
Bug: T352481
Change-Id: I8e19ee262a64b00742bb9203b2a2610ec0cc39fa
Replace ->where( array_merge( a, b ) ) with ->where( a )->andWhere( b ).
It's shorter and I find it easier to read.
Change-Id: I94fef6219b5611659f7a09fd3a555aba001f5339
This method is no longer used anywhere.
This looks like it was never used since its introduction in r3516
(0c2fba0ac4).
Change-Id: I4318ac4ffded2b0b5f7ed2b1bb7aeba84d198da7
Introduce a new interface Wikimedia/Http/TelemetryHeadersInterface
that provides telemetry information that could be attached to
HTTP Requests. MultiHttpClient is expecting `telemetry` option
of TelemetryHeadersInterface type.
The MediaWiki/Http/Telemetry implements the interface, therefore
ObjectCache can inject it to RESTBagOStuff, that further injects
it to MultiHttpClient.
Bug: T344926
Change-Id: I59a3f1048c403fe2e4ef0c74353dfe74ff9ca893
This patch process a more general approach to injecting the correct
setting the value of DebugDumpSql when its override is available, like
in a wiki install and removes the knowledge about this from SqlBagOStuff.
We can rely on the default of false if we're in the Installer, or
other standalone use of DatabaseFactory, where we don't care much about
logging SQL queries. This doesn't change behaviour since before, SqlBag
would read the default value of $wgDebugDumpSql which is also false.
This follows-up to 5f37db6 and 00dc6de so that the issue is not only
fixed in SqlBagOStuff but in all places that get a DatabaseFactory via
the service container.
Bug: T318272
Change-Id: Ia89de3b32b4db1b7abc9ed3ac676af4e7ac18ffa
The design principle for SelectQueryBuilder was to make the chained
builder calls look as much like SQL as possible, so that developers
could leverage their knowledge of SQL to understand what the query
builder is doing.
That's why SelectQueryBuilder::select() takes a list of fields, and by
the same principle, it makes sense for UpdateQueryBuilder::update() to
take a table. However with "insert" and "delete", the SQL designers
chose to add prepositions "into" and "from", and I think it makes sense
to follow that here.
In terms of natural language, we update a table, but we don't delete a
table, or insert a table. We delete rows from a table, or insert rows
into a table. The table is not the object of the verb.
So, add insertInto() as an alias for insert(), and add deleteFrom() as
an alias for delete(). Use the new methods in MW core callers where
PHPStorm knows the type.
Change-Id: Idb327a54a57a0fb2288ea067472c1e9727016000
Follows-up I8ba1d35f0e36ae259 which started a mix between hanging $server
for one key, and merging with an inline array for their keys. Move the
remaining two changes out for consistency, so that there is not a state
where $server is left in a "wrong" state where it is only partially correct.
Clarify reason for deducting DBO_TRX.
Change-Id: I1bf57dea5f1a755152e84d4984144369fc392e2f
This is to be consistent with the DBO_* constants in IDatabaseFlags.php
not use the setting value directly.
This worked in the past because it evaluates to true or false based on
$sqlDump, which means instead of setting flags to DBO_DEBUG (int), it was
setting it to bool(true).
This worked by accident because:
* when we perform -, +, | or & math on a value, it is casted to an integer,
* and intval(true) === 1,
* and DBO_DEBUG is the first constant in DatabaseFlagsHolder, which we gave
the number 1.
It is only the combination of all three of these facts that made it work by
accident.
Follow-up: I4ce691b77f775c20bcf4e8deb2ec1aae1b4674d8
Bug: T318272
Change-Id: I8ba1d35f0e36ae259830fc1f65fcb4e4dc92a8ec
This enables DatabaseFactory::create() aware of the DebugDumpSql
setting. To be able to log the SQL queries executed, process this
setting as the default.
Bug: T318272
Change-Id: I4ce691b77f775c20bcf4e8deb2ec1aae1b4674d8
== Background
Most of this was introduced in commit 5c335f9d77 (I1eb897c2cea3f5b7).
The original motivation was:
* Ensure wrappers like MultiWriteBagOStuff naturally do the right
thing. In practice, makeKey() results are interchangeable, with
the most contrained one (Memcached) also generally used as the first
tier. However, this is not intuitive and may change in the future.
To make it more intuitive, the default implemention became known
as "generic", with proxyCall() responsible for decoding these,
and then re-encoding them with makeKey() from the respective
underlying BagOStuff. This meant that MultiWriteBag would no longer
use the result of the Memcached-formatted cache key and pass it
to SqlBagOStuff.
* Allow extraction of the key group from a given key cache,
for use in statistics.
Both motivations remains valid and addressed after this refactor.
== Change
* Remove boilerplate and indirection around makeKey from a dozen
classes. E.g. copy-paste stubs for makeKey, makeKeyInternal, and
convertGenericKey.
Instead, let BagOStuff::makeKey and ::makeKeyInternal hold the
defaults. I believe this makes the logic easier to find, understand,
and refer to.
The three non-default implementations (Memcached, WinCache, Sql)
now naturally reflect what they are in terms of business logic,
they are a method override.
Introduce a single boolean requireConvertGenericKey() to let the
three non-default implementations signal their need to convert
keys before use.
* Further improve internal consistently of BagOStuff::makeKeyInternal.
The logic of genericKeyFromComponents() was moved up into
BagOStuff::makeKeyInternal. As a result of caling this directly
from BagOStuff::makeKey(), this code now sees $keyspace and $components
as separate arguments. To keep the behaviour the same, we would
have to either unshift $keyspace into $components, or duplicate
the strtr() call to escape it.
Instead, excempt keyspace from escaping. This matches how the most
commonly used BagOStuff implementations (MemcachedBag, and SqlBag)
already worked for 10+ years, thus this does not introduce any new
responsibility on callers. In particular, keyspace (not key group)
is set by MediaWiki core in service wiring to the wiki ID, and so
is not the concern of individual callers anyway.
* Docs: Explain in proxyCall() why this indirection and complexity
exists. It lets wrapping classes decode and re-encode keys.
* Docs: Explain the cross-wiki and local-wiki semantics of makeKey
and makeKeyGlobal, and centralise this and other important docs
about this method in the place with the most eye balls where it is
most likely seen and discovered, namely BagOStuff::makeKey.
Remove partial docs from other places in favour of references to this one.
Previously, there was no particular reason to follow `@see IStoreKeyEncoder`
much less to know that it holds critical that communicate the
responsibility to limit the key group to 48 chars.
* Docs: Consistently refer to the first component as the "key group",
thus unifying what was known as "key class", "collection",
"key collection name", or "collection name".
The term "key group" seems to be what is used by developers in
conversations for this concept, matching WMF on-boarding docs and
WMF's Grafana dashboard for WANObjectCache.
Change-Id: I6b3167cac824d8bd8773bc66c386f41e4d380021
This was previously blocked by T332329, but is now possible since change
I79c308a895 (commit 7cf42e26c9).
Change-Id: If120650262bf5103b858e1ad9996819d2787c4ad
I did this using a script written on top of antlr4 parser so it doesn't
have some clean ups a human would do but it's pretty nice already.
Bug: T330640
Change-Id: I608566700c6d737ee986bf47dda87effc69614d6
When $wgMainCache is CACHE_ANYTHING, recursion proceeds via
LoadBalancer::isPrimaryRunningReadOnly() until the DB server hits its
maximum connection limit. The try/catch blocks in ServiceWiring were no
longer effectively preventing this.
So, inspect the configuration to detect whether the main cache type will
be CACHE_DB, without actually instantiating it.
Bug: T334970
Change-Id: Ibdbc19c45eb20bb85df720669f7ce1d50e3656ff
* Move getConnection() call to the callback. While our handlers
are DBConnRef and lazy-connecting by default, it still seems bad
form to hold on to a handle acquired during pre-send, across
a closure, and into post-send DeferredUpdates (via asyncHandler).
* Replace inconsistent time() with getCurrentTime() so that the data
source is obviously compatible with the conditional above.
* Use a non-blocking IDatabase::lock() to discard overlapping attempts
to perform selects and deletes over the same data range.
Bug: T330377
Change-Id: I0fcb8d77ccab040898090a4726b7ef54dc3732a1
* 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
