Page MenuHomePhabricator

ext.visualEditor.data causes cache churn
Closed, ResolvedPublic8 Story Points

Description

The version hash of the ext.visualEditor.data ResourceLoader module is unstable. On nlwiki, where we happen to have some monitoring of version hashes set up, the module version has oscillated between VPsvd4pk and M03uRLLD about five times a day for the past six days. The intervals are irregular. Each change causes the start-up module to change as well, meaning clients must re-retrieve it.

Here is what I believe is happening:

  • The module version is derived from a hash of the module script content.
  • The short SHA1 of HEAD of the repository at $IP/extensions/VisualEditor (obtained using MediaWiki's GitInfo class) is embedded in the script content as ve.version.id.
  • Git metadata is not available (or not consistently available) on the application servers, so ve.version contains either real values from Git or fallbacks.

For example, the following command outputs bool(false) when run on tin, and string(40) "c895ba38b16efaf70add12ca60e5dbffcfb87c13" when run on mw1017:

mwscript eval.php nlwiki <<<'$git = new GitInfo( "/srv/mediawiki/php-1.27.0-wmf.7/extensions/VisualEditor" ); var_dump( $git->getHeadSHA1() );'

The easiest way to fix this might be to stop embedding git metadata in the module script content. It can't be very important, because it is very often wrong (due to this bug), and no one has noticed.

Event Timeline

ori created this task.Nov 27 2015, 5:26 AM
ori updated the task description. (Show Details)
ori raised the priority of this task from to High.
ori added a subscriber: ori.
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptNov 27 2015, 5:26 AM
krenair@tin:~$ SSH_AUTH_SOCK=/run/keyholder/proxy.sock dsh -F 20 -M -g mediawiki-installation -r ssh -o -oUser=mwdeploy -- "sudo -u www-data php /srv/mediawiki/multiversion/MWScript.php eval.php nlwiki <<<'\$git = new GitInfo( \"/srv/mediawiki/php-1.27.0-wmf.7/extensions/VisualEditor\" ); var_dump( \$git->getHeadSHA1() );' | head -n 1" | grep -v c895ba38b16efaf70add12ca60e5dbffcfb87c13
tin.eqiad.wmnet: No entry for terminal type "unknown";
tin.eqiad.wmnet: using dumb terminal settings.
snapshot1001.eqiad.wmnet: [Fri Nov 27 18:31:22 2015] [hphp] [63176:7f23d9b6ad00:0:000001] [] 
snapshot1003.eqiad.wmnet: [Fri Nov 27 18:31:22 2015] [hphp] [29476:7f341ee73d00:0:000001] [] 
snapshot1002.eqiad.wmnet: [Fri Nov 27 18:31:23 2015] [hphp] [9465:7fce8d731d00:0:000001] [] 
snapshot1004.eqiad.wmnet: [Fri Nov 27 18:31:23 2015] [hphp] [2921:7fd182c80d00:0:000001] [] 
mw1040.eqiad.wmnet: <jemalloc>: creat("jeprof.20611.0.f.heap"), 0644) failed
mw1107.eqiad.wmnet: <jemalloc>: creat("jeprof.17579.0.f.heap"), 0644) failed
krenair@tin:~$
krenair@tin:~$ sudo -u www-data php /srv/mediawiki/multiversion/MWScript.php eval.php nlwiki <<<'$git = new GitInfo( "/srv/mediawiki/php-1.27.0-wmf.7/extensions/VisualEditor" ); var_dump( $git->getHeadSHA1() );'
string(40) "c895ba38b16efaf70add12ca60e5dbffcfb87c13"


krenair@tin:~$ mwscript eval.php nlwiki <<<'$git = new GitInfo( "/srv/mediawiki/php-1.27.0-wmf.7/extensions/VisualEditor" ); var_dump( $git->getHeadSHA1() );'
bool(false)


