aboutsummaryrefslogtreecommitdiffstats
path: root/includes/libs/rdbms/ChronologyProtector.php
Commit message (Collapse)AuthorAgeFilesLines
* libs: Use type declaration on undocumented private functionsUmherirrender2025-03-311-1/+1
| | | | Change-Id: I9a74c316b87ae35597ce846a830a55542d9aa14c
* rdbms,objectcache: Replace wgChronologyProtectorStash with MicroStashTimo Tijhof2024-10-091-1/+1
| | | | | Bug: T336004 Change-Id: I2f769aa703ce98b15fa0fe98eda092ff19c27d0a
* Add namespace to the root classes of ObjectCacheEbrahim Byagowi2024-07-101-2/+2
| | | | | | | | | And deprecated aliases for the the no namespaced classes. ReplicatedBagOStuff that already is deprecated isn't moved. Bug: T353458 Change-Id: Ie01962517e5b53e59b9721e9996d4f1ea95abb51
* rdbms: Move IReadableDatabase::primaryPosWait() to IDatabaseForOwnerAmir Sarabadani2024-06-121-1/+1
| | | | | | | Only used in LB Bug: T363839 Change-Id: I0b66dc85b1dd282b567c0a1847cdf07d2bcfe600
* rdbms: Remove ILoadBalancer::getWriterIndex()Amir Sarabadani2024-06-031-2/+2
| | | | | | | | It doesn't need to have its own method, We can just use the constant instead. Bug: T363839 Change-Id: Iaec5a8e88dc3e5ae4eaf1f24aebf4c5d73f4b350
* rdbms: Drop ILoadBalancer::getAnyOpenConnection()Amir Sarabadani2023-11-281-5/+1
| | | | | | | | | | This is marked @internal and thus can be dropped without notice. To avoid CP opening a connection, simply do the logic of checking in LBF where it has access to better tooling. Bug: T325389 Change-Id: I4b30c2fb2158a5ef0588b585366dc9411a08dc12
* rdbms: Minor clean up in ChronologyProtectorTimo Tijhof2023-09-141-31/+14
| | | | | | | | | | | | | | | | | | | | | | | | * Clarify log messages e.g. instead of has or has no positions, say "waiting" and "skip waiting". Copied from abandoned I27518ff0d2. * Rename hasImplicitClientId to hasNewClientId. I added this I7cc280261e (c11664d1cd) as a temporary variable to distinguish between implicitly and explicitly waiting for positions. Since If7082f62f2 (54aeb04fc5) removal of the implicit case is completed. We could leave $clientId as null and generate it lazily, but for now we can keep the bool under a clearer name, and it's outdated docs removed. * Remove json_encode call and debug log call for request info. It is disabled in production and thus overhead for no value. This info is redundant with the log context set a few lines down. If/when CP is enabled, it will log one message at some point and thus log this information already. Bug: T314434 Change-Id: Iadf14b1aa8f8b2ef7ae9e8eeadf24e0b23ef3dfb
* rdbms: Rename CP::yieldSessionPrimaryPos to ::getSessionPrimaryPosAmir Sarabadani2023-08-301-1/+1
| | | | | | | | While get... is not 100% correct here, it's much more understandable than yield. Bug: T275713 Change-Id: I7431dbb52b12437bc745553efe1ac937de4b04cf
* rdbms: Decouple ChronologyProtector from LBFAmir Sarabadani2023-08-291-17/+66
| | | | | | | | | | | | | | | | | | | | LBF carries a several attributes that it only need to init CP object, including $secret that might be confusing to reader what the secret is about. Also, LBF should not be really coupled to CP, at least not this much. Most stuff happen via calling CP functions passed by $lb object. Some callers of LBF methods also don't need LBF really, they need the CP object (e.g. ::disableChronologyProtection()) which in turn we can make ChronologyProtector a dedicated service (it only needs BagOStuff and secret from the LBF configuration, the rest are CLI mode or logger) and call those separately and slowly just inject it to LBF instead. This patch has already reduced the size of LBF by 10%. Bug: T275713 Change-Id: If5e34a372030238093b66c292a02d11e5933ff88
* rdbms: Drop partial disablement of CP and setting client id via headerAmir Sarabadani2023-08-241-12/+2
| | | | | | | None are used Bug: T275713 Change-Id: I17e450bd307e226f5f819997049180032b4a675c
* rdbms: Move two static methods of LBFactory to ChronologyProtectorAmir Sarabadani2023-08-011-0/+50
| | | | | | | | This clearly has nothing to do with LBF but is more related to CP. It's not used outside of core anywhere, all usages fixed. Bug: T326274 Change-Id: I6d07337fc2a9144c960073100d6078001283ace3
* rdbms: Remove LB::getReplicaResumePosAmir Sarabadani2023-08-011-1/+5
| | | | | | | | | | | | | It's only used in CP, CP doesn't really need replica positions. It only needs the master position, if there is no connection to master, it means either nothing happened on that section or connection got dropped (extremely rare) and regardless, it's not worth the complexity and specially a new method in LB that now almost every code piece can access. Bug: T342564 Bug: T326274 Change-Id: I0579e688a44d7a13d6b42622f7e88608f88e9841
* rdbms: Avoid making wasteful memcached calls in CPAmir Sarabadani2023-07-251-15/+9
| | | | | | | | | | | See https://phabricator.wikimedia.org/T314434#9039400 This is making one memcached call for every request while it doesn't need basically any of it. Bug: T314434 Co-Authored-by: Máté Szabó <mszabo@fandom.com> Change-Id: If7082f62f2d6cdedb7a505ac68ba79f08634034d
* rdbms: Fix line indent in ChronologyProtectorUmherirrender2023-03-171-11/+11
| | | | Change-Id: I9b48bed152f4e0be9607092863741ad68e765f54
* rdbms: clean up LoadBalancer/ChronologyProtector primary pos methodsAaron Schulz2023-02-091-6/+7
| | | | | | | | | | | | | | | | | | | | | | In LoadBalancer: * Make the "chronologyCallback" return the DBPrimaryPos and make loadSessionPrimaryPos() set the "waitForPos" more directly by calling setSessionPrimaryPosIfHigher(). Previously, it relied on the callback calling waitFor() to set the position as a side effect. * Remove redundant debug log entry in loadSessionPrimaryPos(). * Use type hints for waitFor()/waitForAll(). All callers already check this for before invocation. * Mark getReplicaResumePos() as @internal. In ChronologyProtector: * Update applySessionReplicationPosition() to return the position. * Rename applySessionReplicationPosition() to yieldSessionPrimaryPos() and stageSessionReplicationPosition() to stageSessionPrimaryPos() for for consistency LoadBalancer/DBPrimaryPos. Bug: T314434 Change-Id: I32aa784b424e7534047c9240e32fa5e0a2ac90b0
* Remove unused key variable from foreach loopsUmherirrender2022-09-211-1/+1
| | | | Change-Id: Id2d91e30a6f7cc4eb93427b50efc1c5c77f14b75
* rdbms: Limit CP warning message for T314434 to initial 10s windowTimo Tijhof2022-09-191-3/+12
| | | | | | | | | | | | | | | | Follows-up I7cc280261eaa. The data in Logstash shows some requests we expect (e.g. bots and login.wikimedia.org), but also a high volume of regular index.php requests from pageviews in the minute after a DB write. This is due to us storing the data in Memcached for 60s but only needing it for 10s (per the cookie). For now, focus the warning on cases where we know we're within the 10s window where a cookie could have persisted. If it's outside that range, then it's expected that we have no clientId coming in and don't need to apply CP. Bug: T314434 Change-Id: I56900622de9dac1738ce249fd3586d6e6bbb3338
* rdbms: Bump ChronologyProtector cache key versionTimo Tijhof2022-09-131-1/+1
| | | | | | | | | | | | In I78096d893a6a (96af469d8) we moved from v2 to v3, which uses a plain array instead of an object in serialised PHP. In Idd79aeb630bf (4e4a02d2d00) I simplified the code thinking the data shape remains the same. While this is true for what we send and receive, it ends up differently internally. Bug: T317606 Change-Id: I9cda0750f6d6662b7274808fb9b6de56a4edc2ff
* Merge "rdbms: Log warning if ChronologyProtector finds data under presumed ↵jenkins-bot2022-09-081-0/+27
|\ | | | | | | clientID"
| * rdbms: Log warning if ChronologyProtector finds data under presumed clientIDTimo Tijhof2022-09-021-0/+27
| | | | | | | | | | | | | | | | | | | | | | | | | | We'd like to remove the unconditional ChronologyProtector roundtrip form the majority of pageviews and other web requests. This should only be needed in the few seconds following a db-write action. Before we remove it, use this logging to confirm our theory that this never happens in practice, or that it happens only in cases where it is not needed. Bug: T314434 Change-Id: I7cc280261eaac14c770dc1c03321c2a7e155db19
* | rdbms: Simplify private marshal logic in ChronologyProtectorTimo Tijhof2022-09-061-13/+6
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Follows-up 96af469d87 (I78096d893a6) which introduced the marshal and unmarshal methods. * Remove code for pre-1.39 values. This is not needed since the cache key was bumped from v2 to v3, so there is no need to support older values. The data is highly ephemeral (stored for a few seconds only), and CP is invisible in practice, degrading gracefully if its data is reset every once in a while during a deployment. * Remove empty() checks in unmarshal. Per <https://www.mediawiki.org/wiki/CC/PHP>, these are an anti-pattern that introduces fear and doubt. We know the array, when found, was formatted by ourselves and has a FLD_POSITIONS key. Anything less is an error that we should get a silent runtime warning from in the logs. Document the falsey value (false), as expected from cache->get() when the key is not found. * Remove all checks from marshal. These are passed directly from mergePositions(). There is no falsey case possible. * Remove even the `!$data[FLD_POSITIONS]` checks that I had as replacement for empty() since the loop can simply no-op. This avoids having multiple non-failure termination paths in which the return statement is duplicated. Change-Id: Ib1c902c354517ced509a68ea0d442eba863cf850
* | rdbms: Use JSON to store ChronologyProtector position datadaniel2022-09-041-8/+42
|/ | | | | | | | | | | | | PHP serialization is brittle, so serialize DBPrimaryPos objects to JSON structures before writing them to the stash. BREAKING CHANGE: This adds to methods to the DBPrimaryPos interface. There are no known implementations of this interface outside MediaWiki core. There appears to be no good alternative to adding these methods to the interface. The breaking change was announced on wikitech-l on August 31. Bug: T316601 Change-Id: I78096d893a6a2c000c35673b78e36b52369560e5
* rdbms: rename "master" variables to "primary" in ChronologyProtectorAaron Schulz2022-08-291-15/+15
| | | | Change-Id: I0cd7043beea40ea1b5ec939bd1c7773f0c82a977
* rdbms: Remove obsolete cross-dc wait from ChronologyProtectorTimo Tijhof2022-08-201-63/+29
| | | | | | | | | | | | | | | | | | | | | | | | | | | | This was written for the scenerio where we would support writes on a primary DC request, immediately followed by a secondary DC request, and we'd then first use the CP cookie to wait-for Redis replication to communicate the MySQL position data, and then wait-for a MySQL replica to have caught up to that point. In the revised 2020 plan for Multi-DC (T254634), CP data is kept dc-local only, and we use a sticky-dc cookie and other traffic routing to remain in the primary DC for certain scenarios. CP remains as playing a rule to ensure consistency within the primary DC between the write request and the next few seconds of GET requests to ensure a replica is picked or waited-for to have caught up with prior writes in the same primary DC. One test needed to be updated as this commit exposed a happy accident where previously one key ('writeIndex') was unset in the mocked array, which was tolerated with the null-ish operator. Now that we use is_array only and then access the key, this undefined offset got exposed. Fix by removing the hardcoded logic and instead use $cp->key and $cp-mergePositions(), which also helps gain more confidence in the test by relying on their side-effects. Bug: T314434 Change-Id: I712e5aeb6712f4038011c3c48799e859aa40009d
* rdbms: Clean up file doc commentsTimo Tijhof2022-08-091-4/+1
| | | | | | | | | | | | | | | | | | | | | | | * Document what the LBFactory/LoadBalancer (sub)classes do. * Move useful descriptions from file doc to class doc so that we don't pointlessly maintain it in two places, and to allow for the file block to be consistently visually ignored instead of sometimes containing useful information. This patch removes various non-applicable or unrelated descriptions from the file block that were blindly copy-pasted, thus proving my point. It also removes two duplicate/clashing definition of the 'Database' defgroup. Keeping only the primary one in IDatabase.php. * Move ingroup tag to class block, as indexing the source file in Doxygen creates noise in the navigation sidebar and does not add add any benefit. * Fix dead-end reference to a sqlite/README. I'll rework this in a later patch. Change-Id: Iad0e67d766f4a7d5b97e7a471b49f2d8e60c506b
* rdbms: Move cpStash message to ChronologyProtector.phpTimo Tijhof2022-06-021-0/+4
| | | | | | | | | | | | | | | | | | * It's now only logged when we actually read or write to CP. This makes it less confusing to see the message in cases where CP isn't or can't be used, as it would claim to use it when really it isn't/wouldn't. * This was adding some overhead as it was the only non-wfDebug() thing I could find on all requests. Mainly noticable locally with mw-docker and writing to mw-www-debug.log. This helps a bit with T85805 and making dozens of load.php requests in debug mode less slow. * It duplicated the channel name outside its class instead of using DI. Change-Id: I0947b98d285f26ab5af8dd58a6dce2e3e0525d44
* Fix typos in comments (C-D)Siddharth VP2021-12-301-3/+3
| | | | Change-Id: I568fb93b53feb83f026d485136dd0d116d677f4f
* ILoadBalancer/ILBFactory: Rename hasOrMadeRecentMasterChanges to ↵James D. Forrester2021-09-021-1/+1
| | | | | | | hasOrMadeRecentPrimaryChanges Bug: T282894 Change-Id: I1d6130bcd09019f9e2de2974878902c7aafe8f0a
* docs: Change wording master to primary in comments and log textUmherirrender2021-09-011-10/+10
| | | | | Bug: T254646 Change-Id: I5379dc79be60c99f0a30f74e5d624f81fe6f921b
* Rename DB primary position interfaces to DBPrimaryPos and MySQLPrimaryPosJames D. Forrester2021-08-021-10/+10
| | | | | | | And replace all uses. Bug: T282894 Change-Id: I5222a8568255ac9fa5e2350e2264b8d2ee5eb968
* More master -> primary documentation and internal var renamingJames D. Forrester2021-07-151-3/+3
| | | | | Bug: T254646 Change-Id: I63cc8895033714bdfbf09aee933a8f0a43b387f3
* rdbms: improve ChronologyProtector comments and variable/method namesAaron Schulz2021-04-131-21/+27
| | | | | | Also added some basic type hints Change-Id: I5ddf18cba9517deb077625bc04c1351a8b49ea66
* rdbms: Improve ChronologyProtector documentationTimo Tijhof2021-03-101-3/+97
| | | | | | | Follows-up bd7cf4dce90006. Bug: T254634 Change-Id: I30e51df7eca64a6d86e5a90e15d5aacf7471ed31
* Add $wgChronologyProtectorStash and improve $wgMainStash commentsAaron Schulz2021-03-011-178/+257
| | | | | | | | | | | | | | | | | | | | | | | | | | 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
* Merge "In ChronologyProtector fix confusion between DB name and master name"jenkins-bot2020-06-031-7/+11
|\
| * In ChronologyProtector fix confusion between DB name and master nameTim Starling2020-06-011-7/+11
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The one caller of LBFactory::getChronologyProtectorTouched() was calling it with a domain ID instead of a master server name. Using the master server name to identify replication position makes sense for ChronologyProtector, since the replication position may be reset when the master changes, but it is an odd convention for LBFactory. So: * Rename all $dbName variables in ChronologyProtector to $masterName, for clarity. * Interpret the first parameter to ILBFactory::getChronologyProtectorTouched() as a database domain, to make its only existing caller work. * Change the first parameter to ChronologyProtector::getTouched() from a string to a strongly typed ILoadBalancer, by analogy with applySessionReplicationPosition(). This removes the master name concept from the public interface. * Mark ChronologyProtector @internal. The accessor in LBFactory is protected, so extensions can't use it anyway. This is just to clarify why I think changing the parameter to getTouched() without b/c is OK. * Add a simple test which mostly just checks that ChronologyProtector gets called with the correct parameters. It's an LBFactory/ChronologyProtector integration test. Change-Id: I3b4832b5a4d7410e94b9c51577b30b31d49bc63d
* | Remove terminating line breaks from debug messagesTim Starling2020-06-031-9/+9
|/ | | | | | | | | | | | | | | | | | | | | | | | | | | 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
* Fix more libs PSR12.Properties.ConstantVisibility.NotFoundReedy2020-05-161-3/+3
| | | | Change-Id: Ie514103708ed631b2c4e320441c7958662418197
* Coding style: Auto-fix MediaWiki.Classes.UnsortedUseStatements.UnsortedUseJames D. Forrester2020-01-101-1/+1
| | | | Change-Id: I94a0ae83c65e8ee419bbd1ae1e86ab21ed4d8210
* Add missing @param and @return to documentationUmherirrender2019-11-101-0/+1
| | | | Change-Id: Ibc5849cc8ea7e7c4eb30ded9c1cfa5f52187c377
* rdbms: remove reference to READ_LATEST in ChronologyProtector::shutdown()Aaron Schulz2019-08-221-4/+3
| | | | Change-Id: I34b2fa940475d5e5e81130221f61d782818a6d17
* rdbms: add ILoadBalancer::getReplicaResumePos methodAaron Schulz2019-07-121-1/+1
| | | | | | | | | | | | | | | | | | | This does what ChronologyProtector wants more rigorously and is better named. Not all replica servers will have the same position, so they should be compared to get the highest one. Simplify the getMasterPos() method to only return master positions as the other current callers do not need anything else. It will now connect if needed as well. This should make the method naming better. Reducing the use of replica derived replication postitions (instead of those from the master) makes certain GTID issues less likely, such as the matter of obsolete domain IDs. Increase general test coverage of LoadBalancer. Bug: T224422 Change-Id: I5420721ee339a24d09c26c38709500c7bbe797c2
* rdbms: add replica server counting methods to ILoadBalancerAaron Schulz2019-06-201-1/+1
| | | | | | | | | | | | | This is slightly more robust and makes the intent much clearer than random calling code checking getServerCount() all over the place. In addition, this yields better separation of concern. Also, cleanup the LoadBalancer constructer a bit and make the validation a bit stricter. Make some server index comparisons strict while at it. Change-Id: Icc1a35bd65c6862ff81faa3ab9b2aa7cafe29443
* rdbms: fix ChronologyProtector client IDs to not be emptyAaron Schulz2019-05-241-23/+35
| | | | | | | | | | Follow up to 8107fddd8fe3d5 Also improve debug logging and add some unit tests Bug: T223310 Bug: T223978 Change-Id: I35484385e4da2912bc10f5e8d2fb07cb1097347e
* rdbms: add "secret" parameter to ChronologyProtector to use HMAC client IDsAaron Schulz2019-04-191-4/+10
| | | | | | Also make $posIndex mandatory and clean up some IDE warnings in LBFactory. Change-Id: I9e686b670bc86eb377f14ca57a94e1aa3fd901d5
* Fix/suppress misc phan errors (#4)Kunal Mehta2019-04-051-1/+1
| | | | | | | | | | * RevisionStore: phan is confused by the array not being the callable * ApiContinuationManger: phan false positive * ChronologyProtector: fix doc block * MssqlBlob: phan doesn't like __construct returning things * WikiPage: Remove Revision typehint that doesn't match doc block Change-Id: I0661d97424d0ad6f7a3357542e79aceb6a4f6c64
* Safe replacement of a lot of `!count()` with `=== []`Thiemo Kreuz2019-01-151-1/+1
| | | | | | | | | | | | | | | This was originally a global search and replace. I manually checked all replacements and reverted them if (due to the lack of type hints) either null (that would be 0 when counted) or a Countable object can end in the variable or property in question. Now this patch only touches places where I'm sure nothing can break. For the sanity of the honorable reviewers this patch is exclusively touching negated counts. You should not find a single `!== []` in this patch, that would be a mistake. Change-Id: I5eafd4d8fccdb53a668be8e6f25a566f9c3a0a95
* Use PHP 7 '??' operator instead of '?:' (round 2)Bartosz Dziewoński2018-09-031-3/+2
| | | | | | A few issues have snuck in since I33b421c8cb11cdd4ce896488c9ff5313f03a38cf. Change-Id: Ib75470a7a3c19e2d48f498b396eee6ed733690e4
* rdbms: fix value of ChronologyProtector::POSITION_COOKIE_TTLAaron Schulz2018-07-111-1/+1
| | | | | | | | This was supposed to be 10 (~LoadBalancer::MAX_WAIT_DEFAULT) but ended up as 60 by mistake in 52af356cad (when the constant was added). Bug: T194403 Change-Id: Ie94949eebaafde2e0c4e2fcffabcb78866363a27
* rdbms: avoid redundant SPI logging fields in ChronologyProtector log entriesAaron Schulz2018-06-111-4/+8
| | | | | | Also renamed the field to a better name. Change-Id: I8bd13a01415a7518b5d9f7dc393b32848efebbf6
generated by cgit v1.2.3 (git 2.25.1) at 2025-04-23 16:06:01 +0000