1434302 - Filter blocklist for app version server-side

Status: RESOLVED FIXED

(Toolkit :: Blocklist Implementation, enhancement)

Description

[:Gijs (he/him)]

Individual blocklist items can currently have OS, OS version, and toolkit version annotations, which get processed client-side. But the file we serve for Windows and OS X requests for the blocklist is, as far as I can tell, identical.

This is despite the fact that the request URI includes:

- app id (guid)
- app version
- OS name and version (e.g. "Darwin 17.4.0")

so the server could be more discriminating in what it sends back to clients.

While long-term we want to start serving binary diffs and so on through kinto anyway, short-term we can shorten the blocklist by an order of magnitude by filtering the list server-side. This will improve bytes-over-the-wire throughput on the server, and IO as well as processing times on the clients. Given the current impact of the blocklist, and the fact that a server-side fix like this could be rolled out independent of the release train (and so deliver measurable speed-ups to users on release 57/58/59 builds while we try to make structural improvements for 60) means I think it may be worth investing some time here assuming this is a change we can make relatively easily.

For clients with versions >= 57, we can omit all non-flash plugin blocks. Add-on blocks could be limited to blocked signed add-ons only. Graphics and plugin restrictions could also be further constrained based on applicable OSes. 

My understanding is the relevant code lives at https://github.com/mozilla-services/amo2kinto/blob/master/amo2kinto/exporter.py and https://github.com/mozilla-services/kinto-amo/blob/master/kinto_amo/views/services.py .

Mathieu, can you confirm if this view of the world is essentially correct? If so, I will try to look at submitting some PRs for the server code. I don't know how often it is deployed - how quickly could we get something out, from the point where a patch is ready?

Orthogonally, from a quick look at the code it seems like there was originally version filtering like this for plugins, but it's disabled in API version 3. Am I reading that right, and do you have more context for why that is the case?


(To be clear, the Firefox team is still committed to switching to a different solution than the giant XML blob, but more client-side work needs to be done to get there, hence filing something for an interim remediation/improvement like this. I will be filing more bugs about this soon, once I have a better handle on what exactly needs to happen.)

Comment 1

[Mathieu Leplatre [:leplatrem]]

> But the file we serve for Windows and OS X requests for the blocklist is, as far as I can tell, identical.

Yes, indeed.

> Mathieu, can you confirm if this view of the world is essentially correct?

Yes, it looks sound. You could filter out records based on your specific assertions by version or platform here:
https://github.com/mozilla-services/kinto-amo/blob/a8e006650ed1f105cd4f5afd1f85a53cafe739db/kinto_amo/views/services.py#L31-L49

I'm not sure that we have enough metadata on each blocklist entry (to determine whether it targets a signed addons or not for example). But it's not very complicated to add new fields (the CRUD form in the UI is automatically generated from a JSON schema).

A little detail to keep in mind: since the last-modified response header value should not go backwards, you should take the highest timestamp of the unfiltered list of records — and not the highest of the subset of records.

>  I don't know how often it is deployed - how quickly could we get something out, from the point where a patch is ready?

Deploying is fully automated, and it's not a burden to deploy a new version. It's more about packaging and QA acceptance. We basically do it like this:
- work the kinto-amo repo
- release a new version
- upgrade it in our meta-package kinto-dist [0]
- request a new deploy on stage [1]
- try sync using aboutremotesettings [2]
- wait for QA approval
- green light for prod :)

> from a quick look at the code it seems like there was originally version filtering like this for plugins, but it's disabled in API version 3. Am I reading that right, and do you have more context for why that is the case?

Before switching from the addons-server to Kinto [3], we made sure the new XML was perfectly identical to the former one. To be honest, we replicated the version filtering naively, without thinking too much why it was only in plugins or why it was disabled for version 3. Maybe :natim remembers more details that me.

I agree with you, that the cost of optimization on the server side is relatively low compared to the potential impact! Thanks for bringing energy to this!

[0] https://github.com/Kinto/kinto-dist/
[1] https://bugzilla.mozilla.org/show_bug.cgi?id=1425513
[2] https://github.com/leplatrem/aboutremotesettings
[3] https://github.com/mozilla/addons-server/pull/4682

Comment 2

[Rémy Hubscher (:natim)]

I am all in for this, we should probably use a new /4/ prefix for these changes and add tests to kinto-amo.

Comment 3

[:Gijs (he/him)]

(In reply to Rémy Hubscher (:natim) from comment #2)
> I am all in for this, we should probably use a new /4/ prefix for these changes

This would require updating the clients, though. We're not changing the format of what we're serving, and all the client request should be compatible, so can you elaborate on why we would need to update the prefix?

This is effectively the same as "unblocklisting" a subset of the items for a subset of clients, which would be possible by just removing them from the DB. The only difference is we want to keep blocklisting those items for 52esr (for now).

Comment 4

[Rémy Hubscher (:natim)]

Ok then, makes sense :)

Comment 5

[:Gijs (he/him)]

I have a PR for filtering out add-ons and plugins based on the app ID + version requested up at https://github.com/mozilla-services/amo2kinto/pull/74 . I expect we would still also need to annotate existing blocklist items to make it clear they don't apply to Fx >= 58.

Comment 6

[:Gijs (he/him)]

This has all landed, and is going to production in bug 1437025.