krenair@tin:~$
ori added a comment.Nov 27 2015, 11:33 PM

@Krenair -- thanks. I should have thought of that. At any rate, on tin $IP is /srv/mediawiki-staging, not /srv/mediawiki:

$ sudo -u www-data php /srv/mediawiki/multiversion/MWScript.php eval.php nlwiki <<<'$git = new GitInfo( "/srv/mediawiki/php-1.27.0-wmf.7/extensions/VisualEditor" ); var_dump( $git->getHeadSHA1() );'
string(40) "c895ba38b16efaf70add12ca60e5dbffcfb87c13"

$ sudo -u www-data php /srv/mediawiki/multiversion/MWScript.php eval.php nlwiki <<<'$git = new GitInfo( "/srv/mediawiki-staging/php-1.27.0-wmf.7/extensions/VisualEditor" ); var_dump( $git->getHeadSHA1() );'
string(40) "8951a7ba73518e6eff97ae16e778a54224b579d2"

So I suspect it is still the cause.

Here is the log, in case you'd like to try to correlate it with actions on tin (such as l10nupdate or deployments):

2015-11-20 23:41:40 ext.visualEditor.data: M03uRLLD -> VPsvd4pk (changed after 1 day, 4 hours, 14 minutes and 46 seconds)
2015-11-20 23:46:43 ext.visualEditor.data: VPsvd4pk -> M03uRLLD (changed after 5 minutes and 2 seconds)
2015-11-22 16:47:33 ext.visualEditor.data: M03uRLLD -> VPsvd4pk (changed after 1 day, 17 hours and 49 seconds)
2015-11-22 16:52:36 ext.visualEditor.data: VPsvd4pk -> M03uRLLD (changed after 5 minutes and 3 seconds)
2015-11-22 23:18:25 ext.visualEditor.data: M03uRLLD -> VPsvd4pk (changed after 6 hours, 25 minutes and 49 seconds)
2015-11-22 23:23:28 ext.visualEditor.data: VPsvd4pk -> M03uRLLD (changed after 5 minutes and 2 seconds)
2015-11-23 03:54:15 ext.visualEditor.data: M03uRLLD -> VPsvd4pk (changed after 4 hours, 30 minutes and 46 seconds)
2015-11-23 03:59:18 ext.visualEditor.data: VPsvd4pk -> M03uRLLD (changed after 5 minutes and 2 seconds)
2015-11-23 05:29:13 ext.visualEditor.data: M03uRLLD -> VPsvd4pk (changed after 1 hour, 29 minutes and 55 seconds)
2015-11-23 05:34:16 ext.visualEditor.data: VPsvd4pk -> M03uRLLD (changed after 5 minutes and 3 seconds)
2015-11-24 00:16:58 ext.visualEditor.data: M03uRLLD -> VPsvd4pk (changed after 18 hours, 42 minutes and 41 seconds)
2015-11-24 00:22:00 ext.visualEditor.data: VPsvd4pk -> M03uRLLD (changed after 5 minutes and 2 seconds)
2015-11-24 04:42:39 ext.visualEditor.data: M03uRLLD -> VPsvd4pk (changed after 4 hours, 20 minutes and 38 seconds)
2015-11-24 04:47:42 ext.visualEditor.data: VPsvd4pk -> M03uRLLD (changed after 5 minutes and 3 seconds)
2015-11-25 01:20:22 ext.visualEditor.data: M03uRLLD -> VPsvd4pk (changed after 20 hours, 32 minutes and 39 seconds)
2015-11-25 01:25:25 ext.visualEditor.data: VPsvd4pk -> M03uRLLD (changed after 5 minutes and 2 seconds)
2015-11-25 03:35:42 ext.visualEditor.data: M03uRLLD -> VPsvd4pk (changed after 2 hours, 10 minutes and 17 seconds)
2015-11-25 03:40:45 ext.visualEditor.data: VPsvd4pk -> M03uRLLD (changed after 5 minutes and 3 seconds)
2015-11-25 04:46:24 ext.visualEditor.data: M03uRLLD -> VPsvd4pk (changed after 1 hour, 5 minutes and 38 seconds)
2015-11-25 04:51:27 ext.visualEditor.data: VPsvd4pk -> M03uRLLD (changed after 5 minutes and 3 seconds)
2015-11-25 09:41:28 ext.visualEditor.data: VPsvd4pk -> M03uRLLD (changed after 5 minutes and 2 seconds)
2015-11-25 11:56:48 ext.visualEditor.data: M03uRLLD -> VPsvd4pk (changed after 2 hours, 15 minutes and 20 seconds)
2015-11-25 12:01:51 ext.visualEditor.data: VPsvd4pk -> M03uRLLD (changed after 5 minutes and 3 seconds)
2015-11-25 12:47:18 ext.visualEditor.data: M03uRLLD -> VPsvd4pk (changed after 45 minutes and 26 seconds)
2015-11-25 12:52:21 ext.visualEditor.data: VPsvd4pk -> M03uRLLD (changed after 5 minutes and 3 seconds)
2015-11-25 20:43:05 ext.visualEditor.data: M03uRLLD -> VPsvd4pk (changed after 7 hours, 50 minutes and 43 seconds)
2015-11-25 20:48:08 ext.visualEditor.data: VPsvd4pk -> M03uRLLD (changed after 5 minutes and 2 seconds)
2015-11-26 14:35:11 ext.visualEditor.data: VPsvd4pk -> M03uRLLD (changed after 4 minutes and 2 seconds)
2015-11-27 04:17:19 ext.visualEditor.data: M03uRLLD -> VPsvd4pk (changed after 13 hours, 42 minutes and 7 seconds)
2015-11-27 04:22:22 ext.visualEditor.data: VPsvd4pk -> M03uRLLD (changed after 5 minutes and 2 seconds)

Removing the git metadata from ve.version still seems easiest to me.

Your should check with our QAs to see if they use this regularly.

Change 255812 had a related patch set uploaded (by Ori.livneh):
Avoid embedding Git metadata in ext.visualEditor.data

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

Copying @Ryasmeen and @Etonkovidova . How much do you need the feature in VE that tells you which Git version you are running?

I do use it to check the commit tree, so that I can track whether a fix is merged on that release or not. But I guess I can do that from Special:version page too.

@ori would it be better if we moved this to a separate module which is only loaded when the help popup is opened?

ori added a comment.Nov 30 2015, 12:48 AM

@ori would it be better if we moved this to a separate module which is only loaded when the help popup is opened?

No. The impact of this bug is not limited to users of VisualEditor. When the module version changes, the startup module (included on every page) also changes, because it contains module manifest which lists the current version of each module. When the startup module changes, clients which have it cached are forced to re-fetch it.

This happens every time we deliberately push out a change to our JavaScript code. The problem with ext.visualEditor.data is that it also changes spontaneously, multiple times a day.

ori added a comment.Nov 30 2015, 1:06 AM

You could get this information from the API, via /w/api.php?action=query&meta=siteinfo&format=json&siprop=extensions. If you did that, then the data would not have to be embedded in the module itself.

Change 255812 merged by jenkins-bot:
Avoid embedding Git metadata in ext.visualEditor.data

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

Jdforrester-WMF closed this task as Resolved.Nov 30 2015, 3:49 PM
Jdforrester-WMF assigned this task to ori.
Jdforrester-WMF removed a project: Patch-For-Review.
Jdforrester-WMF set Security to None.
Jdforrester-WMF edited a custom field.

Change 256015 had a related patch set uploaded (by Jforrester):
Avoid embedding Git metadata in ext.visualEditor.data

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

Change 256015 merged by jenkins-bot:
Avoid embedding Git metadata in ext.visualEditor.data

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