Closed Bug 1472435 Opened 6 years ago Closed 6 years ago

bookmarks failed: Error: Error(s) encountered during statement execution: UNIQUE constraint failed

Categories

(Firefox :: Sync, defect)

63 Branch
defect
Not set
normal

Tracking

()

RESOLVED FIXED
Firefox 63
Tracking Status
firefox61 --- wontfix
firefox62 --- fixed
firefox63 --- fixed

People

(Reporter: tortino, Assigned: lina)

References

(Blocks 1 open bug)

Details

Attachments

(7 files)

Attached file error-sync-1530360012516.txt (deleted) —
User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:63.0) Gecko/20100101 Firefox/63.0 Build ID: 20180629220105 Steps to reproduce: enabled new sync engine, and started the bookmark sync. same account on different pc is syncing correctly. Actual results: sync failed Expected results: complete sync without error
Component: Untriaged → Sync
Thanks for reporting this! Could you please: * Install About Sync (https://addons.mozilla.org/en-US/firefox/addon/about-sync/), if you haven't already, and open about:sync. * Under "General Options", set "Actively looking for issues and want detailed logging". * Under "bookmarks", click the "SQL" tab, run this query, then copy-paste the results into a file: SELECT guid, id, parent, position, syncStatus, syncChangeCounter FROM moz_bookmarks ORDER BY id; ...Then sync again, and attach the new error log and SQL query output to this bug. Thanks again!
Flags: needinfo?(tortino)
If you're comfortable with it, you're also welcome send me your `places.sqlite` and `weave/bookmarks.sqlite` via private email for analysis. Both contain all your bookmarks (`places.sqlite` also contains your history), so I completely understand if you'd rather not...it just might take a bit longer to track down the issue.
(In reply to Lina Cambridge (she/her) [:lina, :kitcambridge] from comment #2) > If you're comfortable with it, you're also welcome send me... You're also welcome *to* send me. I accidentally a word. :-)
Also, Tortino, could you please upload all your logs, not just the most recent one? I wonder if a previous timeout and a failed cleanup left some state in the temp tables, and caused all future merges to fail. I'm also curious if restarting Firefox after seeing this error lets you keep syncing, or if you see the same error again.
Attached file about sync sql query output.txt (deleted) —
Flags: needinfo?(tortino)
Attached file error-sync-1531234554294.txt (deleted) —
Attached file error-sync-1531234658595.txt (deleted) —
Attached file error-sync-1531234696314.txt (deleted) —
Attached file error-sync-1531235231616.zip (deleted) —
I have no older sync logs. there were more but now they are vanished. (I didn't deleted any log). Also I keep getting the error. Today I just updated nightly to latest build, and then I got the error again.
Thanks so much for uploading those! Unfortunately, nothing stands out to me. :-( Both your local and remote trees look correct, so I'm not sure why we're trying to change an item twice. I also tried to reconstruct the databases from the logs, and was able to merge them without any errors. Would you be willing to email me your `places.sqlite` and `weave/bookmarks.sqlite`, from your profile directory? (You can also upload them to https://send.firefox.com/, and email me the link). I won't share them with anyone, and will delete them once I've figured out the issue.
Flags: needinfo?(tortino)
Thanks so much for sending those over, Tortino! I see the problem...your `places.sqlite` has two rows for the same URL: > sqlite> SELECT h.id, h.guid, h.url_hash, h.url FROM moz_places h JOIN (SELECT url_hash FROM moz_places GROUP BY url_hash HAVING count(*) > 1) d ON d.url_hash = h.url_hash; > 9555|llYUXLyl3X2S|268507875886102|place:parent=menu________&parent=unfiled_____&parent=toolbar_____&queryType=1&sort=12&maxResults=10&excludeQueries=1 > 20956|nGACqHWbgUzC|268507875886102|place:parent=menu________&parent=unfiled_____&parent=toolbar_____&queryType=1&sort=12&maxResults=10&excludeQueries=1 That breaks https://searchfox.org/mozilla-central/rev/bdfd20ef30d521b57d5b6feeda71325e8b4cad66/toolkit/components/places/SyncedBookmarksMirror.jsm#2247-2252, which assumes one row per `(url_hash, url)`. The fix for the mirror is pretty easy, and I wonder if we should also add a maintenance task for this, too. Looking at the bookmarks: > sqlite> SELECT b.dateAdded, b.lastModified FROM moz_bookmarks b WHERE b.fk IN (9555, 20956); > 1475582533000000|1529745706802000 > 1476600318383000|1529745706815000 Both added in 2016, so it's unlikely anything recent caused this.
Assignee: nobody → lina
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Flags: needinfo?(tortino)
(In reply to Lina Cambridge (she/her) [:lina] from comment #13) > I wonder if we should also > add a maintenance task for this, too. Redundant adverb is redundant. :-)
Blocks: 1433177
If `moz_places` has duplicate rows for the same URL, joining on the URL and hash includes the same item multiple times in `itemsToMerge`. This causes constraint errors when we try to record observer notifications for an item that we already saw.
Comment on attachment 8994753 [details] Ensure the `itemsToMerge` view in the bookmarks mirror ignores duplicate URLs. Marco Bonardo [::mak] has approved the revision. https://phabricator.services.mozilla.com/D2350
Attachment #8994753 - Flags: review+
Pushed by kcambridge@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/abfa418aed59 Ensure the `itemsToMerge` view in the bookmarks mirror ignores duplicate URLs. r=mak
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 63
Lina, since you set status-firefox62 to affected, want to request uplift now?
Flags: needinfo?(lina)
Comment on attachment 8994753 [details] Ensure the `itemsToMerge` view in the bookmarks mirror ignores duplicate URLs. Sure. :-) Thanks, Julien! New bookmark sync is pref'd off in 62, but this is very low risk, and should drop our error counts. Approval Request Comment [Feature/Bug causing the regression]: Bug 1305563. [User impact if declined]: Users with duplicate URLs in their Places database won't be able to use new bookmark sync. This shouldn't happen (we have a debug-only assert in https://searchfox.org/mozilla-central/rev/51268dcbdff0f6f4a5cff7986df0f616efc5bcfd/toolkit/components/places/Database.cpp#3035-3042), but it's the most common error we've seen in telemetry so far. [Is this code covered by automated tests?]: Yes. [Has the fix been verified in Nightly?]: Yes. [Needs manual test from QE? If yes, steps to reproduce]: No. [List of other uplifts needed for the feature/fix]: None. [Is the change risky?]: No. [Why is the change risky/not risky?]: Changes a few lines in a SQL query, has an xpcshell test, verified manually, and the feature is pref'd off by default. [String changes made/needed]: None.
Flags: needinfo?(lina)
Attachment #8994753 - Flags: approval-mozilla-beta?
Comment on attachment 8994753 [details] Ensure the `itemsToMerge` view in the bookmarks mirror ignores duplicate URLs. fix for bookmarks sync, approved for 62.0b16
Attachment #8994753 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Flags: in-testsuite+
Needs a rebased patch for Beta uplift.
Flags: needinfo?(lina)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: