Closed Bug 618403 Opened 14 years ago Closed 14 years ago

Orphan reparenting too aggressive, can lead to double bookmarks on storage version upgrade

Categories

(Firefox :: Sync, defect)

Product:
Firefox
Component:
Sync
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
VERIFIED FIXED
Tracking Flags:
Tracking Status
blocking2.0 --- betaN+
fennec 2.0b4+ ---

People

(Reporter: bugzilla, Assigned: philikon)

References

Details

(Whiteboard: [hardblocker][has patch][has review])

Attachments

(3 files, 2 obsolete files)

ashah's log
30.96 KB, text/plain
Details
sync-log
8.44 KB, text/plain
Details
patch v2
16.35 KB, patch
rnewman
: review+
Details | Diff | Splinter Review 
This morning all of my bookmarks are double. My bookmark toolbar contain double of all of my folders.
And if I edit my bookmarks all of the are double.
This happended this morning

I use sync to sync between my home pc and my work pc

I have the sync logs if that helps
Mozilla/5.0 (Windows NT 6.0; rv:2.0b8pre) Gecko/20101209 Firefox/4.0b8pre - Build ID: 20101209030340
(In reply to comment #0)
> I have the sync logs if that helps

Please attach them. Thanks!
Attached file sync logs (obsolete) — 
attached are my sync logs for both the home pc and my work pc.
I've included both the about:sync and about:sync-1 logs

hope it helps
Comment on attachment 496941 [details]
sync logs

hmmm those are my old sync as an extension logs. where is the firefox sync logs?
Attachment #496941 - Attachment is obsolete: true
(In reply to comment #4)
> hmmm those are my old sync as an extension logs. where is the firefox sync
> logs?

Minefield does not log by default. See http://philikon.wordpress.com/2010/11/12/sync-in-firefox-4-0-beta-7/
blocking2.0: --- → ?
tracking-fennec: --- → ?
Attached file ashah's log — 
So... this was in pastebin from the dupe, from the look of this log, the dupe detection failed earlier than this machine, since you can see where it's adding dupes in the same sync.

The tricky part is that we didn't change dupe detection, so it's not clear if this is a regression in b8.  It blocks final, for sure, but need more info to make a call for b8.
approval per mconnor.
blocking2.0: ? → final+
tracking-fennec: ? → 2.0+
I think I wrote that all my bookmarks are double.

But it turns out it's only the folders within the "Bookmarks Toolbar" that are double.
(In reply to comment #9)
> I think I wrote that all my bookmarks are double.
> 
> But it turns out it's only the folders within the "Bookmarks Toolbar" that are
> double.

Thanks for that info, that might help us track down the actual problem. Mossop told me last night on IRC that his toolbar bookmarks were reordered, too, so it looks like something specific to the toolbar broke.
blocking2.0: final+ → beta9+
tracking-fennec: 2.0+ → 2.0b4+
Multiple folders in my bookmarks are showing double bookmarks in them. There doesn't seem to be much logic to it though, some folders all the bookmarks are duplicated, some folders none are, some only one or two are duplicated (in one case I have one bookmark with six copies and all the others are just singles). In one case the duplicated bookmark had different URLs, I'm guessing one came from an older profile that I tried to sync with in 3.6.
I wonder if bug 617521 is in play for some of these cases.... did everyone affected use nightlies?
Not sure if I'm supposed to confirm that I'm also affected by this bug. At least I think so, but it definitely isn't all of my bookmarks, and it's some of the bookmarks inside the Bookmarks Menu, too.

At this point I'm on one of the machines that was NOT damaged, but now it has duplicated bookmarks, including many pairs that I had already fixed.
Actually, in the two places where I've noticed it, they seem to be breeding more and more.

I really with that OSS could succeed, but the economic model seems to be fatally broken. There needs to be more funding for more effective testing.
I backuped my place.sqlite file and let Sync 1.6 sync my bookmarks.
Then all unsorted bookmarks are duplicated. After this, I use the old place.sqlite file to replace the new one which is "synced". 
Open firefox and sync again. No more duplicated bookmarks happens.
In my previous email, /with that/ should have been /wish that/. Trivial matter, but the testing problem continues to loom.

Mostly just commenting that I haven't seen any fresh damage. I'm not sure if I fixed all of the damage myself, or if someone at the other end helped out. I'll be keeping an eye on it.

If I felt more positive about the situation, I'd go offer a suggestion for a new feature... However, as it stands, and in the absence of a belief that the testing would be sufficient to implement it safely, I think I won't... That's another kind of example of why reliability and testing are important.
(In reply to comment #17)
> That's another kind of example of why reliability and testing are important.

At the risk of using a bug tracker for side discussions: the Services QA team actually do a really great job of picking up the bugs that we developers don't catch. I've never worked with better QA folks than I have at Mozilla.

More broadly, the entire Firefox development process is built around continuous testing, including a truly enormous automated build and test suite:

http://tbpl.mozilla.org/

Unfortunately, even automated tests and great QA are unable to catch every possible problem with software -- the number of combinations of OSes, usage patterns, add-ons, and network environments are simply too great. I occasionally see bug reports that I'm simply unable even to reproduce locally.

That's why our large community of active nightly and beta users are so important: so we can ensure that our stable releases are rock solid by catching bugs that we miss during development and testing, fixing them, and adding them to the test suite to prevent future regressions.

We -- both developers and testers -- appreciate your reports as you try out our pre-release builds!

Shannon, have you been using nightlies (Minefield or add-on builds), beta releases (of Firefox or the Sync add-on), or some combination?
Blocks: 621584
Attached file sync-log — 
I have restored and fixed my bookmarks on one machine, then synced it. When I tried to sync on another machine with already duplicated bookmarks, it would put the duplicates into the Sync database again.

So I exported a bookmarks.json from the machine with the fixed bookmarks, and imported it at the machine with the broken bookmarks. The bookmarks were allright after that. However, when I started a sync at the latter machine, it took a few minutes, and then it failed with an "unknown error".

When retrying, it always fails again with the same reason. It seems that on that machine, Sync is completely broken now. :(

This attachment contains the sync-log. I hope it'll be useful for you to find and fix the bug. (I have removed the name of my private sync server.)
(In reply to comment #20)
> I have restored and fixed my bookmarks on one machine, then synced it. When I
> tried to sync on another machine with already duplicated bookmarks, it would
> put the duplicates into the Sync database again.

You could also have reset sync, setting it to replace all data on other computers.

> This attachment contains the sync-log. I hope it'll be useful for you to find
> and fix the bug. (I have removed the name of my private sync server.)

Unfortunately the log doesn't contain enough information, but you can easily turn on more logging for the bookmarks engine. Just set services.sync.log.logger.engine.bookmarks to Trace in about:config. Then restart and sync again. The log will now contain the individual bookmark records, hopefully with a clue of what's going wrong. If you don't want that information to be public, you may also send it to me privately via email.
(In reply to comment #21)
> You could also have reset sync, setting it to replace all data on other
> computers.

I tried so on a third machine. It left the bookmark bar in a total mess. All bookmark folders and their contents were left in a random order. Finally Sync failed again with an "unknown error".

Sync 1.6 is still available on addons.mozilla.org! I strongly recommend to remove it from there, to avoid that even more harm is done to more people, and to avoid even more damage to the reputation of this great project...

I also wonder why this bug still has a "normal" importance, when it acutally damages bookmark databases.

As for me, I deactivated the Sync plugin for the time being, as it hampers my daily work.
I noticed that starting with Sync 1.6, some major changes were made to the way the bookmarks are stored (see Bug 615285 and Bug 615410). I wonder if this bug could be a migration issue? What would happen if Sync 1.6 is connected to a pre-Sync 1.6 database (with predecessors and old-style IDs, but without a list of children and GUIDs)? For example, I have found no part in the current source code that would handle the predecessor annotation of a pre-Sync 1.6 database, which would explain why in comment #22 the bookmarks were in a random order.

A migration issue would also explain why unit tests (like test_bookmark_order.js) did not fail, as they test against a clean slate with no bookmarks.
(In reply to comment #23)
> I noticed that starting with Sync 1.6, some major changes were made to the way
> the bookmarks are stored (see Bug 615285 and Bug 615410). I wonder if this bug
> could be a migration issue?

The problems are very likely related to these two changes indeed. It's not a migration issue because we don't, well, migrate :)

> What would happen if Sync 1.6 is connected to a pre-Sync 1.6 database 

I'm not sure what you mean by database here. Do you mean the local Places db or the server? It doesn't really matter, though, because there's no actual migration needed.

In the local Places db Sync now uses different annotations but it will create them on the fly. The old ones will be ignored, they're no longer needed.

On the server, Sync 1.5 and 1.6 are incompatible. Sync 1.6 has a newer storage format version precisely because of changes like the one you mentioned. When Sync 1.6 encounters an account that was previously connected to Sync 1.5, it reuploads all data in the new format.

> For example, I have found no part in the current source
> code that would handle the predecessor annotation of a pre-Sync 1.6 database,

There's no need to. The predecessor annotation isn't what's used to maintain order in Places. Places maintains the order itself, Sync just used that annotation for internal purposes. Since we no longer use predecessors to track ordering, we don't need to use that annotation.

> which would explain why in comment #22 the bookmarks were in a random order.

Sadly it doesn't.

> A migration issue would also explain why unit tests (like
> test_bookmark_order.js) did not fail, as they test against a clean slate with
> no bookmarks.

Sadly this is not the case at all.
To Mr Newman's peripheral response to my peripheral comments:

I am just a user, probably a power user, and I almost never volunteer to use any software that isn't supposedly in production use. However, I know a bit about programming since I made my living as a programmer for some years, mostly for database applications. I can say that Mozilla seems to be doing a relatively good job of testing, but not good enough when they jeopardize my bookmarks this way.

These days I'm just part of the staff for a large research lab for a big company, but they are big supporters of OSS. Still, looking at the big picture, I think the economic models of OSS are NOT succeeding. I think there are solutions, and here is one I advocate:

http://eco-epistemology.blogspot.com/2009/11/economics-of-small-donors-reverse.html

I have no financial interest in it, though I think it would get some of my money. There are lots of programs I would like to support, but my main donation to OSS these years was time wasted on Ubuntu as it went down the tubes...

Back to everyone:

As far as the bug at hand, from reading all of the posts here, I still cannot tell if the bug was ever identified or fixed. It seems to be okay here--but the current status message from Sync is an "Error While Syncing" message. Actually, I'm running on Ubuntu and Windows XP right now, and both of them have the error status...
(In reply to comment #25)

> I am just a user, probably a power user, and I almost never volunteer to use
> any software that isn't supposedly in production use.

Does that mean you're still using Firefox 3.6.12 and Sync 1.5.1?

If so, you should not have been bitten by this bug, and we'd be very keen to know if you have. If not, then you apparently did volunteer to use software that isn't yet stable or widely released...

> As far as the bug at hand, from reading all of the posts here, I still cannot
> tell if the bug was ever identified or fixed.

Philipp has identified some issues in the bookmarks engine; he's working on that now, and I have every confidence that he'll fix whatever he finds (at some cost to his sanity!)

> It seems to be okay here--but the
> current status message from Sync is an "Error While Syncing" message. 
> Actually, I'm running on Ubuntu and Windows XP right now, 
> and both of them have the error status...

The only way to figure out what's wrong is to look at your about:sync-log. Please post that in a new bug, under Mozilla Services : Firefox Sync Backend, and we'll take a look.
(In reply to comment #26)
> Does that mean you're still using Firefox 3.6.12 and Sync 1.5.1?
> 
> If so, you should not have been bitten by this bug, and we'd be very keen to
> know if you have. If not, then you apparently did volunteer to use software
> that isn't yet stable or widely released...

I have to disagree here!

I was using Firefox 3.6.13 and Sync 1.6 when the bug hit me. Both were available to the common public at mozilla.org and addons.mozilla.org, with no hint that it would be beta software and not suited for production use.

At addons.mozilla.org, it is even explicitly recommended to always use the latest version of an addon: https://addons.mozilla.org/de/firefox/addon/10868/versions/
(In reply to comment #27)

> I was using Firefox 3.6.13 and Sync 1.6 when the bug hit me.

Ah yes, Sync 1.6. That'll explain it. I forgot that 1.6 had come out of beta; sorry.

In the case of the add-on, it reached a stable release (after five betas) before the bug was reliably observed. For you this is just a straight-up bug.
(In reply to comment #28)
> Ah yes, Sync 1.6. That'll explain it. I forgot that 1.6 had come out of beta;
> sorry.

So it seems that you haven't read my comment #22... :(
As per today's meeting, beta 9 will be a time-based release. Marking these all betaN+. Please move it back to beta9+ if you believe it MUST be in the next beta (ie: trunk is in an unshippable state without this)
No longer blocks: 621584
blocking2.0: beta9+ → betaN+
Undoing some damage from LegNeato's script.
Blocks: 621584
Fixing fields my automated script accidentally blanked. Apologies for the bugspam
I'm hesitant to spin off a new bug report on the basis of a bug that I haven't even seen in a while. However, if I understand your new data, what you telling me is that users are effectively picked at random to be testers. I have NOT, to the best of my knowledge, volunteered to run any non-production software--but it seems that the 'timed release' mechanism will allow my Firefox to run beta software simply because I am unlucky. In addition, as a power user running several computers at a time, I would seem to be at extra risk of bad luck. (Unless someone clarifies why my understanding is incorrect, I will take it as additional evidence of why the OSS financial model is not working well. I certainly appreciate the tediousness of proper testing--but I appreciate honesty and full disclosure even more. (My 'counter-proposal' would be to budget for testing work, with priority in volunteering to test going to the virtual shareholders of the project in question.))

As regards the actual bug considered from a database design perspective, the definitive solution would require historical tracking of all of the bookmarks including the individual machines where events take place, with some efforts to determine reliable timestamps. They are not independent events, so (just to pick one nasty example) deleting a bookmark on one machine may well be followed by a correct-but-heuristically-confusing decision to re-add the same bookmark on a different machine. I'm almost sure that you are not using such a definitive design, but rather using something with heuristics and sanity checks. That would imply this bug was probably a one-off or race condition of some kind, and I'm mostly just glad I don't have to look at the code.
(In reply to comment #33)
> However, if I understand your new data

What new data?

> me is that users are effectively picked at random to be testers.

That's not the case. Nightly and beta releases are opt-in, and a much more difficult channel to find, which discourages naïve users.

> it seems that the 'timed release' mechanism will allow my Firefox to run beta
> software simply because I am unlucky.

Nope. If you're running nightlies they'll prompt to upgrade (not much point otherwise), but stable versions will upgrade to stable versions.

> (Unless someone clarifies why my understanding is incorrect, I will take it as
> additional evidence of why the OSS financial model is not working well. I
> certainly appreciate the tediousness of proper testing--but I appreciate
> honesty and full disclosure even more. (My 'counter-proposal' would be to
> budget for testing work, with priority in volunteering to test going to the
> virtual shareholders of the project in question.))

Are you kidding?

Do you realize that Mozilla employs a large number of full-time QA staff (2 or 3 working on Sync alone, as I recall), let alone the work of an enormous community of contributors?

Sync in particular is currently developed almost entirely by Mozilla staff. Aside from the fact that you can look at and use our code and documents freely, it's developed just like commercial software. Open-source really doesn't factor into whatever your complaint is.

> As regards the actual bug considered from a database design perspective ...

Sync -- like most real software -- has to work in an environment in which it doesn't have complete control. In the case of bookmarks, we're constrained by having to look at Firefox's Places database. Green-field suggestions are not very helpful, I'm afraid, but thanks for your enthusiasm.

If you have any further insight into the meat of this bug -- preferably reproducing it -- I'd be happy to hear it, but I'd like to remind everyone to refrain from discussions of how "the dreaded open source" and Mozilla's apparently complete lack of testing was evidently its root cause.
Summary: Double bookmarks → Double bookmarks from Firefox Sync
1) Please, change platform from Windows to All, Linux and Mac are also affected.

2) For what it's worth, double bookmarks are not reflected back to the source. I mean, I have one "main" Firefox instance on a laptop and I sometimes use other computers which then are synced. After last sync update all other Firefox instances have double bookmarks after syncing but not the main instance. It seems Sync somehow ignores previous bookmarks in other Firefox instances (but doesn't delete them) and they are not synced back to the server.

If you need some additional info, just ask.
Sorry, my mistake, bookmarks are doubled, I just didn't notice them.

(In reply to comment #9)
> I think I wrote that all my bookmarks are double.
> 
> But it turns out it's only the folders within the "Bookmarks Toolbar" that are
> double.

I can't confirm this, all of my bookmarks get doubled. :(

For me this is a very serious issue, it creates a complete mess in bookmarks which is crucial data.
OS: Windows Vista → All
Hardware: x86 → All
Could you please withdrawn the newest Sync version before more people upgrade and mess their bookmarks? And bump the severity, it's critical.
(In reply to comment #37)
> Could you please withdrawn the newest Sync version before more people upgrade
> and mess their bookmarks? And bump the severity, it's critical.

Thank you for your feedback above. We're sorry you're having these issues, we're working on fixing them and will be releasing updates shortly. In the mean time, you can restore your bookmarks from backups that Sync makes before each operation: https://philikon.wordpress.com/2010/12/28/bookmark-sync-issues/
OS: All → Windows Vista
Hardware: All → x86
(In reply to comment #38)
> In the mean
> time, you can restore your bookmarks from backups that Sync makes before each
> operation: https://philikon.wordpress.com/2010/12/28/bookmark-sync-issues/

Thank you for instructions, I was doing it manually...

BTW, it could be nice to use diffs instead of backups which would allow to withdraw only harmful changes. This could be also integrated with Sync allowing Mozilla to remotely restore after a bug like this is discovered (of course, this could be an option). For now it's only an extension but in Firefox 4 it's integrated and all possible bugs will affect much wider audience.
Whiteboard: [hardblocker]
OS: Windows Vista → All
Hardware: x86 → All
Is there a burst of new problem activity on this? I have not (yet) noticed any bookmark doubling (or tripling), though I have seen a lot of sync failure messages over the last few days.

I don't remember if I noted that I am also a Windows and Ubuntu user. However, at the time I was seeing the bug, I only remember one time when it crossed over to a Ubuntu machine. For some reason I had a pretty strong impression that the bug was mostly acting between two Windows XP machines...

On the testing topic, I will say that I have NOT done anything to encourage my machine to load any non-release software. However, I have noticed long delays between updates on machines... If there is a place to turn such an option off, I would do so. I don't want to go so far as to say that there is no such thing as too much testing, and I do want to give credit to Mozilla for doing much better testing than most OSS projects, and especially much better than Ubuntu.
(In reply to comment #40)
> Is there a burst of new problem activity on this?

I think it mostly happened for users when they upgraded to Sync 1.6 / Firefox 4.0b8 which introduced a new storage format, so all data was re-uploaded and re-synced, forcing the dupe detection to kick in. After that Sync only deals with deltas and those seem to be handled fine.

The funny thing is is that Sync 1.6 / Firefox 4.0b8 did not change the dupe detection for bookmarks. Now we've had sporadic reports about duplicate bookmarks before, but not at this scale. So something's definitely wrong, but the question is whether it's an old bug or something that's been introduced recently. Fact is, the problem occurs randomly so it's been rather hard to reproduce and analyze.

One thing we did analyze and hopefully fix was the reordering problem people were seeing.

> I have not (yet) noticed any
> bookmark doubling (or tripling), though I have seen a lot of sync failure
> messages over the last few days.

Can you please go to about:sync-log and post those logs? Preferably in new bugs. If they end up being duplicates of existing ones, don't worry, we'll sort them out.

> I don't remember if I noted that I am also a Windows and Ubuntu user. However,
> at the time I was seeing the bug, I only remember one time when it crossed over
> to a Ubuntu machine. For some reason I had a pretty strong impression that the
> bug was mostly acting between two Windows XP machines...

I doubt that that makes a difference.
(In reply to comment #40)
> Is there a burst of new problem activity on this? I have not (yet) noticed any
> bookmark doubling (or tripling), though I have seen a lot of sync failure
> messages over the last few days.

Maybe because you sync often so changes are insignificant. This is best visible when you sync on a Firefox instance not used for a long time.

> I don't remember if I noted that I am also a Windows and Ubuntu user. However,
> at the time I was seeing the bug, I only remember one time when it crossed over
> to a Ubuntu machine. For some reason I had a pretty strong impression that the
> bug was mostly acting between two Windows XP machines...

My main machine is Ubuntu but bookmarks were doubled on Windows and Mac. I'm pretty sure it would be the same on my uni's PLD but I'm not sure if I want to check it.
(In reply to comment #41)

> I think it mostly happened for users when they upgraded to Sync 1.6 / Firefox
> 4.0b8 which introduced a new storage format, [...]

Again: I don't think it's related to Firefox 4 at all! This bug hit me on Firefox 3.6.13. I also doubt that all the people who complained (and are still complaining) at the add-on reviews, were using Firefox 4.

I'm using Fedora Linux on all machines, FWIW.
(In reply to comment #43)
> Again: I don't think it's related to Firefox 4 at all!

Yup. That's why I said Sync 1.6 / Firefox 4.0b8, meaning the Sync addon version 1.6 and the version of Sync that ships with Firefox 4.0b8 (they're identical).
rnewman and I analyzed the dupe detection and orphan reparenting code today. There are some academic edge cases in the dupe detection code (e.g. parallel folder structures with similarly named folders and bookmarks) but they don't explain the errors users have reported and we've been seeing randomly ourselves. Moreover, the dupe detection code wasn't modified at all in Sync 1.6 / Firefox 4.0b8.

We believe that the problem is due to the rather aggressive reparenting that BookmarksStore combined with the fact that we gave bookmarks new GUIDs in the last storage version bump.

Imagine we're syncing down a bunch of bookmarks spread over a folder structure with a high dupe count in the local profile but different GUID assignments. This is very much the scenario we had for the recent storage version upgrade, where all bookmarks received new GUIDs. So profile A would have assigned bookmarks one set of GUIDs and profile B would have assigned a different set of GUIDs to its copy of the same bookmarks.

Now imagine we sync. Due to the fact we don't actually prioritize folders right now (filed bug 625295), a lot of bookmarks will be synced down before their folders. Of course the dupe detection kicks in, so we associate the incoming bookmark with a local record. We apply the incoming record which points to some parentid. That parentid will not exist locally, though, because locally the folder was assigned a different GUID!

The rather aggressive orphan-reparenting code now moves that bookmark to the Unsorted Bookmarks folder. In theory, that'll be ok because once we sync the folder down, dupe it to the local record, we move the bookmark back into the folder. Except when you hit a problem mid-stream, say, a network error during bookmark sync. Then your bookmarks will be left in this intermediary stage. Even if you resume the sync, the local folder structure is different now because we moved a bunch of bookmarks to Unsorted, so the dupe detection no longer works for those bookmarks (because it's based on folder names). So we add bookmarks to those folders AND, once the sync completes, we move the bookmarks we had moved to Unsorted back there as well and voila, double bookmarks appear. If you hit a sync error yet again this time, you might even get triple bookmarks... (which in fact some users have reported)

This is the most likely explanation we could find for the symptoms users have consistently reported over the past weeks: bookmarks "disappearing" and being moved into Unsorted as well as double bookmarks.

The solution is to be much less aggressive in the orphan reparenting code: We actually track orphans independently of where they are located, so until we sync their missing folder down, there's no need to move them anywhere. This will leave the folder structure intact and thus the dupe detection can work reliably even when sync needs multiple attempts to complete.

In addition to that we can try to be graceful when updating local bookmark records and reparenting orphans so that sync errors don't occur when they can be in fact ignored or not recovered from anyway.
Summary: Double bookmarks from Firefox Sync → Orphan reparenting too aggressive, can lead to double bookmarks on storage version
Summary: Orphan reparenting too aggressive, can lead to double bookmarks on storage version → Orphan reparenting too aggressive, can lead to double bookmarks on storage version upgrade
Attached patch patch v1 (obsolete) — — Splinter Review 
* Don't move orphans to Unsorted Bookmarks. We simply leave them where they are until we find the right parent for them.

* Be more graceful with incoming record data in BookmarksStore.update() and when reparenting orphans.
Assignee: nobody → philipp
Status: NEW → ASSIGNED
Attachment #503420 - Flags: review?(mconnor)
Whiteboard: [hardblocker] → [hardblocker][has patch][needs review mconnor]
(In reply to comment #45)

> The solution is to be much less aggressive in the orphan reparenting code...

In addition, Bug 622762 will allow sync to complete even in the face of minor errors, which will greatly limit the scope of any problems like this in the future.
Comment on attachment 503420 [details] [diff] [review]
patch v1

This makes crossweave fail, need to investigate why. Cancelling review for now.
Attachment #503420 - Flags: review?(mconnor)
Whiteboard: [hardblocker][has patch][needs review mconnor] → [hardblocker][has wip]
Blocks: 620889
Blocks: 545647
Attached patch patch v2 — — Splinter Review 
We weren't handling delete records properly in patch v1, so here's a revised version that passes crossweave. It also does a better job of graceful input handling in store.create() and store.update(). Added unit tests for all that, too.
Attachment #503420 - Attachment is obsolete: true
Attachment #503903 - Flags: review?(rnewman)
Comment on attachment 503903 [details] [diff] [review]
patch v2

Looks good, and I got to follow the reasoning as we paired!

Verified test passes.

Nit: could you please wrap the traces at bookmarks.js:386 and 436?
Attachment #503903 - Flags: review?(rnewman) → review+
Thanks Richard, will do.
Whiteboard: [hardblocker][has wip] → [hardblocker][has patch][has review]
1.6.x: https://hg.mozilla.org/services/fx-sync/rev/a08d65f5358d
default: https://hg.mozilla.org/services/fx-sync/rev/72ec211e413a
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Blocks: 625684
If I may put my two cents here, it's not that a simple fix is just to restore bookmarks from automatically created backup. Now if I only open a browser on a long non-accessed computer all of this comes back, double bookmarks, all the mess etc. and I have to repair it again. It would be useful to be able to somehow mark current bookmarks state as demanded so that other instances copy it at next run.

I don't understand why this version wasn't pulled back from Mozilla Add-ons. Its current rating is IMHO just a reflection of this and similar decisions (compare rating of Sync and Xmarks and you'll see what I'm talking about). It's understandable to have such bugs in beta-quality software but we're talking here about Firefox 3.6.x users, too.

And no, lots of people just don't know how to repair their bookmarks considering the problem I described in the first paragraph.
(In reply to comment #54)
> If I may put my two cents here, it's not that a simple fix is just to restore
> bookmarks from automatically created backup. Now if I only open a browser on a
> long non-accessed computer all of this comes back, double bookmarks, all the
> mess etc. and I have to repair it again. It would be useful to be able to
> somehow mark current bookmarks state as demanded so that other instances copy
> it at next run.

Yes, we're going to fix that. A restore from backup on one machine should a reupload of all bookmarks automatically. I thought we had a bug on file for this, but I couldn't find it. So I filed bug 626796. Thanks for the reminder!

> I don't understand why this version wasn't pulled back from Mozilla Add-ons.

To achieve what? The damage was already done (and we're very sorry about that). I just don't see how it's better to downgrade to a known-good version rather than upgrade to a fixed version. (Also, because of the storage version upgrade from Sync 1.5 to 1.6, simply pulling 1.6 would've broken a lot of users who rely on Firefox 4 interoperability.)

> And no, lots of people just don't know how to repair their bookmarks
> considering the problem I described in the first paragraph.

We're aware of that, that's why I blogged about it to get the word out:  https://philikon.wordpress.com/2010/12/28/bookmark-sync-issues/

If you continue to see issues, please file a new bug. We want to fix any outstanding issues asap! If you'd like to discuss things in further detail, I suggest using the mailinglist. This bug is closed now and arguing about the mistakes that led to it is kind of a moot debate. Thanks!
Were solid STR's for this discovered. If so what are they?
(In reply to comment #56)
> Were solid STR's for this discovered. If so what are they?

Set up two profiles with Sync 1.5.1 syncing a good number of bookmarks between them. The more folder hierarchy there is, the more likely you are to going to hit issues. Then upgrade both profiles to Sync 1.6, one after the other. Many users were now seeing duplicate bookmarks and bookmarks being moved into the Unsorted Bookmarks folder.
This still occurs upgrading 1.5.1 to 1.6.2. 

upgraded client A then Client B.

Client A is showing many dupes. Client B is show Triplicates.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
With clean profiles (ones that hadn't been back and forth between 1.5.1 and 1.6.x versions I can't reproduce this.  update from 1.5.1 to 1.6.2 did not create any duplicate BM entries.
Status: REOPENED → RESOLVED
Closed: 14 years ago → 14 years ago
Resolution: --- → FIXED
Status: RESOLVED → VERIFIED
Updated all machines to Firefox Sync 1.6.2, and AGAIN I got a lot of duplicates. I wish there was a serious alternative to Sync. :(
(In reply to comment #63)
> Updated all machines to Firefox Sync 1.6.2, and AGAIN I got a lot of
> duplicates. I wish there was a serious alternative to Sync. :(

I'm sorry to hear that. I should note that Sync will not be able to magically remove the duplicates that were added previously. It might also be that, due to the behaviour described in this bug, it moved a lot of your duplicates into the Unsorted folder and now it's moving them back, thereby creating lots of dupes.

Please try these steps on all computers:

* Restore bookmarks from backup
* Restart
* Sync

If you're still seeing problems, please file a *new* bug and attach your logs. Thank you!
Component: Firefox Sync: Backend → Sync
Product: Cloud Services → Firefox
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: