Closed Bug 1405563 Opened 2 years ago Closed 2 years ago

Uptick of failures in Firefox 57: bookmarks fails with "Duplicate entry for <guid> in changeset"

Categories

(Firefox :: Sync, defect, P1)

defect

Tracking

()

VERIFIED FIXED
Firefox 58
Tracking Status
firefox-esr52 --- unaffected
firefox56 --- unaffected
firefox57 --- wontfix
firefox58 --- fixed

People

(Reporter: markh, Assigned: lina)

References

Details

(Keywords: regression)

Attachments

(1 file)

And I think guid is most often null, but often what appears to be a valid guid.

Appears to start on Sept 9 and accounts for ~30% of the failures, putting it second to bug 1380606.

Probably not a huge deal as 57 has a higher success rate for that engine than 56 or 55, but still worth seeing if there's a sane and simple fix we can upload. I'm marking it as a regression anyway.

https://nbviewer.jupyter.org/gist/mhammond/649a6f6c5606cff0e0d9d8a4f0b9e9a1/Success-Failure-Error%20rates%20per%20engine%2C%20by%20version.ipynb
This is informative! I suspect this happens because `insertTree` didn't remove tombstones for already existing items (bug 1393904), so it's possible for the same GUID to appear in `moz_bookmarks` *and* `moz_bookmarks_deleted`. (One advantage of iOS storing tombstones and bookmarks in the same table is that collisions like this can't occur).

We always want the "real" bookmark to supersede the tombstone, and, separately, we want to drop GUIDs from `moz_bookmarks_deleted` that also exist in `moz_bookmarks`. The latter can be part of Places maintenance.
Assignee: nobody → kit
Status: NEW → ASSIGNED
Priority: -- → P1
Speaking with Kit, we've decided that this is too risky for 57, especially given 57 has the highest success rate for this engine.
Comment on attachment 8915321 [details]
Bug 1405563 - Ignore and clean up tombstones for undeleted synced bookmarks.

https://reviewboard.mozilla.org/r/186518/#review191686

Looks great, thanks. I think you should get review from Mak too for the maint. task.
Attachment #8915321 - Flags: review?(markh) → review+
Comment on attachment 8915321 [details]
Bug 1405563 - Ignore and clean up tombstones for undeleted synced bookmarks.

Mak, how does this Places maintenance task look to you? See comment 1 for context. This is going to conflict with the task in bug 1380606; I'll rebase on top of whichever one lands first.
Attachment #8915321 - Flags: review?(mak77)
Comment on attachment 8915321 [details]
Bug 1405563 - Ignore and clean up tombstones for undeleted synced bookmarks.

https://reviewboard.mozilla.org/r/186518/#review192748

As you said, it will need some unbitrot with the other bug, but nothing too complex.
Attachment #8915321 - Flags: review?(mak77) → review+
Pushed by kcambridge@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/a97ced54c839
Ignore and clean up tombstones for undeleted synced bookmarks. r=mak,markh
https://hg.mozilla.org/mozilla-central/rev/a97ced54c839
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 58
Telemetry indicates this is indeed fixed - nice work!
Status: RESOLVED → VERIFIED
Depends on: 1447832
You need to log in before you can comment on or make changes to this bug.