Commit graph

612 commits

Author SHA1 Message Date
Umherirrender
e662614f95 Use explicit nullable type on parameter arguments
Implicitly marking parameter $... as nullable is deprecated in php8.4,
the explicit nullable type must be used instead

Created with autofix from Ide15839e98a6229c22584d1c1c88c690982e1d7a

Break one long line in SpecialPage.php

Bug: T376276
Change-Id: I807257b2ba1ab2744ab74d9572c9c3d3ac2a968e
2024-10-16 20:58:33 +02:00
jenkins-bot
17d6efe4c3 Merge "objectcache: Add support for sister keys in SqlBagOStuff" 2024-10-12 14:04:18 +00:00
Amir Sarabadani
990c4c984f objectcache: Add support for sister keys in SqlBagOStuff
This would allow us to force parsercache idhash and idoptions keys next
to each other so when e.g. depooling 1 of 4 hosts in a parser cache cluster,
the amount of cache misses due to rehashing will be ~25% instead of ~%50.

This is similar to the implementation in WANCache and the same structure
has been used to make it consistent.

Bug: T373037
Change-Id: I920fe76e45298aeee6acf725324a5e1ed2b57a37
2024-10-09 12:15:52 +02:00
thiemowmde
b1c9ec74fa Remove meaningless @var documentation from constants
A constant is not a variable. The type is hard-coded via the value
and can never change. While the extra @var probably doesn't hurt much,
it's redundant and error-prone and can't provide any additional
information.

Change-Id: Iee1f36a1905d9b9c6b26d0684b7848571f0c1733
2024-10-09 09:33:12 +02:00
jenkins-bot
999ffaae3a Merge "objectcache,profiler,externalstore: Simpler mt_rand() for 1 in N chance" 2024-09-25 16:46:28 +00:00
Timo Tijhof
44db493ed1 objectcache,profiler,externalstore: Simpler mt_rand() for 1 in N chance
Based on https://gerrit.wikimedia.org/r/1057269,
as confirmed by https://3v4l.org/XW30U.

Change-Id: I5dc2e140a213bf59f48e39c333280f9662979964
2024-09-25 00:44:31 +00:00
jenkins-bot
ef41dedbd4 Merge "objectcache: Add regression test for MultiWrite dependency injection" 2024-09-24 04:38:56 +00:00
Timo Tijhof
70f9eb8f91 objectcache: Add regression test for MultiWrite dependency injection
Follows-up 4e596f5112 (I59266726ad), which fixed bug T318272, but
did not add a regression test for it.

Bug: T318272
Bug: T327158
Change-Id: Ia8af6671887d3914fdc761d8e5d10fd33fb40f88
2024-09-23 15:07:23 -07:00
Adam Wight
188d2cbbb0 Remove unchecked exception annotations
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
2024-09-17 22:20:58 +02:00
Máté Szabó
f89aa38f69 objectcache: Remove WinCache support
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
2024-09-05 17:59:26 +00:00
Ebrahim Byagowi
c0d1f7694c objectcache: Remove ReplicatedBagOStuff, deprecated since 1.42
Bug: T352481
Change-Id: I4e1ee5680b7ba0207dfe30a1208db35eca07e218
2024-09-04 20:21:29 +00:00
Aaron Schulz
d680a916cc objectcache: make SqlBagOStuff correctly ignore DBO_DEFAULT
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
2024-08-07 19:17:39 -07:00
Derick Alangi
65a8853742 objectcache: Update tests that are really for ObjectCacheFactory
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
2024-07-24 02:57:23 +00:00
Ebrahim Byagowi
fab78547ad Add namespace to the root classes of ObjectCache
And deprecated aliases for the the no namespaced classes.

ReplicatedBagOStuff that already is deprecated isn't moved.

Bug: T353458
Change-Id: Ie01962517e5b53e59b9721e9996d4f1ea95abb51
2024-07-10 00:14:54 +03:30
Umherirrender
9879723ef3 Use namespaced classes (1)
Changes to the use statements done automatically via script
Addition of missing use statement done manually

Change-Id: Ic4d4dd61de5ab896fb6173eb579c81f164a1e4a3
2024-06-16 20:18:23 +02:00
Amir Sarabadani
f33b5515b5 rdbms: Remove ILoadBalancer::getWriterIndex()
It doesn't need to have its own method, We can just use the constant
instead.

Bug: T363839
Change-Id: Iaec5a8e88dc3e5ae4eaf1f24aebf4c5d73f4b350
2024-06-03 14:17:57 -07:00
Umherirrender
cd5d751573 Use RawSQLValue for some SET clauses in upsert
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
2024-05-29 22:34:30 +02:00
Timo Tijhof
ca82b89232 objectcache: Remove "for b/c" from internal ObjectCacheFactory comment
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
2024-05-23 22:52:07 +01:00
Derick Alangi
7475063bfd objectcache: Complete refactor of ObjectCache.php
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
2024-05-21 14:03:08 +00:00
Amir Sarabadani
27f5ba9ab3 rdbms: Remove IDatabase::getTopologyBasedServerId()
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
2024-05-14 13:47:37 +00:00
jenkins-bot
f0a2f1dd1d Merge "fix: use objectcachefactory methods instead of deprecated objectcache methods" 2024-05-05 11:08:07 +00:00
Irina Balaban
dc989f680f fix: use objectcachefactory methods instead of deprecated objectcache methods
Bug: T363770
Change-Id: Ie732f6925ec2b1316a60bebbe3c27f963c9dacb1
2024-05-05 12:40:30 +03:00
James D. Forrester
3ed6668745 Formally deprecate code marked with @deprecated
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
2024-05-03 18:25:03 +03:00
Derick Alangi
e2f6161980
objectcache: Inject DBLoadBalancerFactory into ObjectCacheFactory
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
2024-04-18 22:04:16 +01:00
Derick Alangi
e0c34987eb
objectcache: Restore default keyspace for LocalServerCache service
* 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
2024-03-28 19:47:44 +01:00
jenkins-bot
71225c8ad9 Merge "objectcache: remove deprecated QOS_EMULATION_SQL constant" 2024-03-28 02:28:58 +00:00
Aaron Schulz
ec90b543ab objectcache: remove deprecated QOS_EMULATION_SQL constant
Change-Id: I8dec3f73faec44a00ef12614039c323dd07b695f
2024-03-27 15:10:21 -07:00
Timo Tijhof
a400de795b objectcache: Fix typo in getLocalServerInstance deprecation notice
Bug: T358346
Change-Id: I16a5f588707b2e79ed5340ec8f0aec96195aa577
2024-03-27 12:11:49 -07:00
Derick Alangi
d372626b97
objectcache: Introduce ObjectCacheFactory MW service
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
2024-03-19 12:38:39 +03:00
Derick Alangi
dc91d2d6c5
objectcache: Migrate BagOStuff from StatsdD to StatsLib
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
2024-02-27 19:23:10 +03:00
Derick Alangi
87b479113b
objectcache: Drop support for $wgObjectCaches['db-replicated']
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
2023-12-08 18:14:37 +01:00
James D. Forrester
67217d08df Namespace remaining files under includes/deferred
Bug: T166010
Change-Id: Ibd40734b96fd2900e3ce12239d09becfb4150059
2023-11-22 10:08:53 -05:00
Bartosz Dziewoński
0454c52dde Improve uses of ->where( array_merge( … ) ) in query builders
Replace ->where( array_merge( a, b ) ) with ->where( a )->andWhere( b ).
It's shorter and I find it easier to read.

Change-Id: I94fef6219b5611659f7a09fd3a555aba001f5339
2023-10-26 19:44:02 +02:00
Amir Sarabadani
d5adc3ca65 Mass migrate simple cases to use expression builder
Done via
'([A-Za-z_\.]+) ?(=|!=|<|<=|>|>=) ?' . (\$db(?:r|w|))->addQuotes\( (.+?) \)
to:
$3->expr\( '$1', '$2', $4 \)

