1636158 - Periodic file update job only partially updates new cascade bloom filter add-on blocklist data

Status: VERIFIED FIXED

(Release Engineering :: Release Automation, defect)

Description

[Ryan VanderMeulen [:RyanVM]]

Today's scheduled periodic file update (https://hg.mozilla.org/integration/autoland/rev/cdcefab9cfc3eaee323cf072189be81ad4e91767) caused xpcshell test failures on autoland due to the changes in addons-bloomfilters.json.
https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=301220692&repo=autoland&lineNumber=9599

I'm guessing it's because we updated the expected metadata in the json file but didn't update the actual binary data in services/settings/dumps/blocklists/addons-bloomfilters?

I can manually revert the changes to addons-bloomfilters.json to avoid future bustage for now, but we'll need a longer-term solution here.

Comment 1

[Rob Wu [:robwu]]

I'm glad that I made that test :)

Please revert the changes to that file for now. The long-term fix to fix this specific bug (about importing) is to update the files in services/settings/dumps/blocklists/addons-bloomfilters/ as well. Where is the source code for the update job?

The updated blocklist has much more entries due to the import from bug 1635739. The size change was deemed unacceptable for mobile, so I'm currently working on a way to reduce the size to an acceptable level again ( https://docs.google.com/document/d/1RIOs-7Ud-QlbTOggBydpm47zngT-kWwNzNuFLZTM3iU/edit ).

Comment 2

[Rob Wu [:robwu]]

I intend to resolve this bug before the 78 -> 79 version bump.

Comment 3

[Ryan VanderMeulen [:RyanVM]]

Here's the source for the script that does the update:
https://searchfox.org/mozilla-central/source/taskcluster/docker/periodic-updates/scripts/periodic_file_updates.sh

This also affects Beta77, FWIW.

Comment 4

[Rob Wu [:robwu]]

Is the script on m-c used to update all branches (m-c, beta, release, esr68, (future) esr78), or does each branch use their own script?

I see a disconnect between the number of updates on m-c vs beta:
https://hg.mozilla.org/mozilla-central/log/tip/services/settings/dumps/blocklists/addons-bloomfilters.json
https://hg.mozilla.org/releases/mozilla-beta/log/tip/services/settings/dumps/blocklists/addons-bloomfilters.json

All things considered, the space increase is acceptable for desktop but not for mobile. Due to the controlled roll-out of add-on support on Fenix, I can focus on fixing the update script for desktop first, and do a follow-up later to address the mobile case.

Comment 5

[Ryan VanderMeulen [:RyanVM]]

Each branch runs its own local version of the script. Regarding the difference on Beta, I manually dropped the change before pushing there.

Comment 6

[Ryan VanderMeulen [:RyanVM]]

