Clients engine doesn't handle duplicate commands correctly, causing multiple events to be "sent" but only one "received"

RESOLVED FIXED in Firefox 56

Status

()

P1
normal
RESOLVED FIXED
a year ago
a year ago

People

(Reporter: markh, Assigned: tcsc)

Tracking

Trunk
Firefox 56
Points:
---

Firefox Tracking Flags

(firefox56 fixed)

Details

MozReview Requests

()

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

Attachments

(1 attachment)

(Reporter)

Description

a year ago
STR:
* On one device, send the same tab to the same device multiple times. Restart, check about:telemetry, notice there are multiple events, each with different flow IDs written for these sends.

* Start the target device, sync. Notice the incoming tab is only opened once. Restart, check about:telemetry, notice that only 1 event for the receive is recorded.

This will cause our telemetry to report a poor success rate - we have multiple sends but only 1 receive.

What's happening is:

* We expect http://searchfox.org/mozilla-central/rev/66d9eb3103e429127c85a7921e16c5a02458a127/services/sync/modules/engines/clients.js#576 to notice the duplicate - however, that fails to work because the commands have already been synced - so we go ahead and write a new event.

* However, the duplicates *are* discarded at http://searchfox.org/mozilla-central/rev/66d9eb3103e429127c85a7921e16c5a02458a127/services/sync/modules/engines/clients.js#873 - but at this time we have already written the "event" to the sync ping.

This will be true for all commands (ie, including repair commands).
Priority: -- → P1
Assignee: nobody → tchiovoloni
Comment hidden (mozreview-request)
(Reporter)

Comment 2

a year ago
Comment on attachment 8876187 [details]
Bug 1368393 - Handle already synced commands in clients collection duplicate checking

Sorry for taking so long with this, but I want to give it a good look. Ed, can you please review it too, given I think it's your code originally?
Attachment #8876187 - Flags: review?(eoger)

Comment 3

a year ago
mozreview-review
Comment on attachment 8876187 [details]
Bug 1368393 - Handle already synced commands in clients collection duplicate checking

https://reviewboard.mozilla.org/r/147618/#review154196

Woops, I thought I commented on that this morning. Looks great!
Attachment #8876187 - Flags: review?(eoger) → review+
(Reporter)

Comment 4

a year ago
mozreview-review
Comment on attachment 8876187 [details]
Bug 1368393 - Handle already synced commands in clients collection duplicate checking

https://reviewboard.mozilla.org/r/147618/#review154924

thanks!
Attachment #8876187 - Flags: review?(markh) → review+

Comment 5

a year ago
Pushed by tchiovoloni@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/e87b8b55577a
Handle already synced commands in clients collection duplicate checking r=eoger,markh

Comment 6

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/e87b8b55577a
Status: NEW → RESOLVED
Last Resolved: a year ago
status-firefox56: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 56
You need to log in before you can comment on or make changes to this bug.