Page MenuHomePhabricator

Future of flaggedtemplates feature
Closed, ResolvedPublic

Description

This feature (parsing only stable version of templates when parsing the articles) is extremely taxing on the infrastructure. It requires a table that usually ends up being the biggest table of that wiki (by a wide margin, e.g. arwiki one was bigger than revision table of English Wikipedia) and has contributed to the general outage of March 6, 2024. It adds a lot of writes to the every edit and given that FlaggedRevs is unmaintained (T185664), I don't see an easy way to fix this.

On top, this feature is not that critical. Many wikis rely on protecting templates instead (as there is no reason usually for an IP/newbie to edit them). Even so, it could be theoretically re-implemented in such a way that wouldn't be so taxing on the infra (e.g. instead of keeping track template revisions, just get the latest stable revision) .

Proposal: It's currently on by default, notify the communities that default will change and they can ask to be a holdout if they heavily rely on it. After a week or two, flip the default and clean up the tables from production. If there is no holdout there, just drop the feature/code altogether.

Event Timeline

So. The benefit of this feature being present is that the template editors are not necessarily tied too much by ‘template stability requirements’ since MediaWiki would show the previous version of the same template if really needed. The bot run can happen replacing one code with another for whatever reason, and in stabilised articles, it wouldn’t be a problem if the page was unreviewed for a while. The removal of this feature would affect the projects where FlaggedRevs is in ‘stable by default’ mode the most, but at the same time, those projects might have less of a backlog. In the case of ‘stable by default in some articles but not all’ setup, it would definitely affect the stabilised pages, potentially leading to some wrong results from a template change that wasn’t reflected in the stable version of the wikitext of the page. I would say that making the behaviour more performant or maybe affecting the articles where the template versioning is a problem is preferable to removing it outright (although I might be not thinking of the right thing in my comment, clarify if needed, I think this is how it is supposed to work?).

What you're referring is called "protect mode" (i.e. you protect some pages to be reviewed) and that's actually can't be enabled with flaggedtemplates feature at the same time. Protect mode disables a lot of FR features. At least that's my understanding of the code.

Also, I think flaggedtemplates doesn't even work with lua modules (does it?) making it less useful even.

What you're referring is called "protect mode" (i.e. you protect some pages to be reviewed) and that's actually can't be enabled with flaggedtemplates feature at the same time. Protect mode disables a lot of FR features. At least that's my understanding of the code.

If I understand correctly, we are talking about the feature that makes the top-page notice ‘This page has unreviewed templates or files: …’, right? Then the above I am not describing the protect mode, I am describing stabilisation. Protect mode is enWP-only (mostly) thing, other wikis usually do not use FR in the same way. If I am correct about this being related to that top-page notice, it works with any transclusions, but non-FR-ed namespaces (not modules) do not work correctly with it.

I think there was a misunderstanding.

The removal of this feature would affect the projects where FlaggedRevs is in ‘stable by default’ mode the most

That mode is called "protect mode" not the flaggedtemplates one. What I'm saying that protect mode is mutually exclusive with flaggedtemplates and as such it wouldn't affect them

Let's try to understand each other. Ruwiki uses FR in this way: by default any page is displayed to everyone in the last version. Very small number of pages (<0.1% of all articles) is "stabilized": it means, anons and users with this setting see last reviewed version of page (and, probably, templates and files on it). Also, when user uses popups on internal links (default WMF extension), pop-up always contains last reviewed version of page, regardless of user's status (anon or experienced user) and stabilized page or not. (This last behaviour may be need to be fixed to show last version on pop-ups, for example, if infobox template was renamed and bot-replaced in unpatrolled revisions of page, pop-up contain only old infobox name and nothing more instead of pop-up content).

If your proposal means that ruwiki anons will see last reviewed version of stabilized pages, but last version of templates (and files) on it, I don't object, as an experienced ruwiki user.

If your proposal means that ruwiki anons will see last reviewed version of stabilized pages, but last version of templates (and files) on it, I don't object, as an experienced ruwiki user.

