Bug 1436215 - Avoid cascading deletes for text foreign keys in the mirror. r?markh draft
authorKit Cambridge <kit@yakshaving.ninja>
Tue, 06 Feb 2018 17:09:46 -0800
changeset 751884 bf95b432396e5c73e0ba025a0fdc8404665d8440
parent 751883 00d018753b331995eb424247041064c8330fbb0a
push id98084
push userbmo:kit@mozilla.com
push dateWed, 07 Feb 2018 01:53:52 +0000
reviewersmarkh
bugs1436215
milestone60.0a1
Bug 1436215 - Avoid cascading deletes for text foreign keys in the mirror. r?markh These can take minutes to run, while using integer foreign keys takes milliseconds. The only time we delete from the mirror is in `reset`, so we can make this more efficient by clearing the `structure` table first, so that there are no foreign key references to clean up when we clear `items`. For `itemsToUpload`, we can simply use integer IDs everywhere. MozReview-Commit-ID: rTfJzjPHi1
toolkit/components/places/SyncedBookmarksMirror.jsm
--- a/toolkit/components/places/SyncedBookmarksMirror.jsm
+++ b/toolkit/components/places/SyncedBookmarksMirror.jsm
@@ -435,16 +435,17 @@ class SyncedBookmarksMirror {
    * reassigned, disables the bookmarks engine, or signs out.
    */
   async reset() {
     await this.db.executeBeforeShutdown(
       "SyncedBookmarksMirror: reset",
       async function(db) {
         await db.executeTransaction(async function() {
           await db.execute(`DELETE FROM meta`);
+          await db.execute(`DELETE FROM structure`);
           await db.execute(`DELETE FROM items`);
           await db.execute(`DELETE FROM urls`);
 
           // Since we need to reset the modified times for the syncable roots,
           // we simply delete and recreate them.
           await createMirrorRoots(db);
         });
       }
@@ -1386,23 +1387,23 @@ class SyncedBookmarksMirror {
       WITH RECURSIVE
       syncedItems(id, level) AS (
         SELECT b.id, 0 AS level FROM moz_bookmarks b
         WHERE b.guid IN (:menuGuid, :toolbarGuid, :unfiledGuid, :mobileGuid)
         UNION ALL
         SELECT b.id, s.level + 1 AS level FROM moz_bookmarks b
         JOIN syncedItems s ON s.id = b.parent
       )
-      INSERT INTO itemsToUpload(guid, syncChangeCounter, parentGuid,
+      INSERT INTO itemsToUpload(id, guid, syncChangeCounter, parentGuid,
                                 parentTitle, dateAdded, type, title, isQuery,
                                 url, tags, description, loadInSidebar,
                                 smartBookmarkName, keyword, feedURL, siteURL,
                                 position)
-      SELECT b.guid, b.syncChangeCounter, p.guid, p.title, b.dateAdded, b.type,
-             b.title, IFNULL(SUBSTR(h.url, 1, 6) = 'place:', 0), h.url,
+      SELECT b.id, b.guid, b.syncChangeCounter, p.guid, p.title, b.dateAdded,
+             b.type, b.title, IFNULL(SUBSTR(h.url, 1, 6) = 'place:', 0), h.url,
              (SELECT GROUP_CONCAT(t.title, ',') FROM moz_bookmarks e
               JOIN moz_bookmarks t ON t.id = e.parent
               JOIN moz_bookmarks r ON r.id = t.parent
               WHERE b.type = :bookmarkType AND
                     r.guid = :tagsGuid AND
                     e.fk = h.id),
              (SELECT a.content FROM moz_items_annos a
               JOIN moz_anno_attributes n ON n.id = a.anno_attribute_id
@@ -1448,51 +1449,50 @@ class SyncedBookmarksMirror {
         folderType: PlacesUtils.bookmarks.TYPE_FOLDER,
         feedURLAnno: PlacesUtils.LMANNO_FEEDURI,
         siteURLAnno: PlacesUtils.LMANNO_SITEURI });
 
     // Record tag folder names for tag queries. Parsing query URLs one by one
     // is inefficient, but queries aren't common today, and we can remove this
     // logic entirely once bug 1293445 lands.
     let queryRows = await this.db.execute(`
-      SELECT guid, url FROM itemsToUpload
+      SELECT id, url FROM itemsToUpload
       WHERE isQuery`);
 
     let tagFolderNameParams = [];
     for (let row of queryRows) {
       let url = new URL(row.getResultByName("url"));
       let tagQueryParams = new URLSearchParams(url.pathname);
       let type = Number(tagQueryParams.get("type"));
       if (type == Ci.nsINavHistoryQueryOptions.RESULTS_AS_TAG_CONTENTS) {
         continue;
       }
       let tagFolderId = Number(tagQueryParams.get("folder"));
       tagFolderNameParams.push({
-        guid: row.getResultByName("guid"),
+        id: row.getResultByName("id"),
         tagFolderId,
         folderType: PlacesUtils.bookmarks.TYPE_FOLDER,
       });
     }
 
     if (tagFolderNameParams.length) {
       await this.db.execute(`
         UPDATE itemsToUpload SET
           tagFolderName = (SELECT b.title FROM moz_bookmarks b
                            WHERE b.id = :tagFolderId AND
                                  b.type = :folderType)
-        WHERE guid = :guid`);
+        WHERE id = :id`);
     }
 
     // Record the child GUIDs of locally changed folders, which we use to
     // populate the `children` array in the record.
     await this.db.execute(`
-      INSERT INTO structureToUpload(guid, parentGuid, position)
-      SELECT b.guid, p.guid, b.position FROM moz_bookmarks b
-      JOIN moz_bookmarks p ON p.id = b.parent
-      JOIN itemsToUpload o ON o.guid = p.guid`);
+      INSERT INTO structureToUpload(guid, parentId, position)
+      SELECT b.guid, b.parent, b.position FROM moz_bookmarks b
+      JOIN itemsToUpload o ON o.id = b.parent`);
 
     // Finally, stage tombstones for deleted items. Ignore conflicts if we have
     // tombstones for undeleted items; Places Maintenance should clean these up.
     await this.db.execute(`
       INSERT OR IGNORE INTO itemsToUpload(guid, syncChangeCounter, isDeleted,
                                           dateAdded)
       SELECT guid, 1, 1, dateRemoved FROM moz_bookmarks_deleted`);
   }
@@ -1503,17 +1503,17 @@ class SyncedBookmarksMirror {
    * @return {Object.<String, BookmarkChangeRecord>}
    *         A changeset containing Sync record cleartexts for outgoing items
    *         and tombstones, keyed by their Sync record IDs.
    */
   async fetchLocalChangeRecords() {
     let changeRecords = {};
 
     let itemRows = await this.db.execute(`
-      SELECT syncChangeCounter, guid, isDeleted, type, isQuery,
+      SELECT id, syncChangeCounter, guid, isDeleted, type, isQuery,
              smartBookmarkName, IFNULL(tagFolderName, "") AS tagFolderName,
              loadInSidebar, keyword, tags, url, IFNULL(title, "") AS title,
              description, feedURL, siteURL, position, parentGuid,
              IFNULL(parentTitle, "") AS parentTitle, dateAdded
       FROM itemsToUpload`);
 
     for (let row of itemRows) {
       let syncChangeCounter = row.getResultByName("syncChangeCounter");
@@ -1636,19 +1636,19 @@ class SyncedBookmarksMirror {
             title: row.getResultByName("title"),
           };
           let description = row.getResultByName("description");
           if (description) {
             folderCleartext.description = description;
           }
           let childGuidRows = await this.db.executeCached(`
             SELECT guid FROM structureToUpload
-            WHERE parentGuid = :guid
+            WHERE parentId = :id
             ORDER BY position`,
-            { guid });
+            { id: row.getResultByName("id") });
           folderCleartext.children = childGuidRows.map(row => {
             let childGuid = row.getResultByName("guid");
             return PlacesSyncUtils.bookmarks.guidToRecordId(childGuid);
           });
           changeRecords[recordId] = new BookmarkChangeRecord(
             syncChangeCounter, folderCleartext);
           continue;
         }
@@ -2409,17 +2409,18 @@ async function initializeTempMirrorEntit
 
   await db.execute(`CREATE TEMP TABLE itemsToWeaklyReupload(
     id INTEGER PRIMARY KEY
   )`);
 
   // Stores locally changed items staged for upload. See `stageItemsToUpload`
   // for an explanation of why these tables exists.
   await db.execute(`CREATE TEMP TABLE itemsToUpload(
-    guid TEXT PRIMARY KEY,
+    id INTEGER PRIMARY KEY,
+    guid TEXT UNIQUE NOT NULL,
     syncChangeCounter INTEGER NOT NULL,
     isDeleted BOOLEAN NOT NULL DEFAULT 0,
     parentGuid TEXT,
     parentTitle TEXT,
     dateAdded INTEGER,
     type INTEGER,
     title TEXT,
     isQuery BOOLEAN NOT NULL DEFAULT 0,
@@ -2428,22 +2429,22 @@ async function initializeTempMirrorEntit
     description TEXT,
     loadInSidebar BOOLEAN,
     smartBookmarkName TEXT,
     tagFolderName TEXT,
     keyword TEXT,
     feedURL TEXT,
     siteURL TEXT,
     position INTEGER
-  ) WITHOUT ROWID`);
+  )`);
 
   await db.execute(`CREATE TEMP TABLE structureToUpload(
     guid TEXT PRIMARY KEY,
-    parentGuid TEXT NOT NULL REFERENCES itemsToUpload(guid)
-                             ON DELETE CASCADE,
+    parentId INTEGER NOT NULL REFERENCES itemsToUpload(id)
+                              ON DELETE CASCADE,
     position INTEGER NOT NULL
   ) WITHOUT ROWID`);
 }
 
 // Converts a Sync record ID to a Places GUID. Returns `null` if the ID is
 // invalid.
 function validateGuid(recordId) {
   let guid = PlacesSyncUtils.bookmarks.recordIdToGuid(recordId);