Page MenuHomePhabricator

HtmlPageLinkRendererBeginHookHandler creates a LanguageFallbackChain when it may not be needed
Closed, ResolvedPublic

Description

related: P10270
In conjunction with T243726 this can cause pain
Example: https://grafana.wikimedia.org/d/lqE4lcGWz/wanobjectcache-key-group?orgId=1&var-kClass=babel&from=1580056084009&to=1580077684009

HtmlPageLinkRendererBeginHookHandler constructs a LanguageFallbackChain for injecting into a LanguageFallbackLabelDescriptionLookup.
But this may happen even when the language fallback chain is not actually needed, ie, getLabel or getDescription will never be called.

Instead the fallback chain should be injected or the creation of the fallback chain deferred.
Such as in https://gerrit.wikimedia.org/r/#/c/mediawiki/extensions/Wikibase/+/567362/

Other code should also be checked for this bad pattern and fixed if it also occurs in other places.

Event Timeline

Addshore created this task.Jan 26 2020, 9:14 PM
Restricted Application added a project: Wikidata. · View Herald TranscriptJan 26 2020, 9:14 PM
Restricted Application added a subscriber: Aklapper. · View Herald Transcript
Addshore triaged this task as High priority.Jan 26 2020, 9:15 PM
Addshore awarded a token.

Change 567362 had a related patch set uploaded (by Addshore; owner: Addshore):
[mediawiki/extensions/Wikibase@master] Don't make a fallback chain if it might not be needed

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

Change 567363 had a related patch set uploaded (by Addshore; owner: Addshore):
[mediawiki/extensions/Wikibase@wmf/1.35.0-wmf.16] Don't make a fallback chain if it might not be needed

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

Change 567364 had a related patch set uploaded (by Addshore; owner: Addshore):
[mediawiki/extensions/Wikibase@wmf/1.35.0-wmf.15] Don't make a fallback chain if it might not be needed

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

Addshore claimed this task.Jan 26 2020, 9:31 PM
Addshore moved this task from Unsorted 💣 to Active 🚁 on the User-Addshore board.
Restricted Application added a project: Structured-Data-Backlog. · View Herald TranscriptJan 26 2020, 9:34 PM
Addshore updated the task description. (Show Details)Jan 26 2020, 9:36 PM
Addshore updated the task description. (Show Details)Jan 26 2020, 10:30 PM

Change 567362 merged by jenkins-bot:
[mediawiki/extensions/Wikibase@master] Don't make a fallback chain if it might not be needed

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

Change 567363 merged by jenkins-bot:
[mediawiki/extensions/Wikibase@wmf/1.35.0-wmf.16] Don't make a fallback chain if it might not be needed

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

I also think we should stop using that hook, it's just too general for our use.

Addshore removed Addshore as the assignee of this task.Jan 27 2020, 10:25 AM
Ladsgroup moved this task from To Do to Doing on the Structured Data Engineering board.
Restricted Application added a project: User-Ladsgroup. · View Herald TranscriptJan 27 2020, 10:29 AM

Change 567364 abandoned by Addshore:
Don't make a fallback chain if it might not be needed

Reason:
Abandoning as we probably wont backport this.
Feel free to revive if needed

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

Change 567467 had a related patch set uploaded (by Ladsgroup; owner: Ladsgroup):
[mediawiki/extensions/Wikibase@master] Make HtmlPageLinkRendererBeginHookHandler do not build LFC unless needed

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

Change 567467 merged by jenkins-bot:
[mediawiki/extensions/Wikibase@master] Make HtmlPageLinkRendererBeginHookHandler do not build LFC unless needed

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

Impact to number of calls to the memcached key when deployed in production today.