Indeed, that's the proposal.

Change 1012702 had a related patch set uploaded (by Ladsgroup; author: Amir Sarabadani):

[mediawiki/extensions/FlaggedRevs@master] Use latest stable revision of templates instead of tracking them

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

One other thing I thought of: this would make it so the main pages can no longer expect that the templates included on them would be from stable version. Which might complicate some things in the community side. I agree, I guess, that the impact on the stabilised pages (I am not sure why you keep calling them protect mode?) is less pronounced since it’s not like the big templates on those pages are unprotected. It might be an interesting alternative to limit this feature to pages not protected to a certain level (to help the issue with the main pages), but that might be a bit complicated.

Change 1012702 merged by jenkins-bot:

[mediawiki/extensions/FlaggedRevs@master] Use latest stable revision of templates instead of tracking them

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

Change 1013022 had a related patch set uploaded (by Ladsgroup; author: Amir Sarabadani):

[operations/puppet@production] mediawiki: Get rid of purge flaggedrevs

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

In commit description you provide another info: stable (=last reviewed, not last) template versions still will be used in stabilized articles, and before that stabilized articles used not last reviewed versions of templates, but versions of templates, that was used on page in time of last reviewing of this page. If it is true, I support this decision even more, I didn't know about this overcomplicated mechanism.

One other thing I thought of: this would make it so the main pages can no longer expect that the templates included on them would be from stable version. Which might complicate some things in the community side. I agree, I guess, that the impact on the stabilised pages (I am not sure why you keep calling them protect mode?) is less pronounced since it’s not like the big templates on those pages are unprotected. It might be an interesting alternative to limit this feature to pages not protected to a certain level (to help the issue with the main pages), but that might be a bit complicated.

Indeed. I have been thinking about this feature and the use on main page is the only important usecase for it.

In commit description you provide another info: stable (=last reviewed, not last) template versions still will be used in stabilized articles, and before that stabilized articles used not last reviewed versions of templates, but versions of templates, that was used on page in time of last reviewing of this page. If it is true, I support this decision even more, I didn't know about this overcomplicated mechanism.

Yeah. I want to mention that Russian Wikipedia is still on CURRENT mode which is the latest revision of the templates, once the patch land in production and we are sure it doesn't cause major issues, I'd be okay to switch Russian Wikipedia back to STABLE mode for templates but if you don't need it, you can stay with CURRENT since even with my patch that mode still adds non-negligible overhead (is it big enough to cause outages again? will see)

Are you serious?

Proposal: It's currently on by default, notify the communities that default will change and they can ask to be a holdout if they heavily rely on it. After a week or two, flip the default and clean up the tables from production. If there is no holdout there, just drop the feature/code altogether.

Instead of this, you’ve removed the feature altogether, without notifying communities (at least I didn’t come across any communication on huwiki) and without input from any wiki except for ruwiki (which isn’t really relevant as they apparently don’t really rely on FlaggedRevs to control what readers see). This is very far from your proposal. I was strongly against the removal in 2021, and I’m strongly against it in 2024 as well. Of course, if ruwiki doesn’t need it, you can disable it there to reduce the database size, and you may contact arwiki as well if they’re okay with disabling it. But breaking things because of database size issues and not even trying to fix them is not very welcoming.

Please check the patch again. The patch makes it when the configuration is set to STABLE. It takes the most recent stable revision of the template. It doesn't remove the feature altogether. I tested it locally even to make sure it works.

If you're not sure, check that the logic for that part still exists: https://gerrit.wikimedia.org/g/mediawiki/extensions/FlaggedRevs/+/master/includes/backend/FRInclusionManager.php#128 and https://gerrit.wikimedia.org/g/mediawiki/extensions/FlaggedRevs/+/master/includes/backend/FlaggedRevision.php#386 and more places.

It simply stops using the revision id of the time of review but still uses the most recent stable version of the template (obtained from flaggedrevs table)

