GNOME Bugzilla – Bug 632615
Autosync is too aggressive when editing notes
Last modified: 2011-08-20 22:19:49 UTC
See https://bugs.launchpad.net/ubuntu/+source/tomboy/+bug/599283 for a typical user reaction, whic his basically "why do my notes keep graying out?". Looking at the code, I believe this is what happens: 1) User begins editing note 2) SyncManager.HandleNoteBufferChanged disposes of the autosync timer 3) User stops editing for a few seconds, triggering a Save 4) SyncManager.HandleNoteSavedOrDeleted resurrects the autosync timer, sets it to sync in 1 minute, regardless of autosync timeout pref 5) User decides to start editing again one minute after (3), but the note is now disabled. Once sync is done, the whole cycle repeats. Conclusion: any time a user stops editing for one minute, they are blocked from editing because a sync is occurring. This could be extremely frustrating and waste server resources, too. I *think* the reason I implemented it this way was because I was concerned that if we set the autosync timeout to whatever the pref was at (4), we would be in a situation where if you touched one note every 14 minutes or so, you'd never ever autosync. But in retrospect, I don't see any reason why we shouldn't change (2) to only dispose the autosync timer if we are less than a minute away from a scheduled sync.
I think the ideal fix would be to not even block the user from editing during a sync. We could just grab the last save and sync that, ignoring whats currently in the buffer, eventually it will get saved and sync'd. Then the question becomes when and how often do we want to autosync? If a user is going back and forth editing a note every few minutes, maybe we have a soft timeout of 1 minute and a hard limit of 15. That way we're guaranteed a sync every 15 minutes or less depending on user activity. If thats not too difficult to implement we can target this milestone. Otherwise we'll stick to a simple fix and improve it later.
(In reply to comment #1) > I think the ideal fix would be to not even block the user from editing during a > sync. We could just grab the last save and sync that, ignoring whats currently > in the buffer, eventually it will get saved and sync'd. That only works if there are no updates for the note currently being edited. If we suck changes down that conflict with the new edits we are making, we're in for a potentially messy user experience.
What is the current behavior in this scenario? We lock the edit and then show the conflict dialog to the user?
Edit is always locked during a sync. In the case of a conflict, we show a somewhat clunky dialog to the user, giving them the option to discard their local changes, or copy them to a new note (so that they can later merge the content by hand). Either way, the original note is overwritten with the changes from the server.
Created attachment 173109 [details] [review] Never autosync more often than the user's settings Took Sandy's suggestion: when the buffer is changed, only drop the timer if we're close (<1 min) to an autosync. Also some minor adjustments so we never sync more often than what the user has specified, specifically the initial autosync will be the user's preference rather than 1 minute.
Review of attachment 173109 [details] [review]: This looks good. Do you have access to GNOME git? ::: Tomboy/Synchronization/SyncManager.cs @@ +239,3 @@ + TimeSpan timeSinceLastCheck = + DateTime.Now - lastBackgroundCheck; + if (timeSinceLastCheck.TotalMinutes > autosyncTimeoutPrefMinutes - 1) Style: opening { goes after the last ) on the if line @@ +279,3 @@ lastBackgroundCheck = DateTime.Now; + // Perform a sync no sooner than user specified + currentAutosyncTimeoutMinutes = autosyncTimeoutPrefMinutes; Yeah, this is probably better. I can't recall why I made it sync right after a settings change. It seems just as natural to have the first autosync happen X minutes later, where X is the pref.
(In reply to comment #6) > Review of attachment 173109 [details] [review]: > > This looks good. Do you have access to GNOME git? > No, haven't applied yet. Would you commit? > ::: Tomboy/Synchronization/SyncManager.cs > @@ +239,3 @@ > + TimeSpan timeSinceLastCheck = > + DateTime.Now - lastBackgroundCheck; > + if (timeSinceLastCheck.TotalMinutes > > autosyncTimeoutPrefMinutes - 1) > > Style: opening { goes after the last ) on the if line > Oops, do you need a new patch?
Hmm, I want to play with this a bit more before pushing, actually. Don't worry about the style change, I'll get it.
Comment on attachment 173109 [details] [review] Never autosync more often than the user's settings Changing the patch status to reflect the above comment. Sandy, any followup?
I'm not going to get a chance to test this out, realistically. If it works for you I say push, don't block on me testing it. Maybe somebody else can give it a try too.
Comment on attachment 173109 [details] [review] Never autosync more often than the user's settings Committed as 89f234fb08c8f95cb6b00f808a0f7e5896880fe0 This will be included in Tomboy 1.7.4