2039791 - Bookmark syncs failing with dogear errors

Status: RESOLVED FIXED

(Application Services :: Places, defect, P2)

Description

[Mark Hammond [:markh] [:mhammond]]

Telemetry is showing many bookmark sync errors. The errors are reported as, eg, Store error: Error merging: Can't insert Bookmark into nonexistent parent, which correspond to dogear's ErrorKind::MissingParentForUnknownChild and ErrorKind::InvalidParent.

It's very odd these these have started relatively recently. dogear hasn't changed in over 2 years, bookmark syncing hasn't changed in any meaningful way which should impact this. Even "bad" incoming records shouldn't cause this.

I haven't tried to work out how many user's are affected, but each impacted user is almost certainly going to hit this on every single sync - so the number of reports is going to be higher than the impacted users. But it's still quite bad.

Both fire from dogear's ParentBuilder::by_children / by_structure. The triggering call lives in app-services, bookmark sync's engine.rs, the third pass of fetch_remote_tree. That pass walks every row of moz_bookmarks_synced_structure and asserts each (guid, parentGuid) edge with the strict by_children.

The two errors correspond to two different bad states in moz_bookmarks_synced_structure:
Error: MissingParentForUnknownChild
Required state: Structure row whose child and parent are both missing from the builder (tombstoned with isDeleted=1,
or
absent from moz_bookmarks_synced entirely).

Error: InvalidParent
Required state: Structure row whose parent exists in moz_bookmarks_synced with isDeleted=0 AND kind != Folder (i.e.,
kind is Bookmark, Query, Livemark, or Separator).

The schema has parentGuid REFERENCES moz_bookmarks_synced(guid) ON DELETE CASCADE but no FK on the child guid column and no constraint on the parent's kind. With foreign_keys=ON, as we do, the cascade should prevent the MissingParentForUnknownChild shape.

So even though these things should remain impossible, we see them. Only in telemetry, so it's very difficult to see why.

What we can do is 2-fold:

• Tighten up the sql, so we only only select items from moz_bookmarks_synced_structure that aren't problematic when merging.
• Add some telemetry to try and work out which of the things is going wrong, which might give us more insight into the root cause.
  A third thing we can consider is making dogear a little more resilient here - there are similar cases where dogear just lets things slide.

Comment 1

[BMO Github Automation]

Attachment: [mozilla/application-services] Bug 2039791 - Filter orphaned and non-folder-parent structure rows from fetch_remote_tree (#7375)

Comment 2

[BMO Github Automation]

Authored by https://github.com/mhammond
https://github.com/mozilla/application-services/commit/0c92caefaa1ef2b95cff60a61be3fd6a95661ce6
[main] Add tests reproducing MissingParentForUnknownChild and InvalidParent from orphaned synced structure rows (#7372)

Comment 3

[Mark Hammond [:markh] [:mhammond]]

Only tests landed, actual fixes yet to come.

Comment 4

[BMO Github Automation]

Authored by https://github.com/mhammond
https://github.com/mozilla/application-services/commit/cffe8121fb67b9b4917a5509ced3a82850f3debf
[main] Bug 2039791 - Filter orphaned and non-folder-parent structure rows from fetch_remote_tree (#7375)

Comment 5

[BMO Github Automation]

Attachment: [mozilla/application-services] Bug 2039791 - Filter orphaned and non-folder-parent structure rows from fetch_remote_tree (backport #7375) (#7384)

Comment 6

[BMO Github Automation]

Attachment: [mozilla/application-services] Bug 2039791 - Filter orphaned and non-folder-parent structure rows from fetch_remote_tree (backport #7375) (#7389)

Comment 7

[Ryan VanderMeulen (PTO, back July 27)]

Included in the A-S 152.0.1 bump landed for 152.0b8.

Comment 8

[Pulsebot]

https://github.com/mozilla-firefox/firefox/commit/333328328b17
https://hg.mozilla.org/releases/mozilla-beta/rev/bd268cfea958