To test, I made a template that has parent rev that outputs "stable" and an unreviewed revision that outputs unstable:

grafik.png (171×1 px, 29 KB)

Using it in a page shows this:

grafik.png (171×1 px, 25 KB)

But logged out it's this (and purged many times just to make sure):
grafik.png (171×1 px, 14 KB)

I'm not saying I haven't broken the feature, I might have accidentally removed that part of it but so far my tests show that the feature is not removed and just got simpler (not the revid of the time of edit but the latest stable revid). Did I miss anything here?

I’ve indeed overseen the word stable in the commit message, sorry about that.

However, what I’ve said about the lack of notification, consultation and holdout, still stands.

Also, even if technically (hopefully) you didn’t break anything, you definitely broke some people’s expectations: previously, if I approved a page, I approved what I saw. This may not be true anymore. For example, if a template’s parameters have changed and at the same transcluding articles were updated accordingly (both by an editor without autoreview right), reviewing the article will now break it for readers (but not the reviewer, who sees the latest version rather than the latest reviewed one).

The communication to the communities has happened with T277883: Drop all low-use and unused features of FlaggedRevs to make it more maintainable, we asked people to subscribe to that ticket if they want to keep track of the changes and many people did. And years ago, I was clear that we have to remove major parts of this extension to be able to keep it in production (T277883#6931052).

The difference between latest stable revision and the revision of template when the article was reviewed is so small and subtle that several people in this ticket confused them including you. Because of that, I'm keen to let it ride with the train and see if people even notice the difference.

This is a follow up to an incident where all wikis went down and were not accessible for 23 minutes. FlaggedRevs is not maintained and not maintainable either (without major removal of features) and if continues to cause outages like this, we will have to undeploy it. The above change is a compromise between "keeping as much as functionality as possible for users" and "keeping the site up" and as I said before, the choice is not between "keeping flaggedtemplates" and "not keeping flaggedtemplates". It's between "getting rid of flaggedtemplates" and "undeploying flaggedrevs altogether".

Change #1014028 had a related patch set uploaded (by Ladsgroup; author: Amir Sarabadani):

[operations/puppet@production] mediawiki: Absent FR purge systemd timer

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

Change #1014028 merged by Ladsgroup:

[operations/puppet@production] mediawiki: Absent FR purge systemd timer

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

Change #1013022 merged by Ladsgroup:

[operations/puppet@production] mediawiki: Get rid of purge flaggedrevs

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

This seems to have started to affect Russian Wikinews where Flaggedrevs are heavily used.

Can you please review Russian Wikinews and possibly shrink the mechanism where its possible? It is unneeded in many cases and causes excessive dullwork.

Example: the article https://ru.wikinews.org/wiki/Забастовка_Википедии_на_русском_языке_завершена — there is a blue upper notice "Изменения шаблонов/файлов этой версии ожидают проверки. Стабильная версия была проверена 16 февраля 2024.". From now, I can't do anything with it: no matter how I push buttons and purge cache, it does nothing and writes nothing. It should be better switched off completely in this case and so on.

The issue for ruwikinews seems to be coming from one heavily used template that's waiting for review:

[email protected](ruwikinews)> SELECT  lt_namespace,lt_title  FROM `templatelinks` JOIN `linktarget` ON ((tl_target_id=lt_id)) LEFT JOIN `page` ON ((page_namespace = lt_namespace AND page_title = lt_title)) JOIN `flaggedpages` ON ((fp_page_id = page_id))   WHERE tl_from = 19061 AND (fp_pending_since IS NOT NULL OR fp_stable IS NULL)  ;
+--------------+----------+
| lt_namespace | lt_title |
+--------------+----------+
|           10 | Cite_web |
+--------------+----------+
1 row in set (0.002 sec)

If you review that, it should fix the issue.

I think the biggest missing feature now is to have somewhere list all transcluded templates waiting for review for a given page (and link to that at the top of the page). I'd say in action=info would be okay.

Change #1023521 had a related patch set uploaded (by Ladsgroup; author: Amir Sarabadani):

[mediawiki/extensions/FlaggedRevs@master] Add action=info hook handler listing transcluded pages pending review

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

Ladsgroup moved this task from Triage to In progress on the DBA board.

Change #1023521 merged by jenkins-bot:

[mediawiki/extensions/FlaggedRevs@master] Add action=info hook handler listing transcluded pages pending review

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

With this change merged (Thanks James!), this wraps the future of flaggedtemplates. FR queries are much safer now. If Russian Wikipedia wants to re-enable it, they can but if they don't really use that feature, then it's better not to enable it since it still has somewhat of an overhead.

If you review that, it should fix the issue.

Thank you, but I still encounter it. The newest article: https://ru.wikinews.org/wiki/Официальный_сайт_Нижегородской_области_перешёл_на_свободную_лицензию

It contains notification above: "Изменения шаблонов/файлов этой версии ожидают проверки. Стабильная версия была проверена 30 апреля 2024."

I cannot remove it no matter what I do. I pressed "Подтвердить версию" many times -- no effect (I have admin rights).

It says a template is unchecked. But it does not say WHICH one. Just appears for no reason. Other articles are also affected.

If you review that, it should fix the issue.

Thank you, but I still encounter it. The newest article: https://ru.wikinews.org/wiki/Официальный_сайт_Нижегородской_области_перешёл_на_свободную_лицензию

It contains notification above: "Изменения шаблонов/файлов этой версии ожидают проверки. Стабильная версия была проверена 30 апреля 2024."

I cannot remove it no matter what I do. I pressed "Подтвердить версию" many times -- no effect (I have admin rights).

It says a template is unchecked. But it does not say WHICH one. Just appears for no reason. Other articles are also affected.

It's because the patch is not deployed yet. It will be deployed by late Wednesday (in your wiki) and you will be able to see the list of templates needing review then by checking action=info of the page.

@ssr now you can see see which template is waiting for review:

Thank you very much! It appears to work now.

I noticed the change in de.Wikipedia now. I understand that it was necessary to make a change. However, I wonder if there is a solution where the list of transcluded pages can be found more intuitively. In de.Wikipedia you have to scroll to the bottom of page information ("Seiteninformationen") to find the list under "Transkludierte Seiten warten auf Überprüfung". How are users supposed to know this?

I would appreciate a comment explaining what is possible from the technical perspective here. Maybe a more user-friendly solution could be implemented easily.

We can anchor that link to subsectio of the page (via adding #mw-pageinfo-header-properties) Would that help in your case? e.g. https://de.wikipedia.org/w/index.php?title=Rosy_Wertheim&action=info#mw-pageinfo-header-properties

This would still require scrolling below the full list of included templates, which can be quite long like in this case with 57 included templates:
https://de.wikipedia.org/w/index.php?title=Bundesministerium_f%C3%BCr_Gesundheit_(Deutschland)&action=info#mw-pageinfo-header-properties

If the list of templates needing review would be displayed in front of the full list of included templates the solution with an anchor would work well (no need of scrolling for users).

Seems like there is a #mw-pageinfo-templates on the table row in there so there’s nothing that stops the devs from putting an ID to FlaggedRevs-generated list and linking to that table row.

Which link(s) are we proposing be altered to anchor to #mw-pageinfo-templates? Are these links on a special page or somewhere else?

Change #1036694 had a related patch set uploaded (by Ladsgroup; author: Amir Sarabadani):

[mediawiki/extensions/FlaggedRevs@master] Anchor the link to transcluded pages waiting for review

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

Change #1036694 merged by jenkins-bot:

[mediawiki/extensions/FlaggedRevs@master] Anchor the link to transcluded pages waiting for review

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

@Kallichore With the next train, you will have a better view of pages needing review.

Thanks, I had a look at the changes and I like the solution for pages needing review. To my knowledge it works well in de.Wikipedia now.