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)
Tracking
()
RESOLVED
FIXED
Firefox 63
People
(Reporter: tortino, Assigned: lina)
References
(Blocks 1 open bug)
Details
Attachments
(7 files)
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
Assignee | ||
Comment 1•6 years ago
|
||
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)
Updated•6 years ago
|
status-firefox63:
--- → affected
Assignee | ||
Comment 2•6 years ago
|
||
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.
Assignee | ||
Comment 3•6 years ago
|
||
(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. :-)
Assignee | ||
Comment 5•6 years ago
|
||
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.
Reporter | ||
Comment 10•6 years ago
|
||
Reporter | ||
Comment 11•6 years ago
|
||
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.
Assignee | ||
Comment 12•6 years ago
|
||
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)
Assignee | ||
Comment 13•6 years ago
|
||
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
status-firefox61:
--- → wontfix
status-firefox62:
--- → affected
Ever confirmed: true
Flags: needinfo?(tortino)
Assignee | ||
Updated•6 years ago
|
Assignee | ||
Comment 14•6 years ago
|
||
(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
Assignee | ||
Comment 15•6 years ago
|
||
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.
Assignee | ||
Comment 16•6 years ago
|
||
Comment 17•6 years ago
|
||
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+
Comment 18•6 years ago
|
||
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
Comment 19•6 years ago
|
||
bugherder |
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 63
Comment 20•6 years ago
|
||
Lina, since you set status-firefox62 to affected, want to request uplift now?
Flags: needinfo?(lina)
Assignee | ||
Comment 21•6 years ago
|
||
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 22•6 years ago
|
||
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+
Updated•6 years ago
|
Flags: in-testsuite+
Comment 24•6 years ago
|
||
bugherder uplift |
Updated•6 years ago
|
Flags: needinfo?(lina)
You need to log in
before you can comment on or make changes to this bug.
Description
•