Page MenuHomePhabricator

Caching Termbox output on the Wikibase side
Closed, ResolvedPublic5 Story Points

Description

AC
Implement the proposal defined in the Termbox and ParserCache Integration ADR.

Background
User-specific html must not be added to the ParserOutput (cache). The "old" PlaceholderEmittingEntityTermsView Termbox prevents that by rendering placeholders in place of the user-specific markup. The new Termbox has to implement some way to ensure that no user-specific markup (e.g. toggle state, language preferences) is added, possibly by rendering the Termbox for logged-in users at a later point in the request life cycle, e.g. on OutputPageBeforeHTMLHookHandler::doOutputPageBeforeHTML.

This has to be solved before deployment.

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptJan 25 2019, 11:07 AM
Jakob_WMDE renamed this task from Termbox Caching to Caching Termbox output on the Wikibase side.Jan 28 2019, 12:51 PM
Jakob_WMDE updated the task description. (Show Details)Feb 11 2019, 11:22 AM
Lea_WMDE triaged this task as Normal priority.Feb 11 2019, 11:53 AM
Lea_WMDE set the point value for this task to 5.
Pablo-WMDE added a comment.EditedMar 5 2019, 2:09 PM

When looking at the different scenarios of how TermboxView is invoked and what happens in error cases we observed an interesting behavior: TermboxView::renderTermbox() was invoked repeatedly (3 times) when running into SSR errors (i.e. no result written into ParserCache) while trying to display an item page.
Upon further investigation it was found that RefreshLinksJob & DerivedPageDataUpdater, i.e. jobs run after the connection to the client is already closed, are the source of the two extra calls.
Given that they do what they are designed to do and (actively or as a side effect) also ensure that ParserCache contains meaningful values this is expected behavior.

Risks:

  • in case of problems generating a termbox result (through a service request) the expiration time for this page's cache will be set to 0, i.e. every following request (incl. but not limited to jobs) will also try to generate a termbox result by the same means until a result is found and successfully persisted in the ParserCache

Possible mitigations:

  • implement some form of throttling (for that scenario in particular or any request in general)

Change 494717 had a related patch set uploaded (by Pablo Grass (WMDE); owner: Pablo Grass (WMDE)):
[mediawiki/extensions/Wikibase@master] OutputPageBeforeHTMLHookHandler: editablity as a service

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

Change 494717 merged by jenkins-bot:
[mediawiki/extensions/Wikibase@master] OutputPageBeforeHTMLHookHandler: editablity as a service

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

Jakob_WMDE added a comment.EditedMar 12 2019, 2:12 PM

The languages shown on entity pages should now be identical for the new and the old termbox, even (edit: found an unrelated inconsistency: T218111) when server-side rendered. Steps to verify:

Lea_WMDE closed this task as Resolved.Mar 19 2019, 1:39 PM
Lea_WMDE claimed this task.