1362493 - e10s beta cohort distribution improvements

Status: RESOLVED FIXED

(Firefox :: General, defect)

Description

[Jim Mathies [:jimm]]

Doing some analysis over on re:dash, a few issues come to mind:

1) are the old multibucket1, 2, 4, 8 users getting converted over to the new webextension-multibuket1, 4 buckets? 

If not, we should probably do this to clean up the data and add more users to the buckets we're interested in.

2) We seem to have very low webextension-multibuket1, 4 client counts. This might be a side effect of #1.

3) The old multibuckets appear to have a number of non-e10s users in them. Can we figure out what's going on there and clean it up?

beta e10s cohort breakdown: 
https://sql.telemetry.mozilla.org/queries/4459/source#8959

beta non-e10s cohort breakdown:
https://sql.telemetry.mozilla.org/queries/4457/source#8955

Comment 1

[Blake Kaplan (:mrbkap) (inactive)]

(In reply to Jim Mathies [:jimm] from comment #0)
> 1) are the old multibucket1, 2, 4, 8 users getting converted over to the new
> webextension-multibuket1, 4 buckets? 

Yes. We recompute the cohort name on every launch.

> 2) We seem to have very low webextension-multibuket1, 4 client counts. This
> might be a side effect of #1.

I guess? It seems more likely to me that webextension-based addons are just pretty rare.

> 3) The old multibuckets appear to have a number of non-e10s users in them.
> Can we figure out what's going on there and clean it up?

That could be because of a race on first startup where we don't detect that e10s is disabled due to a11y (overholt ran into this), but I'd expect that to fix itself on the second run (and then we'd end up with a cohort of either disqualified-control or disqualified-test).

Comment 2

[Blake Kaplan (:mrbkap) (inactive)]

Attachment: Bug 1362493 - Let users with mpc=true addons into the multi experiment.

Review commit: https://reviewboard.mozilla.org/r/137196/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/137196/

Comment 3

[:Felipe Gomes (needinfo for replies!)]

So let me add a bit of historical context here:

There has always existed a race between these prefs being set, and/or any code reading mozilla::BrowserTabsRemoteAutostart/Services.appinfo.browserTabsRemoteAutostart, including any add-on code  that can trigger it.

It used to be that at the time of our past experiments I tested this, and on a fresh profile this did not happen, and the e10s prefs were all set before the backend needed to check for e10s. (It was certainly the case that some profiles with add-ons could change the order of things, but it was not widespread).
Now, I wouldn't doubt that any code that landed in the last ~6months could have changed the order of things, guaranteeing that the e10srollout addon will lose the race.

This doesn't cause any issues, except that a user who gets tagged for multi-process might need to wait a restart for that to take effect. And that we'll see this in the data.


In talking with Blake on IRC, we decided to change the "multiBucket-N" cohort names to something entirely new, so that all data that we look is fresh. Let's see how that goes and when we look at better data, we can revisit this to see if there are any other problems.

Comment 4

[:Felipe Gomes (needinfo for replies!)]

Comment on attachment 8865570 [details]
Bug 1362493 - Let users with mpc=true addons into the multi experiment.

https://reviewboard.mozilla.org/r/137196/#review140292

::: browser/extensions/e10srollout/bootstrap.js:198
(Diff revision 1)
>    let buckets = MULTI_BUCKETS[updateChannel];
>  
>    let multiUserSample = getUserSample(true);
>    for (let sampleName of Object.getOwnPropertyNames(buckets)) {
>      if (multiUserSample < buckets[sampleName]) {
>        setCohort(`${cohortPrefix}multiBucket${sampleName}`);

Let's change this to something else to clear the data

Comment 5

[Blake Kaplan (:mrbkap) (inactive)]

(In reply to :Felipe Gomes (needinfo me!) from comment #4)
> Let's change this to something else to clear the data

I spoke to felipe on IRC and we agreed to keep the current names.

Comment 6

[Blake Kaplan (:mrbkap) (inactive)]

Comment on attachment 8865570 [details]
Bug 1362493 - Let users with mpc=true addons into the multi experiment.

Approval Request Comment
[Feature/Bug causing the regression]: e10s-multi experiment
[User impact if declined]: n/a
[Is this code covered by automated tests?]: n/a
[Has the fix been verified in Nightly?]: n/a
[Needs manual test from QE? If yes, steps to reproduce]: n/a
[List of other uplifts needed for the feature/fix]: n/a
[Is the change risky?]: no, it's well understood
[Why is the change risky/not risky?]: n/a
[String changes made/needed]: n/a

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4dff49910a61
Let users with mpc=true addons into the multi experiment. r=Felipe

Comment 8

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/4dff49910a61

Comment 9

[Erin Lancaster [:elan]]

Hi Gerry, we are hoping to get this into Monday's GTB for Beta 8. Please NI Blake or Felipe with any questions. Thank you for the support!

Comment 10

[Gerry Chang [:gchang]]

Comment on attachment 8865570 [details]
Bug 1362493 - Let users with mpc=true addons into the multi experiment.

Support for e10s-multi experiment. Beta54+. Should be in 54 beta 8.

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/8be7562b858f