TPS failure caused by resyncs: "Automatic sync got triggered, which is not allowed"

RESOLVED FIXED in Firefox 55

Status

()

Firefox
Sync
P1
normal
RESOLVED FIXED
11 months ago
11 months ago

People

(Reporter: tcsc, Assigned: tcsc)

Tracking

unspecified
Firefox 55
Points:
---

Firefox Tracking Flags

(firefox55 fixed)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment)

(Assignee)

Description

11 months ago
This looks like it's caused by the resyncs from bug 1335891, and appears to break all TPS tests every time (on all platforms), since it triggers a sync TPS doesn't expect.

It's unclear to me why the followup sync is being triggered so consistently, and seems like a bug...

But even if it is, TPS needs to be able to handle these, which seems straightforward enough (patch incoming).

Another option would be to set a services.sync.maxResyncs to 0, but it seems worth keeping TPS's runtime environment as close as possible to the real world.
Comment hidden (mozreview-request)

Comment 2

11 months ago
mozreview-review
Comment on attachment 8870989 [details]
Bug 1367543 - Make sync's TPS tests handle cases where resyncs are triggered

https://reviewboard.mozilla.org/r/142560/#review146170
Attachment #8870989 - Flags: review?(kit) → review+
I would like to figure out why we're triggering two syncs after logging in, though.
Capturing IRC discussion for posterity:

> tcsc> no, tps will 100% have to have this either way
> tcsc> in general tps relies on syncs never happening in the background (since it asserts that the state is what it expects, which may be an unsynced state)
> tcsc> so it needs to wait for all the syncs to finish
> tcsc> so, while it may be a bug that we sync too frequently, it just means that bug made us fix tps sooner than we might have otherwise
> kitcambridge> there are other cases where we trigger an immediate sync http://searchfox.org/mozilla-central/source/services/sync/modules/policies.js#309,414,416,967,981 but i'm guessing TPS doesn't need to worry about those, otherwise we'd have seen them long before this
> tcsc> so the way tps prevents those is by scheduling a sync for the distant future in the sync finished handler
> tcsc> i think i like this approach better than making that work for the resync case too though, since it's closer to what happens in the real world though

Comment 5

11 months ago
Pushed by tchiovoloni@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/f49518e8597c
Make sync's TPS tests handle cases where resyncs are triggered r=kitcambridge

Comment 6

11 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/f49518e8597c
Status: ASSIGNED → RESOLVED
Last Resolved: 11 months ago
status-firefox55: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 55
You need to log in before you can comment on or make changes to this bug.