Closed
Bug 1451537
Opened 6 years ago
Closed 6 years ago
Simplify tag query rewriting in the mirror
Categories
(Firefox :: Sync, enhancement, P2)
Firefox
Sync
Tracking
()
RESOLVED
FIXED
Firefox 61
Tracking | Status | |
---|---|---|
firefox61 | --- | fixed |
People
(Reporter: lina, Assigned: lina)
References
Details
Attachments
(2 files)
See bug 1293445, comment 39. Migration doesn't preserve other params for tag contents queries, so we can remove `rewriteRemoteTagQueries`, rewrite when we insert into `moz_places`, and *maybe* flag rewritten queries for weak reupload. (Or make sure the validator ignores them; users can fix them manually if they notice, and then we'll sync the change). These queries are already uncommon, so the simpler we can make this, the better. It's also not critical.
Updated•6 years ago
|
Priority: -- → P2
Updated•6 years ago
|
Assignee: nobody → kit
Assignee | ||
Updated•6 years ago
|
Summary: Remove tag contents query rewriting from the mirror → Simplify tag query rewriting in the mirror
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment 3•6 years ago
|
||
mozreview-review |
Comment on attachment 8967608 [details] Bug 1451537 - Simplify tag query rewriting in the bookmarks mirror. https://reviewboard.mozilla.org/r/236316/#review242160
Attachment #8967608 -
Flags: review?(mak77) → review+
Comment 4•6 years ago
|
||
mozreview-review |
Comment on attachment 8967609 [details] Bug 1451537 - Bump bookmarks mirror schema version and add a migration test. https://reviewboard.mozilla.org/r/236318/#review242164 you may want to file a follow-up to attach some kind of telemetry to this db. What we discovered with places.sqlite is that you can't really know what happens when you try to replace a corrupt db in the wild. It would be nice to have at least some basic telemetry like db size and the reached point when handling a corrupt db. ::: toolkit/components/places/SyncedBookmarksMirror.jsm:1922 (Diff revision 1) > * @param {Sqlite.OpenedConnection} db > - * The mirror database connection. > + * The Places database connection. > + * @param {String} path > + * The full path to the mirror database file. > */ > -async function migrateMirrorSchema(db) { > +async function attachMirrorDatabase(db, path) { attachAndInitMirrorDatabase?
Attachment #8967609 -
Flags: review?(mak77) → review+
Comment 6•6 years ago
|
||
mozreview-review |
Comment on attachment 8967608 [details] Bug 1451537 - Simplify tag query rewriting in the bookmarks mirror. https://reviewboard.mozilla.org/r/236316/#review242864 Sorry, I thought I already reviewed this!
Attachment #8967608 -
Flags: review?(markh) → review+
Assignee | ||
Comment 7•6 years ago
|
||
mozreview-review-reply |
Comment on attachment 8967609 [details] Bug 1451537 - Bump bookmarks mirror schema version and add a migration test. https://reviewboard.mozilla.org/r/236318/#review242164 Thanks for reviewing! I added telemetry for recording why we failed to open the database in this patch (`initialize` for non-corruption errors, `remove` if we failed to remove the old file, and `replace` if we failed to initialize the replacement database). DB size telemetry is in bug 1451152.
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment 10•6 years ago
|
||
Pushed by kcambridge@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/1fa6af3138d5 Simplify tag query rewriting in the bookmarks mirror. r=mak,markh https://hg.mozilla.org/integration/autoland/rev/8dbdf2617258 Bump bookmarks mirror schema version and add a migration test. r=mak
Comment 11•6 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/1fa6af3138d5 https://hg.mozilla.org/mozilla-central/rev/8dbdf2617258
Status: NEW → RESOLVED
Closed: 6 years ago
status-firefox61:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 61
You need to log in
before you can comment on or make changes to this bug.
Description
•