Closed Bug 1046297 Opened 5 years ago Closed 5 years ago

Change frequency about:home checks the snippet service

Categories

(Firefox :: General, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 38
Iteration:
38.1 - 26 Jan
Tracking Status
firefox37 --- fixed
firefox38 --- fixed

People

(Reporter: cmore, Assigned: Gavin)

References

Details

Attachments

(1 file)

Currently, about:home checks the snippet services once every 24 hours or whenever the person comes online next. The 24 hour lag doesn't give the Engagement team the ability to deliver timely messages or react to some condition. We would like to increase the frequency of snippet checking/delivery to something less than every 24 hours and after the infrastructure is set up to allow for it.

Let's dig into the Firefox code and see what type of changes would need to happen to allow the frequency to be changed. It should be a fairly trivial change given that the code looks if 24 hours has passed since the past timestamp was set. We wouldn't want to merge this change until the dependent bugs are resolved.
Depends on: 1040933
(In reply to Marco Bonardo [:mak] from comment #1)
> just change SNIPPETS_UPDATE_INTERVAL_MS
> http://mxr.mozilla.org/mozilla-central/source/browser/base/content/abouthome/
> aboutHome.js#147

That's about as trivial as they come! :) Thanks for the link to the repo. We can move forward with that change and testing after the performance work is complete.
Flags: firefox-backlog+
OS: Mac OS X → All
Hardware: x86 → All
Gavin: What is the timeframe on when we can get the interval changed to something less than the amount of seconds in one day? Is this trivial enough that it could be uplifted? We could start with 14400 seconds, which is 4 hours.
Depends on: 1058759
We would like to change the interval after bug 1058759 is resolved.
We can change the interval and uplift to beta at any time.
(In reply to :Gavin Sharp [email: gavin@gavinsharp.com] from comment #5)
> We can change the interval and uplift to beta at any time.

Ok, who can do the patch to change this value to 14400?

http://mxr.mozilla.org/mozilla-central/source/browser/base/content/abouthome/aboutHome.js#149

Thanks
Flags: needinfo?(gavin.sharp)
I can write the patch - what do you want to reduce the minimum interval to?

(14400 would be "check every 14.4 seconds", so I don't think it's what we want).
Flags: needinfo?(gavin.sharp) → needinfo?(chrismore.bugzilla)
We'd like to change the delay to check for an update every four hours.

Also, I'd like to hold off on landing this change on Nightly until we had a chance to see the affect bug 1058759 has on the service's load (rather than just waiting until the bug was resolved, as comment 4 implied). I'll update this bug when we're ready for reals. :D
Flags: needinfo?(chrismore.bugzilla)
(In reply to Chris More [:cmore] from comment #6)
> (In reply to :Gavin Sharp [email: gavin@gavinsharp.com] from comment #5)
> > We can change the interval and uplift to beta at any time.
> 
> Ok, who can do the patch to change this value to 14400?
> 
> http://mxr.mozilla.org/mozilla-central/source/browser/base/content/abouthome/
> aboutHome.js#149
> 
> Thanks

Oh, yes, it is MS and not S. So it would be 14400000.

As Osmose said, we'll hold off from landing this until we have seen how the CDN is holding up and then deploy this change.
Any updates here?
Flags: needinfo?(mkelly)
The CDN didn't work out (bug 1058759 comment 17) so we're trying a few other things. We still eventually want this feature, but it's on hold indefinitely until we're confident the service can handle it. Should we leave this open for now or close and re-file when we're ready?
Flags: needinfo?(mkelly)
Fine to leave it open, just ping when you want it done.
After successfully testing some recent performance improvements to snippets, I think we're ready to bump this down to 4 hours. I'd also be comfortable with uplifting this to Aurora if we give it at least 3 days on Nightly to see how it affects the snippets service.
Flags: needinfo?(gavin.sharp)
Also, is it possible to make this change for a certain percentage of users when it hits beta and release? IE can we hit beta and release with only 10% of users switching to 4 hours, then ramp it up to eventually hit 100%? Would each ramp-up require a new build of Firefox?
Not possible to stagger the rollout to the different interval on the client side without additional work. We can do that work if necessary, but it will take more time.

I will get this change landed on Nightly soon.
Flags: needinfo?(gavin.sharp)
Attached patch patchSplinter Review
Assignee: nobody → gavin.sharp
Status: NEW → ASSIGNED
Attachment #8551436 - Flags: review?(mak77)
Attachment #8551436 - Flags: review?(mak77) → review+
(In reply to :Gavin Sharp [email: gavin@gavinsharp.com] from comment #15)
> Not possible to stagger the rollout to the different interval on the client
> side without additional work. We can do that work if necessary, but it will
> take more time.

Thanks for the info. Would it be possible to rollback the change instead in case we run into major issues? Would that be a "lots of effort, last resort" option, or something we can do easily? Just want to make sure I understand our options in case we can't handle the load.
Without investing in building some sort of dynamic interval update system, reverting the change requires re-spinning the Firefox build, which has a decent amount of overhead (i.e. minimum 1 day turnaround+Firefox update lag, and relatively high cost). If you can work around that by just increasing your ability to adapt on the server side, that would be ideal, but it sounds like probably we'll need to investigate that better interval update system.
Pushed to fx-team:
https://hg.mozilla.org/integration/fx-team/rev/c8019c6eda14

This should be live in Nightly within 1-2 days.
Target Milestone: --- → Firefox 38
https://hg.mozilla.org/mozilla-central/rev/c8019c6eda14
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Iteration: --- → 38.1 - 26 Jan
Flags: qe-verify?
We've noticed no noticeable change to traffic/load on the service, so I'm comfortable with asking to uplift this to Aurora. Pretty please?
(In reply to Michael Kelly [:mkelly,:Osmose] from comment #21)
> We've noticed no noticeable change to traffic/load on the service, so I'm
> comfortable with asking to uplift this to Aurora. Pretty please?

mkelly - nominate the patch for aurora uplift and you'll get a pre-filled in comment box with some questions to answer.
[Tracking Requested - why for this release]: The code change itself is simple, and we'd like the ability to send out new snippets or rescind snippets without a huge 24-hour delay as soon as possible to help us better control our marketing through snippets. Also, the main potential issue is whether the snippets service can handle the load or not, and we're ready to test at Aurora levels of traffic.
Let's just ignore that tracking-firefox37 nonsense and pretend that I found the right checkbox the first time, at which point I was unsure what to put down for the test coverage or UUID change fields. Any tips on what to fill in for those?
Comment on attachment 8551436 [details] [diff] [review]
patch

Let me save you the trouble :)
Attachment #8551436 - Flags: approval-mozilla-aurora+
Flags: qe-verify? → qe-verify-
Keywords: checkin-needed
Curious, was there any discussion around other techniques for avoiding the 24 hour lag? We have similar issues with Tile's once-in-24-hours fetching, and we're looking into some additional logic in Firefox to handle start-time and end-time. This requires us to send the data to Firefox ahead of time and trusting the local clock but doesn't have the increased bandwidth aspects of fetching more frequently.
You need to log in before you can comment on or make changes to this bug.