aboutsummaryrefslogtreecommitdiffstats
path: root/includes/objectcache
Commit message (Collapse)AuthorAgeFilesLines
...
| * | rdbms: Move DebugDumpSql knowledge from SqlBagOStuff to DatabaseFactoryDerick Alangi2023-09-111-10/+3
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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
* | | Merge "rdbms: Introduce ReplaceQueryBuilder"jenkins-bot2023-09-101-1/+5
|\ \ \ | |_|/ |/| |
| * | rdbms: Introduce ReplaceQueryBuilderAmir Sarabadani2023-09-081-1/+5
| |/ | | | | | | | | | | | | To replace IDatabase::replace() Bug: T335377 Change-Id: I446f7a09cfc0ee37c2e016052d452751f7333e27
* / In query builders, use insertInto() and deleteFrom() instead of insert() and ↵Tim Starling2023-09-081-10/+10
|/ | | | | | | | | | | | | | | | | | | | | | | | | 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
* Merge "Migrate Database::upsert() calls to InsertQueryBuilder"jenkins-bot2023-09-061-59/+62
|\
| * Migrate Database::upsert() calls to InsertQueryBuilderAmir Sarabadani2023-09-061-59/+62
| | | | | | | | | | Bug: T335377 Change-Id: I0e0c3f3a9150c7a62d8fff95fe8867bdce356071
* | objectcache: Minor clean up SqlBagOStuff::getConnectionFromServerInfoTimo Tijhof2023-09-051-14/+9
| | | | | | | | | | | | | | | | | | | | | | 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
* | objectcache: Use DBO_DEBUG when DebugDumpSql setting is set to `true`Derick Alangi2023-09-051-1/+6
|/ | | | | | | | | | | | | | | | | | | | | | 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
* objectcache: Make SqlBagOStuff aware of wgDebugDumpSql settingDerick Alangi2023-08-301-2/+5
| | | | | | | | | 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
* objectcache: Reduce boilerplate and indirection around makeKey()Timo Tijhof2023-08-031-0/+4
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | == 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
* Use DeleteQueryBuilder in one more placeLucas Werkmeister2023-07-061-1/+4
| | | | | | | This was previously blocked by T332329, but is now possible since change I79c308a895 (commit 7cf42e26c9). Change-Id: If120650262bf5103b858e1ad9996819d2787c4ad
* Replace IDatabase::delete with DeleteQueryBuilderUmherirrender2023-06-211-8/+10
| | | | Change-Id: Ie0c1c955ca1a7028f75f24563fdeb9f94285af30
* Migrate Database::update() to UpdateQueryBuilderAmir Sarabadani2023-06-081-6/+5
| | | | | | | | 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
* Fix infinite recursion in DBLoadBalancerFactoryConfigBuilder serviceTim Starling2023-05-031-9/+63
| | | | | | | | | | | | | 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
* objectcache: Use DB lock in SQLBagOStuff purge to avoid deadlockTimo Tijhof2023-04-051-8/+20
| | | | | | | | | | | | | | | | * 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
* Fix even more PHPStorm inspections (#3)Tim Starling2023-03-251-1/+1
| | | | | | | | | | | | | | | * 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
* objectcache: remove deprecate BagOStuff::incr() methodAaron Schulz2023-03-101-36/+0
| | | | Change-Id: I107c04e05585c975dc37ce402746a4671f2f43b5
* objectcache: remove deprecate BagOStuff::decr() methodAaron Schulz2023-03-101-4/+0
| | | | Change-Id: I8284289f3d37c763eabdecba9b8d0c4beed4e5de
* Reorg: Migrate WikiMap to WikiMap/ out of includesAmir Sarabadani2023-02-271-0/+1
| | | | | | | And WikiReference Bug: T321882 Change-Id: I60cf4b9ef02b9d58118caa39172677ddfe03d787
* rdbms: Remove Database::attributesFromType and hard-deprecate ::factoryAmir Sarabadani2023-02-231-1/+3
| | | | | | | | They have been soft-deprecated since 1.39 and the first one is not used anywhere, ::factory is used in some third party code but not in Wikimedia-deployed ones Change-Id: Icd0f743d9c76554dd02471485ee732e25b9fb932
* objectcache: Move main cache to internal "_LocalClusterCache" serviceTimo Tijhof2023-02-211-3/+1
| | | | | | | | | | | | | 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
* objectcache: make BagOStuff::makeKeyInternal protected in subclassesAaron Schulz2023-02-211-1/+1
| | | | | | The base class already defines the method as protected Change-Id: If7dc6ddc7932eb846027632cd1f32707f5809ad4
* Allow some maintenance scripts to be called without a LocalSettingsdaniel2023-02-161-3/+8
| | | | | | | | | | | | | | | | | | | | 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
* ObjectCache: Make newFromParams() more suitable for internal re-useTimo Tijhof2023-02-151-5/+6
| | | | | | | | | | | | | | 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
* Remove unused arguments to private functionsUmherirrender2023-02-081-21/+9
| | | | | | Found by phan dead detection Change-Id: I93379b7b9a733206d0e53add04fcdb9478c58755
* Remove unused local variable assignmentUmherirrender2023-02-041-1/+0
| | | | | | Dead code found by phan Change-Id: I9fc404d546a4fb1c61394cb6359eb774fd94383a
* objectcache: Fix DI for MultiWriteBagOStuff sub cachesTimo Tijhof2023-01-171-1/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Follows-up Ia893ddb36427eb5, which added unreachable code because is_subclass_of is strictly for matching subclasses. Test plan: * Add the following to your LocalSettings: $wgParserCacheType = 'mysql-multiwrite'; $wgObjectCaches['mysql-multiwrite'] = [ 'class' => 'MultiWriteBagOStuff', 'caches' => [ 0 => [ 'factory' => [ 'ObjectCache', 'getInstance' ], 'args' => [ CACHE_DB ] ], 1 => [ 'class' => SqlBagOStuff::class, 'loggroup' => 'SQLBagOStuff', 'server' => [ 'type' => 'sqlite', 'dbname' => 'wikicache', 'tablePrefix' => '', 'variables' => [ 'synchronous' => 'NORMAL' ], 'dbDirectory' => $wgSQLiteDataDir, 'trxMode' => 'IMMEDIATE', 'flags' => 0 ] ], ], 'replication' => 'async', ]; * Main_Page fatals without this patch, renders with. Bug: T327158 Change-Id: I59266726ad72e78c9f99d3cc8f9c8838fabed3b5
* objectcache: Fix lack of DI for MultiWriteBagOStuff sub cachesTimo Tijhof2023-01-131-3/+14
| | | | | | | | | | | | | | | 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
* objectcache: Move default 'stats' assignment with the othersTimo Tijhof2023-01-111-4/+1
| | | | Change-Id: Ia3b318d84e7636a0b8b682d4d9e7dca274217ff3
* rdbms: Consolidate logger channels into oneTimo Tijhof2023-01-031-2/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Notable changes: * In SqlBagOStuff::getConnectionFromServerInfo, only two loggers were injected. The rest implicitly got a NullLogger due to being absent. These are now effectively unsilenced. * Database::__construct() required almost all parameters, even the loggers. I've wanted to move some of DatabaseFactory into the ctor here for a while. In order to make this change not a breaking change, the new 'logger' parameter is optional with NullLogger as default. This allowed some of the test cases, which were simply passing NullLogger, to be fixed by passing nothing instead of passing the new option name. The Database class is behind a dozen layers of indirection for real use, so this will still be injected just fine (DBF, LB, LBF, MWLBF, etc.). * In LegacyLogger, the handling for $wgDBerrorLog was previously limited to DBConnection and DBQuery. This now includes errors from other (generally, newer) parts of Rdbms as well, which were previously missing. This only affects sites (typically CI and dev setup) where $wgDBerrorLog is used, as opposed to the more common $wgDebugLogGroups by-channel configuration. * TransactionProfiler gets its logger injected in a rather odd way, via entrypoints (MediaWiki.php, ApiMain.php, and MaintenanceRunner) as opposed to service wiring. This is kept as-is for now. * In LBFactoryTest, in particular testInvalidSelectDBIndependent2, there are cases that intentionally produce failures of which the result is then observed. In CI we assert that dberror.log is empty so instead of adding the missing logger fields to that LBFactory instance, the only one set (replLogger) is removed. The alternative is to set 'logger' now, which would naturally cause CI failures due to unexpected entries coming through to non-mocked error log. Bug: T320873 Change-Id: I7ca996618e41b93f488cb5c4de82000bb36e0dd3
* Merge "objectcache: suppress TransactionProfiler in ↵jenkins-bot2022-11-171-0/+2
|\ | | | | | | occasionallyGarbageCollect()"
| * objectcache: suppress TransactionProfiler in occasionallyGarbageCollect()Aaron Schulz2022-11-101-0/+2
| | | | | | | | | | Bug: T258125 Change-Id: I3b0f5b5efb85ccd868b8d797c43565d1786a86b6
* | objectcache: use multi-row upsert support in SqlBagOStuffAaron Schulz2022-11-171-64/+71
|/ | | | | | This cuts down on needless roundtrips and lock contention Change-Id: Ic9681512a4b17f000da21b45cacad173e644a44e
* Merge "Use array_key_first()/array_key_last() in some places"jenkins-bot2022-10-211-2/+1
|\
| * Use array_key_first()/array_key_last() in some placesTim Starling2022-10-211-2/+1
| | | | | | | | | | | | | | | | | | | | | | Introduced in PHP 7.3. I used it to replace reset()/end() followed by key() where the return value of reset() is not captured and internal pointer iteration is not locally occuring. I also used it in a couple of places when reset() is absent but array_key_first() is semantically desired. Change-Id: I750d3fa71420cbdca5fb00d82ac5ca40821769d4
* | Merge "Use short array destructuring instead of list()"jenkins-bot2022-10-211-14/+14
|\ \
| * | Use short array destructuring instead of list()Tim Starling2022-10-211-14/+14
| |/ | | | | | | | | | | | | | | Introduced in PHP 7.1. Because it's shorter and looks nice. I used regex replacement. Change-Id: I0555e199d126cd44501f859cb4589f8bd49694da
* / Use the null coalescing assignment operatorTim Starling2022-10-211-1/+1
|/ | | | | | | | Available since PHP 7.4. Automated search, manual replacement. Change-Id: Ibb163141526e799bff08cfeb4037b52144bb39fa
* Merge "SqlBagOStuff: Fix modtoken comparison"jenkins-bot2022-08-191-1/+2
|\
| * SqlBagOStuff: Fix modtoken comparisonTim Starling2022-08-191-1/+2
| | | | | | | | | | | | | | | | | | | | | | | | String offsets in MariaDB are 1-based, except in "Oracle compatible" mode. SUBSTR(modtoken,0,13) was always the empty string and so the modtoken comparison was always true. I was able to reproduce a failure to reach consistency using ring replication. Add regression test. Bug: T315271 Change-Id: I74e54e8aba44505dd04426c12d91a9ea0de17f22
* | SqlBagOStuff: Migrate from IDatabase::select to SelectQueryBuilderDerick Alangi2022-08-181-36/+46
|/ | | | | Bug: T311866 Change-Id: I0101a0d5f4008bfd79e823248bb625e15f7135bd
* Merge "SqlBagOStuff: use cancelAtomic()"jenkins-bot2022-08-171-10/+17
|\
| * SqlBagOStuff: use cancelAtomic()Tim Starling2022-08-171-10/+17
| | | | | | | | | | | | | | | | | | | | | | | | | | | | * When an exception is thrown from modifyTableSpecificBlobsForIncrInit(), call cancelAtomic() on the connection. Otherwise a subsequent call to LBFactory::commitPrimaryChanges() will throw a user-visible error, violating the goal of graceful failure. * After getting a DBConnRef, call ensureConnection() on it. Otherwise the try/catch blocks around getConnection() have no purpose since connections are always lazy-initialised. Bug: T315274 Change-Id: I8a5bc90bb340f34bd4a88872a1a2e72612ac91f4
* | objectcache: Add trace to SqlBagOStuff DBError loggingTimo Tijhof2022-08-171-4/+4
|/ | | | | Bug: T315274 Change-Id: I7551f2d41238f2dd064e1049723b22805b495516
* objectcache: Remove unused WRITE_SYNC flagTimo Tijhof2022-08-021-39/+0
| | | | | | Bug: T270225 Depends-On: I7e72c1180b7ba9e479ade62ab3dd3139d7bd5bb0 Change-Id: I9f59ff35bbb806d7b1375739001d1cf458f366a8
* PHPUnit: introduce setMainCachedaniel2022-07-071-1/+1
| | | | | | | | | | | | 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
* objectcache: Optimise SqlBagOStuff::incrWithInit with WRITE_BACKGROUNDTim Starling2022-07-041-1/+43
| | | | | | Bug: T310662 Bug: T261744 Change-Id: I9549722ff6f0c4d62d1bcbe8de55f51758157ec4
* objectcache: Deprecate BagOStuff::addBusyCallback() and reduce to a stubTim Starling2022-05-271-10/+1
| | | | | | | | | | It's not called by anything since it was removed from ChronologyProtector, and it is not needed for production. Pass the timeout directly to LoadBalancer::waitForAll() instead of using a WaitConditionLoop, as it was before e99bb1b7dcc4c52a74b Change-Id: I5a1903e6e9ac145c06f77c690a0f6a487b4aa43f
* objectcache: Simplify SqlBagOStuff class configurationTim Starling2022-05-272-170/+113
| | | | | | | | | | | | | | | | | | | | | | | | | 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
* Merge "objectcache: reduce function_exists() calls in SqlBagOStuff"jenkins-bot2022-05-131-2/+7
|\
generated by cgit v1.2.3 (git 2.25.1) at 2025-04-26 23:02:43 +0000