Closed Bug 1376618 Opened 7 years ago Closed 7 years ago

Conflicts in diff-based JSON sync for plugins blocklist

Categories

(Toolkit :: Blocklist Implementation, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla56
Tracking Status
firefox56 --- fixed

People

(Reporter: leplatrem, Assigned: leplatrem)

Details

Attachments

(2 files)

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)
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 on attachment 8882400 [details] Bug 1376618 - Load initial blocklist data as synced https://reviewboard.mozilla.org/r/153514/#review158782 Looks good to me!
Attachment #8882400 - Flags: review?(mgoodwin) → review+
Keywords: checkin-needed
Pushed by ryanvm@gmail.com: https://hg.mozilla.org/integration/autoland/rev/5e61007eb1ae Load initial blocklist data as synced r=mgoodwin
Keywords: checkin-needed
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
Component: Blocklist Policy Requests → Blocklist Implementation
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: