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
And deprecated aliases for the the no namespaced classes.
ReplicatedBagOStuff that already is deprecated isn't moved.
Bug: T353458
Change-Id: Ie01962517e5b53e59b9721e9996d4f1ea95abb51
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
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
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
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
This is preparation for re-using it in the MainWANObjectCache
service in a way that preserves proper dependency injection.
This is related to a declined task (T243233) which proposed offering
it as a non-internal service. It is expected to remain internal, with
public callers generally migrated to WANObjectCache or WRStats or
in some cases to a (not yet implemented) lock manager service.
Bug: T329680
Change-Id: I8af9667529b4896f17a22b66823e7eac859d4229
Subclasses of Maintenance that can function without database access can
now override canExecuteWithoutLocalSettings to return true.
This is useful for scripts that need to be able to run before or without
installing MediaWiki.
When no config file is present, the storage backend will be disabled.
Any attempt to access the database will result in an error.
NOTE: This makes MediaWikiServices::disableBackendServices() more
comprehensive, to avoid premature failures during service
construction. This change makes it necessary to adjust how the
installer manages service overrides.
The CLI installer is covered by CI, I tested the web installer
manually.
Change-Id: Ie84010c80f32cbdfd34aff9dde1bfde1ed531793
I added the `Config` parameter not so long ago, but this still seems
awkward and insufficient. Instead of requiring the caller in
ServiceWiring to unpack each service, do this here instead to better
separate the concerns between what a service owner should know vs
what maintainers of ObjectCache need to know.
Document the parameter as internal going forward. There exist no
callers Codesearch Everywhere outside these two core files.
Bug: T329680
Change-Id: I37e3fcae0551ba96d480071b2da5166ac966092e
I noticed that things like 'logger', 'keyspace', and 'stats' are
unset in WMF production for the SqlBag embedded in MultiWriteBag.
This is a regression from switching its constructor from
calling ObjectCache::newFromParams to calling ObjectFactory directly.
It happens to have little to no impact on prod, but only thanks to
various coincidences, each of which can change and start to cause
problems.
Bug: T318272
Change-Id: Ia893ddb36427eb5e9bff0a3776a6856d10d01adc
The main object cache is disabled during testing. Some integration tests
need it though. This provides a clean way to enable it, to replace the hacks
that were used so far.
Note that we may want to enable the main cache during testing soon. When
that happens, this method is still useful to disable the cache in certain
tests, and to set a specific cache instance.
Change-Id: I04ae1bf1b6b2c8f6310acd2edf89459d01a9c870
Substantive changes:
* Remove the parameters globalKeyLB and localKeyLB, not needed in
production, probably not used anywhere.
* Add a "cluster" parameter, for production.
* Don't call markServerDown() in LoadBalancer mode.
Style changes:
* Document config parameters separately from constructor parameters, to
clarify the role of the wiring normalisation.
* Inject a load balancer from ObjectCache to SqlBagOStuff, using a
closure to defer construction.
* Clarify the split between LoadBalancer mode and server array mode by
introducing a useLB property. Use it instead of special shard index
values and array counts.
* multiPrimaryModeType wasn't needed anywhere and wasn't available in
the constructor, so it just became a boolean multiPrimaryMode.
* Add a test covering most of the server array (!useLB) branches.
Bug: T212129
Change-Id: Ib097082efb40b514a29a42286dc362f2e3743ee4
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
By default, CACHE_DB and db-replicated are similar, with the main
difference being that db-replicated will prefer to read from a local
replica server rather than the primary server. Setups using sqlite
do not have use for replication logic, since everything uses local
database files. To reduce contention, SqliteInstaller makes CACHE_DB
use a different database file (rather than a table in the main wiki
database file).
This change makes 'db-replicated' reuse the same cache instance.
Tweak ObjectCache::newFromParams() to make ObjectFactory usable.
Also set $wgResourceLoaderUseObjectCacheForDeps to true to avoid
similar contention in SqlModuleDependencyStore.
Bug: T303225
Change-Id: Ia644f35698b69fe340fc1bcae8899c7e332f332d
This replaces references to DefaultSettings with
references to config-schema.yaml where appropriate.
NOTE: this does not yet change Setup.php. DefaultSettings.php
remains intact and is still being used.
NOTE: this does not remove usages in the installer, see I5d8843a1062fbf
for that.
Bug: T300129
Change-Id: Ie6152cf510c3be61bc22167ca6d90dfc28910a45
Dependency inject the write batch size into SqlBagOStuff.
Clean up SqlBagOStuff method names and comments.
Renamed getServerIndexByTag() to getShardServerIndexForTag() and
getServerShardIndexes() to getShardServerIndexes(). Since each of these
servers manages one of the shards, it makes sense to call them "shard
servers".
Change-Id: I9af1f64602dabe85288937ab52c08470138f4c91
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
Update SQL, REST, and redis subclasses to emit call count and
payload size metrics for cache key operations. These metrics
are bucketed by cache key collection (similar to WANCache).
Bug: T235705
Change-Id: Icaa3fa1ae9c8b0f664c26ce70b7e1c4fc5f92767
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
Follows-up 746d67f5fc which implicitly caused the APCUBagOStuff
object to no longer have a wiki-dependent keyspace. This meant
that all cache keys were shared across wikis, even if they used
makeKey() instead of makeGlobalKey() because the keyspace was
not defined (e.g. it was the string "local" for all wikis, instead
of a string like "enwiki").
Bug: T247562
Change-Id: I469e107a54aae91b8782a4dd9a2f1390ab9df2e5
This was added in ce84590988e, and is no longer needed as
of 10dce13709.
Also remove the comment that announces deprecation/removal,
given newFromParams is used as callback in wgObjectCaches,
that might not be feasible. We can keep supporting it as an
optional parameter for now for uses in tests and for uses
outside configuration (e.g ServiceWiring), so that best
practices can be followed where they make sense, but still
allow bypass for config use case, since that would only ever
inject the One True Config object anyway.
Change-Id: I8cc4bfb1862b81df2c31fdc0886364b092636cc2
This bypasses the indirection of global wgObjectCaches config and
ObjectCache::newFromParams static methods, which don't do anything
in practice.
This solves the ExtensionRegistry use-case where it couldn't use
the service container, which in turn opens the path for
being able to deprecate absence of Config being passed to
ObjectCache::newFromParams(), which isn't possible right now because
ExtensionRegistry depended on being able to call it without that,
and that is a hard requirement because ExtensionRegistry isn't
allowed to use the service container.
Change-Id: Ic88da279662f33d3585cb2232358d6faf590b5b3
* Avoid direct $GLOBALS lookups.
* Avoid MediaWikiServices singleton for Config object.
Also given that newFromParams() constructs its Logger object
unconditionally as of I2aa835c5ec79, stop creating it ahead of
time in ServiceWiring.
This code, including the default loggroup value, was duplicated by
me from ObjectCache.php to ServiceWiring.php in commits 3828558140
and bd16c5eb34 because I needed to obtain the Logger object
in order to send a message to it about how it was created.
Solve this debt by letting ServiceWiring access the actual logger
that was created by newFromParams() through a new getLogger()
object.
Change-Id: Ib2de7b22f6c79db0c700fae06269d04fbe27831d
Apply the same duplication logging and asyncHandler defaults for
factory-based entries as constructor-based entries.
Change-Id: I2aa835c5ec7932432d2c739ffa761a7bd9c21198
This is not used anywhere. Once removed we can remove a bunch
more protected/private methods in this class that are all only
here still due to this one public method.
It was obsolete when the creation of WANObjectCache was
moved to ServiceWiring. In theory multiple instances can be
created but in the future that should be done by either constructing
the WANObjectCache directly where needed, or by implementing
it as a custom service.
Also deprecate ObjectCache::newWANCacheFromParams. This should
have been `@internal`, but oh well. Deprecate as well since
only needed by getWANInstance(). No longer used internally
either as of I118b6f01e4940.
Change-Id: Icc500e76c860c35ff40fdee47142c63091829dfe