(In reply to Rob Wu [:robwu] from comment #4)

All things considered, the space increase is acceptable for desktop but not for mobile. Due to the controlled roll-out of add-on support on Fenix, I can focus on fixing the update script for desktop first, and do a follow-up later to address the mobile case.

I don't fully understand this comment. The periodic file update script is just trying to update what's currently checked into source control, not making any decisions about what products those checked-in files are ultimately shipped with.

Do we have an ETA on when a patch might be ready? I'm having to manually edit and land 4 patches a week until this is resolved.

Comment 7

[Rob Wu [:robwu]]

(In reply to Ryan VanderMeulen [:RyanVM] from comment #6)

(In reply to Rob Wu [:robwu] from comment #4)

All things considered, the space increase is acceptable for desktop but not for mobile. Due to the controlled roll-out of add-on support on Fenix, I can focus on fixing the update script for desktop first, and do a follow-up later to address the mobile case.

I don't fully understand this comment. The periodic file update script is just trying to update what's currently checked into source control, not making any decisions about what products those checked-in files are ultimately shipped with.

The current blocklist file in the tree is 64KB, but the proposed update (via the update script) will increase this to 700KB. That size increase is not acceptable for mobile.
To resolve that issue the new blocklist implementation will be disabled on mobile, and the in-tree blocklist dump won't be bundled with the mobile binary any more. The functionality and packaging will be added after implementing an optimization that reduces the file size by over 80%.

Do we have an ETA on when a patch might be ready?

I will to attach a patch early next week. Or maybe earlier, but at least land it after tomorrow.

I'm intentionally keeping the packaged v3 blocklist out of date, to enable QA to test real blocks and updates with the actual blocklist backend. I expect QA to finish this part of testing tomorrow, hence my plan to resolve this bug by early next week.

I'm having to manually edit and land 4 patches a week until this is resolved.

Sorry about that! Thanks for doing this.

Comment 8

[Rob Wu [:robwu]]

Attachment: Bug 1636158 - Remove addons-bloomfilters from Android package

Those will be restored in bug 1639050

Comment 9

[Rob Wu [:robwu]]

Attachment: Bug 1636158 - Update the addons-mlbf.bin attachment via periodic_file_updates.sh

Tested by running the following code and verifying that the script runs
successfully and generates a reasonable diff with the latest metadata
and matching attachment content:

$ cd taskcluster/docker/periodic-updates/
$ tar -czf- -s "#$(git rev-parse --show-cdup)#topsrcdir/#" . $(awk -v d="$(git rev-parse --show-cdup)" '/^# %include/{print d$3}' Dockerfile) | docker build - -t hsts-local
$ docker run -e DO_REMOTE_SETTINGS=1 -e PRODUCT="firefox" -e BRANCH="mozilla-central" -e USE_MOZILLA_CENTRAL=1 hsts-local

After that, I opened a shell in the container, built Firefox and ran the
following test, to confirm that the updated records are used correctly:

mach test toolkit/mozapps/extensions/test/xpcshell/rs-blocklist/test_blocklist_mlbf_dump.js

Comment 10

[Rob Wu [:robwu]]

Attachment: Bug 1636158 - Document how to use attachment dumps in RemoteSettings

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d07f8d9d1a01
Remove addons-bloomfilters from Android package r=agi,geckoview-reviewers,snorp
https://hg.mozilla.org/integration/autoland/rev/d3770d94e7c0
Update the addons-mlbf.bin attachment via periodic_file_updates.sh r=sfraser
https://hg.mozilla.org/integration/autoland/rev/049bdaa3e410
Document how to use attachment dumps in RemoteSettings r=leplatrem

Comment 12

[Rob Wu [:robwu]]

Attachment: Bug 1636158 - Record usage of blocklist v3 in telemetry

Although the data review request granted permission to keep the probes
for 6 months, I'm using a shorter expiration (81) so that all
blocklist-related telemetry can be coverd by one update request, in the
future.

"geckoview" is not listed yet, because blocklist v3 is not yet enabled
on mobile (bug 1639050). Once enabled, "geckoview" should be added to
every scalar from this commit.

Comment 13

[Phabricator Automation]

Comment on attachment 9151151 [details]
Bug 1636158 - Record usage of blocklist v3 in telemetry

Revision D76539 was moved to bug 1633466. Setting attachment 9151151 [details] to obsolete.

Comment 14

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/d07f8d9d1a01
https://hg.mozilla.org/mozilla-central/rev/d3770d94e7c0
https://hg.mozilla.org/mozilla-central/rev/049bdaa3e410

Comment 15

[Rob Wu [:robwu]]

Ryan - we aren't going to use the v3 blocklist on 77, so instead of uplifting the above, you can do the following when you run the update script on the beta channel at the next time:

• replace "dump_match" with "dump_fallback" at https://searchfox.org/mozilla-central/rev/fc91a093e40dde71d10ad219946b8ae775aca9eb/toolkit/mozapps/extensions/test/xpcshell/rs-blocklist/test_blocklist_mlbf_dump.js#85
• replace inputRecord.attachment.hash, with downloadResult.record.attachment.hash, at https://searchfox.org/mozilla-central/rev/fc91a093e40dde71d10ad219946b8ae775aca9eb/toolkit/mozapps/extensions/test/xpcshell/rs-blocklist/test_blocklist_mlbf_dump.js#91

The out-of-sync attachment dump is still kept in the tree, to allow users who manually flip the preference to still have a reasonable baseline of protection by the blocklist (out of date by a few weeks is better than nothing at all).

Comment 16

[Ryan VanderMeulen [:RyanVM]]

We had a successful update on m-c today with addons-bloomfilters changes included (though there was a test update needed as covered in bug 1640953). NI myself to take comment 15 account with the next pfu run against 77. I'll verify it on Try and get Rob to review the changes before pushing as well.

Comment 17

[Ryan VanderMeulen [:RyanVM]]

This was done.
https://hg.mozilla.org/releases/mozilla-release/rev/e167ef80f941b59a715c2aa24dec51b4fc05ece0