Anyone got a link to a Gerrit patch showing the suggested fix?

In T348573#9245808, @Novem_Linguae wrote:

Anyone got a link to a Gerrit patch showing the suggested fix?

https://gerrit.wikimedia.org/r/c/mediawiki/extensions/UrlShortener/+/963293 ?

ArticleFeedbackv5 isn't currently WMF-deployed, though who knows, perhaps it will once again be someday. :-) In the meanwhile, I definitely could use a helping hand with this...I wished to implement this change in AFTv5 in a non-BC-breaking way but the code turned out to be somewhat convoluted and I don't have a 1.41+ test setup to test out this part. What I can definitely say is that everything I tried out ended up breaking things like Special:Contributions (!) and such, so, yeah. :-/

Is there a patch I can take a look at?

I think simplest way to keep BC would be to use the MediaWikiServices hook to look at the configuration, and if $wgVirtualDomainsMapping['articlefeedback'] is not set but the legacy cluster/DB variables are set, copy that latter to the former.

Thanks for the swift replies, @Ladsgroup and @Tgr; much appreciated! :) I didn't submit a patch to gerrit as there was essentially nothing submission-worthy; quite the opposite, I had to revert my futile attempts to unbreak Special:Contributions so that I could test out some different extensions which hook into Special:Contributions as well.

So in ArticleFeedbackv5 there are three classes which define (and also use) a getLB method: DataModelBackendLBFactory (in /data/DataModelBackend.LBFactory.php), ArticleFeedbackv5BackendLBFactory (extends the previously mentioned class; in /includes/ArticleFeedbackv5BackendLBFactory.php) and finally ArticleFeedbackv5Utils (in /includes/ArticleFeedbackv5Utils.php).
Additionally the to-be-deprecated global variable $wgArticleFeedbackv5Cluster is used in three maintenance scripts in addition ot the ArticleFeedbackv5BackendLBFactory.php file, and I wouldn't have the slightest clue on how to update the waitForReplication calls in these: /maintenance/archiveFeedback.php, /maintenance/legacyToShard.php, and /maintenance/setArchiveDate.php.

One main issue I had was that the DataModelBackendLBFactory class has an allowCache method, which calls the class' getLB method and calls laggedReplicaUsed on whatever getLB returns. AIUI (please do correct me if I'm wrong) this would be problematic in the future if and when extensions shouldn't be using LBFactory the way e.g. AFTv5 currently does. When I was trying to refactor getLB and friends, the laggedReplicaUsed call ended up fataling and breaking Special:Contributions IIRC. I'd guess that all the other getLB uses would be easier to refactor, but that one really ended up confusing me.

@ashley sorry for the very slow response. waitForReplication can be replaced with IConnectionProvider::commitAndWaitForReplication. laggedReplicaUsed has no straightforward replacement; probably just ignore it and cache unconditionally? AFTv5 is not used on Wikimedia and it probably won't matter for smaller websites.

Change #1032800 had a related patch set uploaded (by Jack Phoenix; author: Jack Phoenix):

[mediawiki/extensions/ArticleFeedbackv5@master] Baby step(s) towards using virtual database domains - swap out laggedReplicaUsed usage

https://gerrit.wikimedia.org/r/1032800

I added a virtual domain parameter to Template:Extension on mediawiki.org. In theory it could be added automatically to the relevant pages via Tool-extjsonuploader, that's not done for now.

Change #1032800 merged by Jack Phoenix:

[mediawiki/extensions/ArticleFeedbackv5@master] Baby step(s) towards using virtual database domains - swap out laggedReplicaUsed usage

https://gerrit.wikimedia.org/r/1032800

Change #1063257 had a related patch set uploaded (by Jack Phoenix; author: Jack Phoenix):

[mediawiki/extensions/ArticleFeedbackv5@master] Use virtual database domain

https://gerrit.wikimedia.org/r/1063257

Added GlobalPreferences to the list after an IRC chat with Reedy.

Is this task only for extensions that use x1? If so, we can remove a bunch from the list such as PageTriage and SecurePoll. If not, then perhaps we should re-title this task to have a broader/different title.

In T348573#11354713, @Novem_Linguae wrote:

Is this task only for extensions that use x1? If so, we can remove a bunch from the list such as PageTriage and SecurePoll. If not, then perhaps we should re-title this task to have a broader/different title.

Virtual domains simplify look ups from a central database regardless of whether it uses x1 or not. For example, centralauth uses a dedicated db in s7 and switched to virtual domains. I remember a couple extensions that also used metawiki's db hard configured and they switched to virtual domains too (central notice for example)

In other words, I think anything doing central lookup could benefit from it but not any cross wiki look up. I don't know how PageTriage does it

PageTriage and SecurePoll don't use central databases. Does that mean I should take them off the checklist?

CheckUser is crossed off, but now has central databases for its cuci_ tables. Maybe that one should be un-crossed-off?

I have some other things I can check once I make sure I understand this ticket correctly. Here's a list of the WMF "central databases" I can think of: x1 wikishared, x1 flowdb, x1 cognate_wiktionary, s7 centralauth. I could go look at the tables in each of those 4 databases and map them back to extensions, then make sure those extensions are on the checklist above, and then remove any extensions that are on the checklist but that weren't found during my investigation. Would that be helpful?

The thing is that many code paths rely on core db of another wikis as the central database. For example, bot_passwords that use virtual domain, uses core db of metawiki for the central database look up so we can't work backwards for x1 :/