Closed Bug 1383338 Opened 7 years ago Closed 7 years ago

fetch+run shield studies soon after first startup

Categories

(Firefox :: Normandy Client, defect, P1)

Product:
Firefox
Component:
Normandy Client
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Tracking Flags:
Tracking Status
firefox55 --- fixed
firefox56 --- fixed

People

(Reporter: rhelmer, Assigned: rhelmer)

References

Details

Attachments

(1 file, 1 obsolete file)

Bug 1383338 - await shield recipe runner so recipes have a chance to see if this is first run
59 bytes, text/x-review-board-request
osmose
: review+
jcristau
: approval-mozilla-beta+
Details
Currently the shield recipe client sets its own timer which runs every 24 hours. This precludes downloading+running recipes in early startup (such as onboarding and retention-related experiments). This work is tracked in https://github.com/mozilla/normandy/pull/895 Since this runs early in startup and only on the first run, we can't rely on the gofaster update mechanism - only Firefox versions that ship with this feature will be able to take advantage of it.
Summary: fetch+run shield studies soon after startup → fetch+run shield studies soon after first startup
Assignee: nobody → rhelmer
Status: NEW → ASSIGNED
Priority: -- → P1
This patch is the same as https://github.com/mozilla/normandy/pull/895, with a minor change to one of the tests so it will apply to both mozilla-central and mozilla-beta repos.
Comment on attachment 8890442 [details] Bug 1383338 - fetch and run shield studies soon after UI startup https://reviewboard.mozilla.org/r/161560/#review166872 Can you update the version number in install.rdf.in to 55.1? That will compare greater than what is there now, so it updates correctly, but will compare less than future versions, especially 61 that I'm working on in bug 1384652.
Attachment #8890442 - Flags: review?(mcooper) → review-
Comment on attachment 8890442 [details] Bug 1383338 - fetch and run shield studies soon after UI startup https://reviewboard.mozilla.org/r/161560/#review166896
Attachment #8890442 - Flags: review?(mcooper) → review+
Comment on attachment 8890442 [details] Bug 1383338 - fetch and run shield studies soon after UI startup https://reviewboard.mozilla.org/r/161560/#review166916 rs=me
Attachment #8890442 - Flags: review?(gijskruitbosch+bugs) → review+
Pushed by rhelmer@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/02874c4ecef4 fetch and run shield studies soon after UI startup r=Gijs,mythmon
Comment on attachment 8890442 [details] Bug 1383338 - fetch and run shield studies soon after UI startup Approval Request Comment [Feature/Bug causing the regression]: bug 1383338 [User impact if declined]: inability to run retention tests for first-time users [Is this code covered by automated tests?]: yes [Has the fix been verified in Nightly?]: no [Needs manual test from QE? If yes, steps to reproduce]: yes, please contact shield team (#normandy in IRC) to coordinate [List of other uplifts needed for the feature/fix]: none [Is the change risky?]: no [Why is the change risky/not risky?]: runs once on first run only, easy to test [String changes made/needed]: none
Attachment #8890442 - Flags: approval-mozilla-beta?
https://hg.mozilla.org/mozilla-central/rev/02874c4ecef4
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox56: --- → fixed
Resolution: --- → FIXED
Comment on attachment 8890442 [details] Bug 1383338 - fetch and run shield studies soon after UI startup run shield client on first run, beta55+ should be in 55.0b13
Attachment #8890442 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
this seems to have introduced a large improvement in fileIO: == Change summary for alert #8376 (as of July 26 2017 21:05 UTC) == Improvements: 95% tp5n nonmain_startup_fileio windows7-32 opt e10s 66,880.17 -> 3,217.00 For up to date results, see: https://treeherder.mozilla.org/perf.html#/alerts?id=8376 this is not on the main thread, are we pushing much fileIO after startup completely?
(In reply to Joel Maher ( :jmaher) (UTC-9) (PTO: back August 2nd) from comment #13) > this seems to have introduced a large improvement in fileIO: > == Change summary for alert #8376 (as of July 26 2017 21:05 UTC) == > > Improvements: > > 95% tp5n nonmain_startup_fileio windows7-32 opt e10s 66,880.17 -> > 3,217.00 > > For up to date results, see: > https://treeherder.mozilla.org/perf.html#/alerts?id=8376 > > this is not on the main thread, are we pushing much fileIO after startup > completely? That's interesting. I did not expect that but we are indeed waiting until after UI startup to do anything now. I wasn't aware that shield was doing much IO before startup before this change, though...
Depends on: 1385177
Could you or someone please submit a request on pi-requests@mozilla.com with the details needed so we can test this? Tried on #normandy but got no answer there. Thanks!
Flags: needinfo?(rhelmer)
(In reply to Cornel Ionce [:cornel_ionce], Desktop Release QA from comment #15) > Could you or someone please submit a request on pi-requests@mozilla.com with > the details needed so we can test this? > Tried on #normandy but got no answer there. > > Thanks! Osmose, can you help with this? I'm thinking maybe we provide a test URL and instructions for QA to ensure that first-run works, that only filtered recipes run, etc. Also note that we need to backport bug 1385177 (disable self-support detection in shield) before this can be effectively tested, that should land momentarily.
Flags: needinfo?(rhelmer) → needinfo?(mkelly)
I'm working on getting a Beta build on trypush for testing, but found a bug. RecipeRunner.run is async, but is called synchronously from init(). By the time the recipes are executed, the first run pref has already been set to false, and the normandy.isFirstRun filter fails on the first run.
Flags: needinfo?(mkelly)
Status: RESOLVED → REOPENED
Flags: needinfo?(rhelmer)
Resolution: FIXED → ---
About to push a fix for this.
Flags: needinfo?(rhelmer)
Attachment #8890442 - Attachment is obsolete: true
Comment on attachment 8891498 [details] Bug 1383338 - await shield recipe runner so recipes have a chance to see if this is first run https://reviewboard.mozilla.org/r/162618/#review167974 This fixes the bug I observed in my test build. Thanks!
Attachment #8891498 - Flags: review+
Comment on attachment 8891498 [details] Bug 1383338 - await shield recipe runner so recipes have a chance to see if this is first run Approval Request Comment [Feature/Bug causing the regression]: bug 1383338 (followup) [User impact if declined]: inability to run retention tests for first-time users [Is this code covered by automated tests?]: yes [Has the fix been verified in Nightly?]: no [Needs manual test from QE? If yes, steps to reproduce]: yes, please contact shield team (#normandy in IRC) to coordinate [List of other uplifts needed for the feature/fix]: none [Is the change risky?]: no [Why is the change risky/not risky?]: runs once on first run only, easy to test [String changes made/needed]: none
Attachment #8891498 - Flags: approval-mozilla-beta?
Pushed by rhelmer@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/d776548e6846 await shield recipe runner so recipes have a chance to see if this is first run r=mkelly
QA request with testing steps has been sent to pi-request.
https://hg.mozilla.org/mozilla-central/rev/d776548e6846
Status: REOPENED → RESOLVED
Closed: 7 years ago → 7 years ago
Resolution: --- → FIXED
Comment on attachment 8891498 [details] Bug 1383338 - await shield recipe runner so recipes have a chance to see if this is first run followup fix needed for 55 rc1
Attachment #8891498 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Depends on: 1385915
No longer depends on: 1390824
Product: Shield → Firefox
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: