1268921 - [e10s] Allow non-integer values to be specified for e10srollout cohort samples

Status: RESOLVED FIXED

(Firefox :: General, defect)

Description

[:Felipe Gomes (needinfo for replies!)]

The original code for the e10srollout system add-on used integers for the sample values in order to avoid storing a float as string and doing parseFloat, which is the traditional way to store a float in the preferences service.

However, it looks like we may start with a sample < 1%, so we need to be able to specify non-integer values.

Comment 1

[:Felipe Gomes (needinfo for replies!)]

Attachment: MozReview Request: Bug 1268921 - Allow non-integer values to be specified for e10srollout cohort samples. r=mconley

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

Comment 2

[Mike Conley (:mconley) (:⚙️) (PTO June 8 - 12)]

Comment on attachment 8747246 [details]
MozReview Request: Bug 1268921 - Allow non-integer values to be specified for e10srollout cohort samples. r=mconley

https://reviewboard.mozilla.org/r/49777/#review46545

LGTM

Comment 3

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/3b7b5c87cf2b

Comment 4

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/3b7b5c87cf2b

Comment 5

[:Felipe Gomes (needinfo for replies!)]

Comment on attachment 8747246 [details]
MozReview Request: Bug 1268921 - Allow non-integer values to be specified for e10srollout cohort samples. r=mconley

Approval Request Comment
[Feature/regressing bug #]: e10s rollout system add-on
[User impact if declined]: Currently the system add-on only allows integer values to be used in the sample size, so 1% is the min value. However, Product might want to start with a value <1%, so we need to allow floats for that 
[Describe test coverage new/current, TreeHerder]: landed on central
[Risks and why]: small. the code converts the old integer value to a float in order to preserve the existing assignment of test/control users that is present on the current beta47 experiment.
[String/UUID change made/needed]: none

Comment 6

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8747246 [details]
MozReview Request: Bug 1268921 - Allow non-integer values to be specified for e10srollout cohort samples. r=mconley

I think we need this in Aurora48 as e10s might be enabled by default to a small percentage of users on release. I do not think this is needed in Beta47. Please let me know if that is not the case.

Comment 7

[:Felipe Gomes (needinfo for replies!)]

The value on beta47 would be to start testing the new code sooner, since the add-on is only used on Beta. However I might be ok with waiting until beta48 to test this change. Let me think some more about it and I'll let you know until the end of the week.

Comment 8

[:Felipe Gomes (needinfo for replies!)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/b6738ca64438