Richard Newman [:rnewman] Assignee
	Description • 11 years ago

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.

	Richard Newman [:rnewman] Assignee
	Comment 1 • 11 years ago

Sorry, that other bug is Bug 836790.

	Richard Newman [:rnewman] Assignee
	Comment 2 • 11 years ago

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.

	Nick Alexander :nalexander [he/him] (PTO until Montreallhands August 21, 2023)
	Comment 3 • 11 years ago

(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.

	Nick Alexander :nalexander [he/him] (PTO until Montreallhands August 21, 2023)
	Comment 4 • 11 years ago

(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.)

	Nick Alexander :nalexander [he/him] (PTO until Montreallhands August 21, 2023)
	Comment 6 • 11 years ago

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.

	Richard Newman [:rnewman] Assignee
	Comment 7 • 11 years ago

This is blocking a 29+ bug, so carrying over flag.

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

needinfo on me for uplift.

	Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)
	Comment 8 • 11 years ago

https://hg.mozilla.org/mozilla-central/rev/c18651a0ccb5

	Richard Newman [:rnewman] Assignee
	Comment 9 • 11 years ago

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.

	Richard Newman [:rnewman] Assignee
	Comment 10 • 11 years ago

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

	Richard Newman [:rnewman] Assignee
	Comment 11 • 11 years ago

https://hg.mozilla.org/releases/mozilla-beta/rev/82ff3f2b38b9