1376618 - Conflicts in diff-based JSON sync for plugins blocklist

Status: RESOLVED FIXED

(Toolkit :: Blocklist Implementation, defect)

Description

[Mathieu Leplatre [:leplatrem]]

Foreword: this does not affect plugin blocklisting (because it still relies on XML, see Bug 1257565)
 
---

According to the new Telemetry probes (introduced in Bug 1254099), around 30% of plugins blocklist sync fail because of conflicts.

It looks like those 8 records are causing the failure. They were updated since the initial dump, and sync ends up in conflicts:

    32d5edbf-0547-fa2d-8cc2-138676a17bc0 1486576001966 != 1487179268707
    24ac1e2b-8e0a-a084-5123-cb87d4ceb898 1486575979474 != 1487179342786
    6ef87143-6593-e234-a77c-6af69ad1567b 1486575908463 != 1487179201987
    a61c906e-fe1f-b569-1807-96ffa59b2b6c 1486575890885 != 1487179606877
    ee3019e9-2572-ecb2-3de3-abc4842d7e0a 1486575869178 != 1487179448668
    c867bfd8-ec29-0800-8689-4b90b89fa870 1486575857101 != 1487265492524
    9234afc8-8f7f-602a-9c84-7d7f2685802a 1486575835268 != 1487179504891
    22b0fa08-8712-69cd-4ba4-9fb5ab9398d1 1486575789909 != 1487178814618

The remote timestamp is superior to the local timestamp, so there is no reason it should not apply the update to the local data (even if the sync strategy is not SERVER_WINS).

Steps to reproduce:

- install about:remotesettings addon
- clear local data for plugins
- hit force sync (initial dump will be loaded, then a sync will be issued, which will eventually fail)

Comment 1

[Mathieu Leplatre [:leplatrem]]

Attachment: Telemetry Screenshot 2017-06-27 16-14-45.png

Comment 2

[Mathieu Leplatre [:leplatrem]]

Attachment: Bug 1376618 - Load initial blocklist data as synced

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

Comment 3

[Mathieu Leplatre [:leplatrem]]

The initial JSON dumps were loaded via the storage adapter directly (instead of via the collection method), which caused the records to be saved locally without sync status. 

The patch solves the problem for future new profiles, as well as existing profiles by switching the sync strategy to SERVER_WINS (clients are not supposed to change the blocklist data locally).

Comment 4

[Mark Goodwin [:mgoodwin]]

Comment on attachment 8882400 [details]
Bug 1376618 - Load initial blocklist data as synced

https://reviewboard.mozilla.org/r/153514/#review158782

Looks good to me!

Comment 5

[Mathieu Leplatre [:leplatrem]]

Comment on attachment 8882400 [details]
Bug 1376618 - Load initial blocklist data as synced

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/153514/diff/1-2/

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5e61007eb1ae
Load initial blocklist data as synced r=mgoodwin

Comment 7

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/5e61007eb1ae