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

Status: RESOLVED FIXED

(Firefox :: Sync, enhancement, P1)

Description

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

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).

Comment 1

[Thom Chiovoloni [:tcsc] (ex-moco)]

Attachment: Bug 1368393 - Handle already synced commands in clients collection duplicate checking

Review commit: https://reviewboard.mozilla.org/r/147618/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/147618/

Comment 2

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

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?

Comment 3

[Edouard Oger [:eoger]]

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!

Comment 4

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

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

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

thanks!

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e87b8b55577a
Handle already synced commands in clients collection duplicate checking r=eoger,markh

Comment 6

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/e87b8b55577a