Change 745935 had a related patch set uploaded (by Jdlrobson; author: Jdlrobson):

[operations/mediawiki-config@master] Default commons search experience is MediaSearch

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

Change 745935 merged by jenkins-bot:

[operations/mediawiki-config@master] Default commons search experience is MediaSearch

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

Mentioned in SAL (#wikimedia-operations) [2021-12-14T00:36:22Z] <tgr@deploy1002> Synchronized wmf-config/CommonSettings.php: Config: [[gerrit:745935|Default commons search experience is MediaSearch (T297484)]] (duration: 00m 56s)

In T295616#7566964, @gerritbot wrote:

Change 744899 merged by jenkins-bot:

[mediawiki/core@master] Add SpecialPage::newSearchPage to replace Skin::setSearchPageTitle

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

This patch replaces Skin::setSearchPageTitle with a completely different soft-deprecated implementation. I am not entirely sure, but I suspect this might be a DBPerformance problem that we'd have to block the train on if it goes out as-is. Please review the following for anything that might warrant adjustments if the information is new to you:

• The patch changes the method from something that makes no assumption about intent, merely applying a value to the current web response, to something that assume intent to remember a sticky preference, establishes a primary DB connection on all page views, and then makes a DB write to mutate a preference on the current user's behalf.
• I understand MediaSearch is the only caller, however it is problematic for that as well given it unconditionally called from a hook called on all pageviews. It seems like it would otherwise easily cause millions of database queries while ungracefully backfilling values for all registered users, and not to mention the load on the primary DB and DBPerf warnings from a GET request.
• I understand there is already a change deployed to wmf-config that changes the default value to MediaSearch on Commons. It is unclear to me whether that eliminates all DB-writes from the setSearchPageTitle code, does it?
• If that mitigation avoids all writes, then it would be less risky opertionally to just remove this method rather than leave something behind that's a breaking change to third-parties anyway. Or perhaps leave it as empty dummy method that emits hard-deprecation, so as to remove the functionality but let it gracefully degrade to unchanged Special:Search during the intermediary release.

Based on a quick code search, I suspect it does not avoid all writes since MediaSearch did not invariably call this, but allows users to change their preference, which means it would still cause DB writes on GET requests after lazily discovering that the value is a mismatch, is that right? Based on how we've handled numerous other migrations like this over the years, I'd suggest something like the following, through a minimal patch backported before the next train:

1. Remove problematic DB-writing code from setSearchPageTitle() in core, leaving it as soft-deprecated no-op.
2. Add setOption() for the new core preference in the MediaSearch code, near where the old option is currently set. This is "write new" for forward-compat.
3. Offline, migrate existing overrides in the commonswiki database via mwmaint CLI. This prepares the DB for "read new", for core to read after the next train rolls out.

Then after the train has rolled out, redundant code in MediaSearch can be removed.

The patch changes the method from something that makes no assumption about intent, merely applying a value to the current web response, to something that assume intent to remember a sticky preference, establishes a primary DB connection on all page views, and then makes a DB write to mutate a preference on the current user's behalf.

TLDR: Only Wikimedia Commons is impacted. A database write will occur at most one time for every single user if their existing "sdms-specialsearch-default" preference is set to 0. My understanding going into this was user's with "sdms-specialsearch-default" set to 0 on Commons is low meaning this method will seldom be called.

• Wikimedia Commons is the only project impacted.
• The new preference is an API only preference. There is no user interface to change the new value.
• I've just backported a change which sets the default value of the preference to Special:MediaSearch Wikimedia Commons users. This can be overriden by the MediaSearch extension:
  • MediaSearch is currently wired up to use the preference "sdms-specialsearch-default"

• The code is set to migrate these preferences to the new "search-special-page" preference. If "sdms-specialsearch-default" is set to 1 it will call Skin::setSearchPageTitle which will lead to no database write.
• However, If "sdms-specialsearch-default" is set to 0 it will call Skin::setSearchPageTitle which will lead to a database write on their first page view after the change ( GET). On the subsequent page view there should be no database write.
• Skin::setSearchPageTitle will only access and write to the database if the Title is different to the default [1] e.g. when the user has opted out of Special:MediaSearch.

Happy to make adjustments to this tomorrow, to make the current patch less risky, if after all the above we do deem this risky. @Cparle let me know what you want to do.

[1] https://gerrit.wikimedia.org/g/mediawiki/core/+/1a1c55c5f38b6ed348a4f38b1d6227913a3d4436/includes/skins/Skin.php#2675

Change 747138 had a related patch set uploaded (by Jdlrobson; author: Cparle):

[mediawiki/extensions/MediaSearch@master] Update the way the search interface is set

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

Change 747189 had a related patch set uploaded (by Jdlrobson; author: Jdlrobson):

[mediawiki/core@master] Remove migration script

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

One option is to backport https://gerrit.wikimedia.org/r/c/mediawiki/core/+/747189 today while we finalize https://gerrit.wikimedia.org/r/747138. Let me know if that makes sense.

I'm trying to catch up. Correct me if I'm wrong, but here's what's in flight:

1. https://gerrit.wikimedia.org/r/c/mediawiki/core/+/747189 would remove the GET write while keeping functionality intact for now (it just won't write the new preference format)
2. On commonswiki, MediaSearch has to be made the default under the new approach (this has not yet happened, AFAICT)
3. https://gerrit.wikimedia.org/r/c/mediawiki/extensions/MediaSearch/+/747138 will start to use the new preference & the old code path becomes useless anyway
4. ^ we need to run the migration script in above patch at around the time (or once shortly before and once again shortly after) this code gets deployed so we don't lose people's preferences

Makes sense to me and patches look good. I'm leaving +2 for whoever (@Jdlrobson?) is coordinating these changes, and until @Krinkle has had a chance to review once more.

Number 2 has been done already, see https://gerrit.wikimedia.org/r/c/operations/mediawiki-config/+/745935/ (merged)

Apart from that I think that's all accurate. I don't know who's co-ordinating either though :) I can do it if you don't want to @Jdlrobson

I think the only timely thing to do is 1 (the core patch). I can't merge the core patch but am happy to backport it if someone confirms it doesn't interfere with MediaSearch and +2s.

@Jdlrobson - confirmed it doesn't interfere with MediaSearch, and +2ed

Change 747483 had a related patch set uploaded (by Jdlrobson; author: Jdlrobson):

[mediawiki/core@wmf/1.38.0-wmf.13] Remove migration script

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

Change 747483 merged by jenkins-bot:

[mediawiki/core@wmf/1.38.0-wmf.13] Remove migration script

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

Mentioned in SAL (#wikimedia-operations) [2021-12-15T20:52:09Z] <hashar@deploy1002> Synchronized php-1.38.0-wmf.13/includes/skins/Skin.php: Remove migration script - T297484 (duration: 01m 06s)

Change 747189 merged by jenkins-bot:

[mediawiki/core@master] Remove migration script

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

Change 745339 had a related patch set uploaded (by Jdlrobson; author: Jdlrobson):

[mediawiki/core@master] Hard deprecate Skin:setSearchPageTitle

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

Change 751836 had a related patch set uploaded (by Cparle; author: Cparle):

[mediawiki/extensions/MediaSearch@wmf/1.38.0-wmf.16] Update the way the search interface is set

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

Change 747138 merged by jenkins-bot:

[mediawiki/extensions/MediaSearch@master] Update the way the search interface is set

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

Yay! Let me know when the migration is done and it's safe to merge https://gerrit.wikimedia.org/r/c/mediawiki/core/+/745339.

Backport scheduled for Mon https://wikitech.wikimedia.org/wiki/Deployments#Monday,_January_10 will ping you afterwards

Change 752653 had a related patch set uploaded (by Cparle; author: Cparle):

[mediawiki/extensions/MediaSearch@master] Updated maint script to use fewer queries

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

Change 752653 merged by jenkins-bot:

[mediawiki/extensions/MediaSearch@master] Updated maint script to use fewer queries

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

Change 752279 had a related patch set uploaded (by Cparle; author: Cparle):

[mediawiki/extensions/MediaSearch@wmf/1.38.0-wmf.16] Updated maint script to use fewer queries

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

Change 752279 abandoned by Cparle:

[mediawiki/extensions/MediaSearch@wmf/1.38.0-wmf.16] Updated maint script to use fewer queries

Reason:

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

Change 752679 had a related patch set uploaded (by Cparle; author: Cparle):

[mediawiki/extensions/MediaSearch@master] Update the way the search interface is set

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

Change 752680 had a related patch set uploaded (by Cparle; author: Cparle):

[mediawiki/extensions/MediaSearch@master] Updated maint script to use fewer queries

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

Change 752680 abandoned by Cparle:

[mediawiki/extensions/MediaSearch@master] Updated maint script to use fewer queries

Reason:

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

Change 752679 abandoned by Cparle:

[mediawiki/extensions/MediaSearch@master] Update the way the search interface is set

Reason:

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

Change 752694 had a related patch set uploaded (by Cparle; author: Cparle):

[mediawiki/extensions/MediaSearch@wmf/1.38.0-wmf.16] Update the way the search interface is set

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

Change 752695 had a related patch set uploaded (by Cparle; author: Cparle):

[mediawiki/extensions/MediaSearch@wmf/1.38.0-wmf.16] Updated maint script to use fewer queries

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

Change 752698 had a related patch set uploaded (by Cparle; author: Cparle):

[mediawiki/extensions/MediaSearch@master] Updated maint script to use fewer queries

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

Change 752695 abandoned by Cparle:

[mediawiki/extensions/MediaSearch@wmf/1.38.0-wmf.16] Updated maint script to use fewer queries

Reason:

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

Change 752694 abandoned by Cparle:

[mediawiki/extensions/MediaSearch@wmf/1.38.0-wmf.16] Update the way the search interface is set

Reason:

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

Change 752698 abandoned by Cparle:

[mediawiki/extensions/MediaSearch@master] Updated maint script to use fewer queries

Reason:

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

Change 752701 had a related patch set uploaded (by Cparle; author: Cparle):

[mediawiki/extensions/MediaSearch@wmf/1.38.0-wmf.16] Updated maint script to use fewer queries

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

Change 751836 merged by jenkins-bot:

[mediawiki/extensions/MediaSearch@wmf/1.38.0-wmf.16] Update the way the search interface is set

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

Change 752701 merged by jenkins-bot:

[mediawiki/extensions/MediaSearch@wmf/1.38.0-wmf.16] Updated maint script to use fewer queries

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

Change 752776 had a related patch set uploaded (by Urbanecm; author: Urbanecm):

[mediawiki/extensions/MediaSearch@wmf/1.38.0-wmf.16] Revert \"Updated maint script to use fewer queries\"

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

Change 752776 merged by Urbanecm:

[mediawiki/extensions/MediaSearch@wmf/1.38.0-wmf.16] Revert \"Updated maint script to use fewer queries\"

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

Change 752777 had a related patch set uploaded (by Urbanecm; author: Urbanecm):

[mediawiki/extensions/MediaSearch@wmf/1.38.0-wmf.16] Revert \"Update the way the search interface is set\"

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

Change 752777 merged by Urbanecm:

[mediawiki/extensions/MediaSearch@wmf/1.38.0-wmf.16] Revert \"Update the way the search interface is set\"

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

Change 753060 had a related patch set uploaded (by Cparle; author: Cparle):

[mediawiki/extensions/MediaSearch@wmf/1.38.0-wmf.17] Updated maint script to use fewer queries

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

Change 753060 merged by jenkins-bot:

[mediawiki/extensions/MediaSearch@wmf/1.38.0-wmf.17] Updated maint script to use fewer queries

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

Change 745339 merged by jenkins-bot:

[mediawiki/core@master] Hard deprecate Skin:setSearchPageTitle

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

Change 753757 had a related patch set uploaded (by Jdlrobson; author: Jdlrobson):

[mediawiki/core@master] Hard deprecate Skin:setSearchPageTitle (attempt 2)

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

Change 753757 merged by jenkins-bot:

[mediawiki/core@master] Hard deprecate Skin:setSearchPageTitle (attempt 2)

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

I think this is done now?

Think so yes \o/