Page MenuHomePhabricator

File dependency tracking unstable (varies by language)
Closed, ResolvedPublic

Description

Since rMW285c52039bf4: Revamp classic edit toolbar not to hardcode paths in HTML (I39d8ed4258c), the set of image files that is referenced by a stylesheet can vary by language. (See resources/src/mediawiki.toolbar/toolbar.less, for example.)

ResourceLoader does not expect file dependencies to vary by language. The database table in which ResourceLoader tracks module dependencies is indexed by module name and skin only.

As a result, compiling a ResourceLoader module in the context of one language will pollute the module dependency table with language-specific data, causing cache churn.

Going by log data, I think that the following modules are likely affected:

  • ext.gather.collection.editor
  • ext.gather.styles
  • ext.popups
  • ext.templateDataGenerator.ui
  • jquery.tipsy
  • jquery.wikiEditor.toolbar
  • mediawiki.toolbar
  • mmv

The obvious solution would be to add an md_lang column to the module_deps table. But using the database to track file dependencies is wrong, and we need to stop doing that sooner or later, and this bug may provide the right occasion for finally doing that.

Related Objects

Event Timeline

ori created this task.Sep 27 2015, 2:59 AM
ori raised the priority of this task from to High.
ori updated the task description. (Show Details)
ori added subscribers: ori, Krinkle, matmarex.
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptSep 27 2015, 2:59 AM
ori added a comment.Sep 27 2015, 6:10 PM

Since this is going to take a bit of time to fix properly, we should consider having a temporary workaround in place to mitigate the performance impact. One possible solution is to reference all the different languages' images under a dummy CSS selector. @matmarex, would you be willing to give that a shot?

Change 241571 had a related patch set uploaded (by Ori.livneh):
mediawiki.toolbar: temporary workaround for T113868

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

Change 241572 had a related patch set uploaded (by Ori.livneh):
mediawiki.toolbar: temporary workaround for T113868

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

Change 241572 merged by jenkins-bot:
mediawiki.toolbar: temporary workaround for T113868

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

Krinkle renamed this task from File dependency tracking for ResourceLoader modules is broken to File dependency tracking unstable (varies by language).Sep 27 2015, 11:08 PM
Krinkle set Security to None.

Let's prioritise migration of module_deps to use one of the standard object stash or object cache interfaces. Adding extra fields in the value array or key will be trivial then.

Change 241571 merged by jenkins-bot:
mediawiki.toolbar: temporary workaround for T113868

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

ori added a comment.Sep 29 2015, 2:30 AM

This actually predates rMW285c52039. It happens due to ltr / rtl flipping of images, too.

Change 242417 had a related patch set uploaded (by Krinkle):
resourceloader: Vary module_deps on language (in addition to skin)

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

Change 242417 merged by jenkins-bot:
resourceloader: Vary module_deps on language (in addition to skin)

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

Krinkle closed this task as Resolved.Sep 30 2015, 10:12 PM
matmarex reopened this task as Open.Jul 14 2016, 4:44 PM

So I just happened upon the "awful workaround" from https://gerrit.wikimedia.org/r/241571 still in the code… can we revert that?

@matmarex Yes, that should be safe to remove. Since file dependency storage is now fragmented by language, the overwrite loop can't happen anymore.

Change 299017 had a related patch set uploaded (by Bartosz Dziewoński):
Revert "mediawiki.toolbar: temporary workaround for T113868"

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

Change 299017 merged by jenkins-bot:
Revert "mediawiki.toolbar: temporary workaround for T113868"

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

matmarex closed this task as Resolved.Jul 14 2016, 7:33 PM