And
'([A-Za-z_\.]+) IS NULL OR ([A-Za-z_\.]+) ?(=|!=|<|<=|>|>=) ?' . (\$db(?:r|w|))->addQuotes\( (.+?) \)
to:
$4->expr( '$1', '=', null )->or\( '$2', '$3', $5 \)

Bug: T210206
Change-Id: I109bf2a712bdefa9e074f775b1bee41ac5b9d665
2023-10-26 16:59:19 +00:00
Bartosz Dziewoński
978d739bc6 Replace single-value $db->buildComparison() with $db->expr()
Find:
->buildComparison\( ('..?'), \[(\s*)([^\],]+) => ([^\],]+)(\s*)\] \)

Replace with:
->expr($2$3, $1, $4$5)

Change-Id: I2cfc3070c2a08fc3888ad48a995f7d79198cc336
2023-10-22 01:05:47 +02:00
Derick Alangi
a24c6a6af6 objectcache: Deprecate unused SqlBagOStuff::deleteAll()
This method is no longer used anywhere.

This looks like it was never used since its introduction in r3516
(0c2fba0ac4).

Change-Id: I4318ac4ffded2b0b5f7ed2b1bb7aeba84d198da7
2023-09-27 03:37:40 +00:00
Derick Alangi
f3ae1dbbb4
objectcache: Deprecate unused SqlBagOStuff::expireAll()
In c860f99a6e, the last usage of the
method was removed. We can soft and hard deprecate it at once, did
not find any known usage per code search.

See: https://codesearch.wmcloud.org/search/?q=expireAll%5C%28&files=&excludeFiles=&repos=

Change-Id: I0e4aefec984d16087a72a72381e31cbc72d529c9
2023-09-25 13:04:35 +01:00
jenkins-bot
7f9538e32a Merge "http: MultiHttpClient supports TelemetryHeadersInterface" 2023-09-12 20:42:27 +00:00
Piotr Miazga
8009e9d027 http: MultiHttpClient supports TelemetryHeadersInterface
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
2023-09-12 18:05:17 +02:00
Amir Sarabadani
c02b928d80 objectcache: Roll up small once-used private methods to the caller
Small clean up

Change-Id: I5816e4e7f8652809a3b8c500d8c798bf1312bb33
2023-09-12 14:03:29 +01:00
jenkins-bot
344824516a Merge "rdbms: Move DebugDumpSql knowledge from SqlBagOStuff to DatabaseFactory" 2023-09-11 23:45:40 +00:00
Derick Alangi
5191a7b573 rdbms: Move DebugDumpSql knowledge from SqlBagOStuff to DatabaseFactory
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
2023-09-11 23:31:15 +00:00
jenkins-bot
342b9d4fc9 Merge "rdbms: Introduce ReplaceQueryBuilder" 2023-09-10 23:49:54 +00:00
Amir Sarabadani
79172aed51 rdbms: Introduce ReplaceQueryBuilder
To replace IDatabase::replace()

Bug: T335377
Change-Id: I446f7a09cfc0ee37c2e016052d452751f7333e27
2023-09-08 11:37:26 +02:00
Tim Starling
95bd40b25c In query builders, use insertInto() and deleteFrom() instead of insert() and delete()
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
2023-09-08 10:16:08 +10:00
jenkins-bot
331fa1d876 Merge "Migrate Database::upsert() calls to InsertQueryBuilder" 2023-09-06 13:54:08 +00:00
Amir Sarabadani
f783b02ff2 Migrate Database::upsert() calls to InsertQueryBuilder
Bug: T335377
Change-Id: I0e0c3f3a9150c7a62d8fff95fe8867bdce356071
2023-09-06 14:28:19 +01:00
Timo Tijhof
00dc6de657 objectcache: Minor clean up SqlBagOStuff::getConnectionFromServerInfo
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
2023-09-05 23:24:38 +00:00
Derick Alangi
5f37db6744
objectcache: Use DBO_DEBUG when DebugDumpSql setting is set to true
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
2023-09-05 19:09:13 +01:00
Derick Alangi
f2be8574fe objectcache: Make SqlBagOStuff aware of wgDebugDumpSql setting
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
2023-08-30 19:03:52 +00:00