Page MenuHomePhabricator

Add reply links to the parser cache
Open, Needs TriagePublic

Description

Currently reply links are added in the onOutputPageBeforeHTML hook, and therefore not cached for logged-in users. While the median time spend in this hook is <50ms, on longer pages this can take a few hundred ms[1].

(At https://www.mediawiki.org/wiki/Topic:Vwtdrzx2hszbdul1 someone reported a page where adding reply links takes about 1.5 s.)

We should be able to store the output in the parser cache, so that this method is called much less frequently.

We will need to split the cache on user language as the reply link itself uses the user language.

Done


  1. https://graphite.wikimedia.org/?width=586&height=308&target=MediaWiki.discussiontools.addReplyLinks.mean

Testing

Environment

  • Beta cluster

Behaviors

  • Check that the availability of the tool is unaffected by this patch, that it can be enabled/disabled via preferences or beta feature
  • Check that the reply tool still loads correctly when enabled

Related Objects

Event Timeline

Esanders updated the task description. (Show Details)
Esanders added subscribers: matmarex, cscott.

We will need to split the cache on user language as the reply link itself uses the user language.

We might be able to avoid this, by caching the page with some magic placeholder in place of the "reply" text, and only replacing it in the ParserOutputPostCacheTransform hook. The section edit links are handled like this in MediaWiki core (see code in ParserOutput::getText).

We need to split on the reply links being enabled or not though.

As discussed during today's team meeting, ideally this will be worked on the week of 3-Dec-2020.

Change 641837 had a related patch set uploaded (by Esanders; owner: Esanders):
[mediawiki/extensions/DiscussionTools@master] Store reply links in the parser cache

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

CC @LSobanski . We are planning to merge this for next week's train. It will split the parser cache on talk pages for user who have this feature enabled.

@Esanders could you help me understand what impact this is expected to have on the parser cache databases?

@LSobanski we are adding a new parser cache option, which can only ever be set on discussion pages.

I suppose in the worst case, where every talk page has been visited by someone with this feature enabled, we would be doubling the cache size for talk pages. In practice it will less than that as many talk pages receive little to no traffic from real users.

In the long term, should the tool become very widely used, we may consider only generating one version of the pages with the reply links, and hiding them with CSS for the minority that have the feature disabled.

@Esanders is this something that can be disabled/expired if needed?
Parsercache is very stable in terms of disk space usage, and any small variation on the new keys and expiration time can lead to the hosts filling up quite quickly.
This is one of the disk usage graphs for the last month

As you see there are pretty much no variations of the disk usage as everything is pretty aligned, so it would be great to know:

  • what if once this is deployed it causes disk issues on our parsercache infra? can this be rolled back/disabled/keys deleted
  • Could your team monitor this once this is deployed and for a few days?:

https://grafana.wikimedia.org/d/000000377/host-overview?viewPanel=12&orgId=1&refresh=5m&var-server=pc1007&var-datasource=thanos&var-cluster=mysql&from=now-24h&to=now
https://grafana.wikimedia.org/d/000000377/host-overview?viewPanel=12&orgId=1&refresh=5m&var-server=pc1008&var-datasource=thanos&var-cluster=mysql&from=now-24h&to=now
https://grafana.wikimedia.org/d/000000377/host-overview?viewPanel=12&orgId=1&refresh=5m&var-server=pc1009&var-datasource=thanos&var-cluster=mysql&from=now-24h&to=now

  • When would this be deployed?
  • Will this be deployed to all wikis at once or will this be enabled slowly? I would request the latter, so we can see how this affects our servers disk growth.

Thanks

@Esanders is this something that can be disabled/expired if needed?

  • what if once this is deployed it causes disk issues on our parsercache infra? can this be rolled back/disabled/keys deleted

It's pretty easy to rollback this feature, we will also have a flag so it can disabled via config.

Every entry will have dtreply=1 in the key.

  • Could your team monitor this once this is deployed and for a few days?:

Sure.

  • When would this be deployed?

The earliest now would be next week's train (wmf 28)

  • Will this be deployed to all wikis at once or will this be enabled slowly? I would request the latter, so we can see how this affects our servers disk growth.

DiscussionTools is only enabled by default on ar/cs/huwikis, and is a beta feature on ~250 other Wikipedias (but not enwiki). The new cache key will only get used if someone with the feature enabled visits the talk page, so by its nature it will be a gradual rollout.

Thanks for the reply, one more question regarding this comment:

DiscussionTools is only enabled by default on ar/cs/huwikis, and is a beta feature on ~250 other Wikipedias (but not enwiki). The new cache key will only get used if someone with the feature enabled visits the talk page, so by its nature it will be a gradual rollout.

The plan I would assume is to enable it everywhere at some point? (including big wikis like enwiki and commonswiki)
Thank you

ok - thanks. Let's monitor how this first deployment goes and the effect on the disk growth.
Thanks for all the answers!

Change 641837 merged by jenkins-bot:
[mediawiki/extensions/DiscussionTools@master] Store reply links in the parser cache

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

ppelberg updated the task description. (Show Details)

@Marostegui This finally went out in wmf30. I don't see any significant change in disk usage, but I don't know what you consider a significant change :)

Thanks @Esanders I will give it a week and if nothing shows up, I will close this.

So far so good, going to keep looking during this week and if it still good, will close this.