Page MenuHomePhabricator

Deal with styling issues of old content versions in RESTBase storage
Closed, ResolvedPublic

Description

https://gerrit.wikimedia.org/r/c/mediawiki/core/+/902203 will go out with next week's train. We waited a month to merge it so that parserCache will have rolled over for legacy output read views.

However, the styling also affects Parsoid content. The necessary class was added to Parsoid's output a few weeks ago and is present in content version 2.7.1

RESTBase meanwhile still has older versions of content in storage. This means that if a user tries to edit a page in VE and gets an older version, the styling of media won't be presented correctly. There won't be any problem with editing though, since that was made compatible.

We can revert the patch until a solution is in place but there are few follow on patches that make the task a little more hairy, though not unfeasible.

We have a few options to deal with this though,

  1. Purge content from RESTBase that's older than the 2.7.1 deploy date. There's a script that can be adapted for that in T296425 but that's never been tried before.
  1. We can bump the version of content that VE requests so that it should never be returned an older version from storage. The only problem with that is that RESTBase ignores the patch version (the 1 in 2.7.1) and only considers major and minor versions of the accept header when doing semver caret semantics. To account for that, Parsoid has been bumped to 2.8.0 which will also go out in next week's train. But that presents two more problems,
    • If the VE patch goes out in the same train as the vendor patch that has the parsoid bump, storage isn't going to be very warm with 2.8.0 content so editors will pay the price of parsing
    • The refactored Parsoid http api in core might return old 2.7.1 in parserCache to RESTBase for 2.8.0 requests. This is noted in T333606 and may (or may not) cause RESTBase to reject it as not satisfying the request and error out. This still needs to be verified.

Event Timeline

@ssastry asks,

Would ContentTranslation also need a 2.8.0 version header bump in their codebase?

What other clients would be affected?

The refactored Parsoid http api in core might return old 2.7.1 in parserCache to RESTBase for 2.8.0 requests. This is noted in T333606 and may (or may not) cause RESTBase to reject it as not satisfying the request and error out. This still needs to be verified.

Yeah, confirmed, RESTBase will throw,
https://github.com/wikimedia/restbase/blob/master/lib/content_negotiation_filter.js#L120-L124

{"name":"restbase","pid":8268,"level":50,"message":"Failed to satisfy required version","requested_version":"2.8.0","stored_version":"2.7.1","root_req":{"method":"get","uri":"/ve/v1/page/html/DiffCompareSLLafJOvcz"},"request_id":"45dfe840-fe64-11ed-b8ab-cbd8665b5d38","api_path":"/ve/v1/page/html/{title}","levelPath":"error/negotiation","msg":"Failed to satisfy required version","time":"2023-05-29T21:03:34.211Z","v":0}

Change 923691 had a related patch set uploaded (by Arlolra; author: Arlolra):

[mediawiki/extensions/VisualEditor@master] Bumped requested Parsoid HTML version to 2.8.0

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

@Arlolra Thanks for this analysis. Very helpful. But in light of what you say in T333606#8887241, looks like this is going to be a non-issue? VE asks RB for 2.8.8. RB finds 2.7.0 and asks Parsoid to give it 2.8.0. Without your patches for T333606, Parsoid looks in PC and gives it what it has. Given the 21 day expiry time for it, it will never be 2.7.0, correct?

So, while your fixes should still land, I think we can land the VE changes whenever without worrying about the scenarios you are concerned about here? We could choose to wait till next week if we are concerned about a cold store/cache wrt 2.8.0 content.

correct?

Not from my understanding, no. If VE asks for 2.8.0 and Parsoid returns a version that doesn't satisfy that (and, to be clear, 2.7.1 does not), RESTBase will return a 406 to VE. See T337596#8887234

Given the 21 day expiry time for it

2.7.1 was deployed to Parsoid May 16th
https://www.mediawiki.org/wiki/Parsoid/Deployments#May_16_-_May_18:_V0.18.0-a10_as_part_of_1.41.0-wmf.9

Change 923691 merged by jenkins-bot:

[mediawiki/extensions/VisualEditor@master] Bump requested Parsoid HTML version to 2.8.0

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

Styling of media in VE on mediawikiwiki is looking good after today's deploy of wmf.13. It should continue to roll out with the train.