Closed Bug 984723 Opened 6 years ago Closed 6 years ago

Further tweaks to sync scheduling

Categories

(Firefox for Android :: Android Sync, defect)

Firefox 29
All
Android
defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 31
Tracking Status
firefox28 --- unaffected
firefox29 --- fixed
firefox30 --- fixed
firefox31 --- fixed
fennec 29+ ---

People

(Reporter: rnewman, Assigned: rnewman)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

Every 30 minutes is too often. Our short-term plan:

* Drop that to 8 hours.
* Install a wake listener which can trigger background syncs when (a) the phone is awake, (b) it's on the network, (c) enough time has passed.
* Bump the debouncer time from 45 seconds to a little bigger, perhaps 90 seconds. This will reduce cost during browser use itself.

Meanwhile, nalexander is addressing Bug 836780, which should help reduce the other half of this multiplier.
Sorry, that other bug is Bug 836790.
I might have an altogether smarter way of doing this: do nothing except rate-limit. Don't schedule periodic syncs at all, or make them daily (the default is 24 hours, but 12 is acceptable).

setSyncAutomatically() -- which we call -- specifically means "sync me whenever we wake up the network". That means *often*, at least according to the Android team.

http://www.youtube.com/watch?v=5onKZcJyJwI

"This approach ensures that your app is regularly updated without having to schedule your own client side polling… that said, the frequency of network tickles can lead to a large number of transfers, so this should only be enabled when your app is in the foreground."

He goes on to suggest in-app rate limiting of these syncs.
(In reply to Richard Newman [:rnewman] from comment #0)
> Every 30 minutes is too often. Our short-term plan:
> 
> * Drop that to 8 hours.
> * Install a wake listener which can trigger background syncs when (a) the
> phone is awake, (b) it's on the network, (c) enough time has passed.
> * Bump the debouncer time from 45 seconds to a little bigger, perhaps 90
> seconds. This will reduce cost during browser use itself.
> 
> Meanwhile, nalexander is addressing Bug 836780, which should help reduce the
> other half of this multiplier.

I believe the earlier (1 year old?) PR had the incorrect ticket, and the new one has the correct ticket (Bug 836790).  For listeners reading along at home, we believe that Bug 968592 will make an even bigger difference, because we should do the minimal amount of work (one network request) to verify that nothing has changed.
(In reply to Richard Newman [:rnewman] from comment #2)
> I might have an altogether smarter way of doing this: do nothing except
> rate-limit. Don't schedule periodic syncs at all, or make them daily (the
> default is 24 hours, but 12 is acceptable).
> 
> setSyncAutomatically() -- which we call -- specifically means "sync me
> whenever we wake up the network". That means *often*, at least according to
> the Android team.
> 
> http://www.youtube.com/watch?v=5onKZcJyJwI
> 
> "This approach ensures that your app is regularly updated without having to
> schedule your own client side polling… that said, the frequency of network
> tickles can lead to a large number of transfers, so this should only be
> enabled when your app is in the foreground."
> 
> He goes on to suggest in-app rate limiting of these syncs.

This is interesting, I had forgotten that the meaning of setSyncAutomatically was interpreted in this way.  Ah, I see from http://developer.android.com/reference/android/content/ContentResolver.html#setIsSyncable%28android.accounts.Account,%20java.lang.String,%20int%29 that setIsSyncable and setSyncAutomatically are different; I was conflating the two.

Then yes, I think the correct approach is to remove our periodic sync entirely, trust to the Android network stack, and rate limit, possibly depending on Fennec's foreground status.

(On a technical note: it's nice to not have such an explicit on the Fennec app because we test against a stand-alone Sync account in android-sync.apk.  But we could make the foreground test depend on more than just Fennec being in the foreground.)
Comment on attachment 8393042 [details] [diff] [review]
Rework intervals and scheduling for Android Sync. v1

Review of attachment 8393042 [details] [diff] [review]:
-----------------------------------------------------------------

I feel like we're still not building the right abstraction or expression of the fundamental (he he) complexity of rate limiting, but let's improve things today and build better tomorrow.
Attachment #8393042 - Flags: review?(nalexander) → review+
This is blocking a 29+ bug, so carrying over flag.

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

needinfo on me for uplift.
tracking-fennec: --- → 29+
Flags: needinfo?(rnewman)
Target Milestone: --- → Firefox 31
https://hg.mozilla.org/mozilla-central/rev/c18651a0ccb5
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Comment on attachment 8393042 [details] [diff] [review]
Rework intervals and scheduling for Android Sync. v1

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 
  New Sync work in 29.

User impact if declined: 
  Battery life impact that's very detrimental to user experience.

Testing completed (on m-c, etc.): 
  Baking for quite a while. Manual testing.

Risk to taking this patch (and alternatives if risky): 
  Pretty much the only risk is that we sync less frequently than we might want -- but still more frequently than in Sync 1.1. 

String or IDL/UUID changes made by this patch:
  None.
Attachment #8393042 - Flags: approval-mozilla-beta?
Attachment #8393042 - Flags: approval-mozilla-aurora?
Flags: needinfo?(rnewman)
Attachment #8393042 - Flags: approval-mozilla-beta?
Attachment #8393042 - Flags: approval-mozilla-beta+
Attachment #8393042 - Flags: approval-mozilla-aurora?
Attachment #8393042 - Flags: approval-mozilla-aurora+
Product: Android Background Services → Firefox for Android
You need to log in before you can comment on or make changes to this bug.