Page MenuHomePhabricator

A/B config flag should be subject to ResourceLoader caching rules not HTML caching rules
Closed, ResolvedPublic2 Story Points

Description

Ronseal.

Config added inside SkinMinerva is subject to the rules of HTML caching and can take several days [1]

Adding it via the onResourceLoaderGetConfigVars hook reduces the cache time to minutes [2], giving us more control over the experiment's enabled state.
[1] T124954
[2] https://www.mediawiki.org/wiki/ResourceLoader

Event Timeline

Jdlrobson triaged this task as High priority.Sep 24 2018, 10:49 PM
Jdlrobson created this task.
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptSep 24 2018, 10:49 PM

Change 462582 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/skins/MinervaNeue@master] Minerva A/B tests are not subject to HTML caching time

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

Jdlrobson updated the task description. (Show Details)Sep 24 2018, 10:49 PM

Change 462658 had a related patch set uploaded (by Pmiazga; owner: Jdlrobson):
[mediawiki/skins/MinervaNeue@wmf/1.32.0-wmf.22] Minerva A/B tests are not subject to HTML caching time

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

Change 462582 merged by jenkins-bot:
[mediawiki/skins/MinervaNeue@master] Minerva A/B tests are not subject to HTML caching time

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

Change 462658 merged by jenkins-bot:
[mediawiki/skins/MinervaNeue@wmf/1.32.0-wmf.22] Minerva A/B tests are not subject to HTML caching time

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

Mentioned in SAL (#wikimedia-operations) [2018-09-25T12:17:36Z] <pmiazga@deploy1001> Started scap: php-1.32.0-wmf.22/skins/MinervaNeue/includes SWAT: [[gerrit:462658|Minerva A/B tests are not subject to HTML caching time (T205355)]]

Mentioned in SAL (#wikimedia-operations) [2018-09-25T12:34:33Z] <pmiazga@deploy1001> Finished scap: php-1.32.0-wmf.22/skins/MinervaNeue/includes SWAT: [[gerrit:462658|Minerva A/B tests are not subject to HTML caching time (T205355)]] (duration: 16m 51s)

Change 462692 had a related patch set uploaded (by Pmiazga; owner: Pmiazga):
[mediawiki/skins/MinervaNeue@master] Fix broken config name for MinervaABSamplingRate

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

Change 462692 merged by jenkins-bot:
[mediawiki/skins/MinervaNeue@master] Fix broken config name for MinervaABSamplingRate

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

Change 462705 had a related patch set uploaded (by Pmiazga; owner: Pmiazga):
[mediawiki/skins/MinervaNeue@wmf/1.32.0-wmf.22] Fix broken config name for MinervaABSamplingRate

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

ovasileva set the point value for this task to 2.Sep 25 2018, 4:13 PM

Tags are lying https://gerrit.wikimedia.org/g/mediawiki/skins/MinervaNeue/+/8c587c58c9c519390c8ccc8f8590d8b1f8989bdc is on mediawiki.org so it made it into 1.32.0-wmf.23

The SWAT today broke wmf.22 (and was done presumably to have this patch 2 days early as it would go out on Thursday, questionable about whether that was necessary).

I need to SWAT https://gerrit.wikimedia.org/r/462705 later today to fix wmf.22 so it's not broken.

Change 462705 merged by jenkins-bot:
[mediawiki/skins/MinervaNeue@wmf/1.32.0-wmf.22] Fix broken config name for MinervaABSamplingRate

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

Over to you. We'll back in action on Latvian Wikipedia. I'd hope without the problems of caching this time round...

Over to you. We'll back in action on Latvian Wikipedia. I'd hope without the problems of caching this time round...

Cool, thanks, I understand this means I can now resume/repeat the data checks at T200792: [EPIC] Run A/B test on page issues (Farsi, Japanese, Russian, English).

But I'm not sure what is involved in signing off on this task here. Inspecting the code to verify that the A/B config flag is now indeed subject to ResourceLoader caching rules? If so, a developer might be better positioned to sign off on it.

Krinkle added a subscriber: Krinkle.

Please consider paying off related technical debt from T186062 while at it. This is easy to fix, but has an immediate benefit.

The current patch has made things worse.

ovasileva added a subscriber: Niedzielski.

assigning to @Niedzielski for technical signoff

Niedzielski closed this task as Resolved.Oct 1 2018, 6:34 PM

Prior to the patch, I see wgMinervaABSamplingRate cached in the HTML's script tag with the other configs. After the patch, I don't see it in the HTML but it is in the MediaWiki startup module. I'm marking this resolved per task description but @Krinkle has highlighted additional work to pick up soon.