Request: Configurable Sync Interval

RESOLVED WONTFIX

Status

--
enhancement
RESOLVED WONTFIX
10 years ago
9 years ago

People

(Reporter: dp11misc, Unassigned)

Tracking

unspecified
Future
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Reporter)

Description

10 years ago
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9) Gecko/2008052906 Firefox/3.0
Build Identifier: 0.2

"Service.Main" begins the process "Running scheduled sync" every 60 seconds. This should be a user configurable interval to better accommodate those of us with under powered computers or limited bandwidth.

Ideally it should also allow the interval to be set to 0 which would result in the scheduled sync being disabled and Weave only syncing on demand.

Reproducible: Always

Steps to Reproduce:
1.
2.
3.



If turning scheduled sync on and off (as well as changing the schedule interval) was easily accessible in Preferences, this would probably also fix Bug 436464 (Ability to have Weave pause syncing).

It would also mitigate, to some extent, the situation seen in Bug 443085 (Delay between sync tryes must contain random part to prevent rush on locks) but, ideally, the two could be combined to add a random number of seconds to the configurable interval.

Updated

10 years ago
Duplicate of this bug: 444991

Comment 2

10 years ago
Thanks Miha, I hadn't noticed this bug before.

Comment 3

10 years ago
This would help with a lot of the problems I am seeing

Comment 4

10 years ago
Created attachment 333952 [details] [diff] [review]
proposed patch

Adds sync tunable to weave... affects files:
chrome/content/preferences.xul
chrome/locale/en-US/preferences.dtd
defaults/preferences/sync.js
modules/service.js
Attachment #333952 - Flags: review?

Comment 5

10 years ago
Created attachment 333953 [details] [diff] [review]
adding same sync patch uncompressed

Adds sync tunable to weave... affects files:
chrome/content/preferences.xul
chrome/locale/en-US/preferences.dtd
defaults/preferences/sync.js
modules/service.js
Attachment #333952 - Attachment is obsolete: true
Attachment #333952 - Flags: review?

Updated

10 years ago
Attachment #333953 - Flags: review?(Todd_Cooper)

Updated

10 years ago
Attachment #333953 - Flags: review?(Todd_Cooper) → review?(thunder)

Comment 6

10 years ago
Comment on attachment 333953 [details] [diff] [review]
adding same sync patch uncompressed

>+<!ENTITY syncInterval.description           "Sync interval in seconds">

Seconds sounds like the wrong unit for a user-facing feature.  I think minutes would be more appropriate.

> // How long we wait between sync checks.
>-const SCHEDULED_SYNC_INTERVAL = 60 * 1000 * 5; // five minutes
>+//const SCHEDULED_SYNC_INTERVAL = 60 * 1000 * 5; // five minutes

Remove if no longer needed.

>+  let local_syncInterval = Utils.prefs.getIntPref("syncInterval");
>+  this._log.config("Weave sync interval set to " + local_syncInterval);

nit: remove "local_", or call it localSyncInterval.
nit2: since we print this information when the timer is set up (below), we don't need to print it here, right?

>+    let local_syncInterval = Utils.prefs.getIntPref("syncInterval");
>+    this._scheduleTimer.initWithCallback(listener,   
>+					 local_syncInterval,
>+					 // was SCHEDULED_SYNC_INTERVAL,
>                                          this._scheduleTimer.TYPE_REPEATING_SLACK);
>+    this._log.config("Weave sync set to " + local_syncInterval);

Same style nits as above: remove commented-out code, use camelCase variable names.

Aside from the above, I have some additional concerns:

1) we should reset the timer when that pref changes.  See our observer implementation, it should be easy to add.

2) the sync interval is not the only value that governs how often data will get synced.  Tracker scores, thresholds, and the decrement step all play a part as well.  Thus, I think it would be misleading to have this pref ui as-is (but I think it would be too complicated to have the other values exposed).  Perhaps a drop-down with some simpler options might be a good alternative.  For example:

Attempt to sync items [As soon as possible        |v]
                      |As soon as possible          |
                      |Within a few hours           |
                      |Never, I will do it manually |
                      +-----------------------------+

Note that we can still have preferences for each value in about:config.

Clearing review? for now.
Attachment #333953 - Flags: review?(thunder)

Comment 7

10 years ago
Btw Todd, your patch is a great start, and I think a lot of users (specially 'power' users) want this feature.  So thanks for helping out!

If you are working on this bug, you can take it (assign it to yourself), that way others can know someone is working on it.

Cc'ing cbeard for his opinion on the prefs UI.
Status: UNCONFIRMED → NEW
Ever confirmed: true

Comment 8

10 years ago
I do not have enough privs. to assign this to myself.

I am fixing the nits and changing the UI to read minutes.
I am looking into the other changes.

Comment 9

10 years ago
Created attachment 334325 [details] [diff] [review]
add sync patch with nits fixed and UI in minutes
Attachment #333953 - Attachment is obsolete: true
Attachment #334325 - Flags: review?

Updated

10 years ago
Attachment #334325 - Flags: review? → review?(thunder)

Comment 10

10 years ago
I don't see how the observer implementation works.
Is there a place that the code runs through when an item in the UI changes?

Also I think I like the idea of syncing with a rougher idea.
We can even tune the other changes to scale (namely the 75 threshold)

I would propose
Attempt to sync items [Every hour                 |v]
                      |As soon as possible          |
                      |Every hour                   |
                      |Every few hours              |
                      |Every day                    |
                      |Never, I will do it manually |
                      +-----------------------------+

I would make the default every hour and warn users that syncing more often
could cause performance problems.

Also, how can I change the assigned to to me?

Comment 11

10 years ago
Oops, sorry for not reviewing this before.  You can assign the bug to yourself (if you're still interested) by going to the top of the bug where it says "Assigned To" and clicking the (edit) link there.

Setting for a future version, though if you add the drop-down UI as above I would review and land for 0.3.
Target Milestone: -- → Future

Comment 12

10 years ago
Comment on attachment 334325 [details] [diff] [review]
add sync patch with nits fixed and UI in minutes

This may have bitrotted a bit, since service.js has changed a bunch.

The patch is looking pretty good, but still has the sync interval UI in minutes, rather than the drop-down.
Attachment #334325 - Flags: review?(thunder) → review-

Updated

9 years ago
Component: Weave → General
Product: Mozilla Labs → Weave
QA Contact: weave → general

Updated

9 years ago
Duplicate of this bug: 508365
We've moved to a model where we sync on both timers and score thresholds, so I think this is WONTFIX at this point, since the UI would be not-really deterministic, except as a max duration, and I don't think we really want to let people set this to DDoS levels in the UI anyway... :)
Status: NEW → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.