Closed Bug 1189267 Opened 4 years ago Closed 4 years ago

Funsize should be able to generate multiple partials in a single run

Categories

(Release Engineering :: Release Automation: Other, defect)

defect
Not set

Tracking

(Not tracked)

VERIFIED FIXED

People

(Reporter: rail, Assigned: rail)

References

(Blocks 1 open bug)

Details

This way we can support chunking and local cache!
CC'ing whimboo, he may be interested too.
FTR, this won't change the manifest format, but will change the routes. Locales will be dropped from the route names because a chunk will handle multiple locales.
Rail, do you have any ETA on that? I assume it's something you don't touch the next couple of weeks? Just to know so I don't have to worry about it for now. I can change that later very easily.
This is a WIP feature and I want to implement it ASAP. It's blocking some release promotion work. It shouldn't take more than 2 weeks, unless I hit some issues.

FTR, I'll need to add multiple routes (per locale):

<whimboo> rail: i assume the removal of the locale from the routing key will be permanent for all notifications?
<whimboo> even with only a single one getting built?
<rail> whimboo: I can add multiple routes per task
<rail> for every locale
<rail> in this case you'll need to watch for the CCed routed
<rail> would that work?
<whimboo> rail: yeah. adds complexity but would be backward compatible
<whimboo> rail: i assume only the locale would vary and the platform is always the same
<rail> yup
<whimboo> great
<whimboo> good solution then
Some good and bad news.

I have a green run of this at https://tools.taskcluster.net/task-graph-inspector/#ynMnwMyyRCGjzjjuOovWfw/22Ickf5xSVCRK-oPx9_otg/

Still need to address the local cache feature.

Bad news that we can't add more than 10 routes per task. This means that we should use some other mechanism to expose the locales. Maybe just in the manifest :/
Rail, something I wonder is what is the optimal number of locales to run in a single task. Means would it make sense to have at maximum 10 locales in the queue, and spread the creation across multiple tasks so it can be run more in parallel.

Without a possibility to announce the contained locales in the pulse message, we would always have to download the manifest. :(

Peete, maybe you have an idea?
Flags: needinfo?(pmoore)
BTW, max 10 routes doesn't mean we can up to 10 locale routes. I expect 2 routes for treeherder, 2 "chunked" artifacts routes (revision + latest). This leaves only 6 routes to use. Ideally I'd like to use 2 routes per locale (revision and latest). That comes to 3 locales only. This is definitely not ideal.
I wonder why we have this limitation of 10 routes only. Tags you can set up to 4096. Maybe Peete can really give some answers here.
As a workaround I can put all locales in a chunk into a single route and separate them by some symbol, probably underscore. Something like:

- index.funsize.v1.{{ branch }}.revision.{{ platform }}.{{ revision }}.en-US_en-GB_de_pt-BR.{{ update_number }}.balrog
- index.funsize.v1.{{ branch }}.latest.{{ platform }}.en-US_en-GB_de_pt-BR.{{ update_number }}.balrog
I think that should be another alternative for sure. The routing keys might be getting long but at least we most likely won't hit the string limit?
FYI the suffix '.updates' will break backward compatibility compared to '.balrog' before. But that's not a problem for us.
It's still ".balrog", see https://tools.taskcluster.net/task-graph-inspector/#3GQZ5NdATqqLzfw0EBRrnA/bC8KqiJTSE-MWIMlgYUMUg/ (the link I gave before was the first task in the same graph).
Ah, I see. I didn't notice that. Thanks Rail. So as Peete told me we also have a 255 char limit for routing keys. Seeing 63 chars being used for the default content, we have 192 chars left for locale content. Taking 'en-GB_' with 6 chars we will be able to cover ~32 locales.
Something tells me that we shouldn't abuse this feature... With chunking enabled, we'll have 6-10 chunks per branch. With 4 update paths (4 days) this will give us 24-40 tasks to handle instead of ~500 now.
I'm not sure what imposes the 10 cc routes limit. I haven't found a rabbitmq/pulse/taskcluster doc that specifies the limit, so I'm not sure at which level it is being imposed. Agree the AMQP message headers can support more.

Most notably I don't see a restriction here:
http://www.rabbitmq.com/sender-selected.html

I also checked the rabbitmq-server codebase, and couldn't see anything obvious there (although I can't read erlang!).

But ..... jonas will know! ;)
Flags: needinfo?(pmoore) → needinfo?(jopsen)
From link above:

The values associated with the "CC" and "BCC" header keys will be added to the routing key if they are present. The message will be routed to all destinations matching the routing key supplied as a parameter to the basic.publish method, as well as the routes supplied in the "CC" and "BCC" headers. The type of "CC" and "BCC" values must be an array of ****longstr***** and these keys are case-sensitive. If the header does not contain "CC" or "BCC" keys then this extension has no effect. 

And from https://www.rabbitmq.com/releases/rabbitmq-java-client/v2.5.1/rabbitmq-java-client-javadoc-2.5.1/com/rabbitmq/client/impl/LongString.html it looks like "longstr" is 4GB !!

4GB should be big enough to hold more than 10 routing keys. :p
@whimboo, any objections for the following route format:

index.funsize.v1.{{ branch }}.latest.{{ platform }}.{{ update_number }}.{{ chunk_name }}.balrog

where
 udpate_number: number representing TODAY-udpate_number
 chunk_name: chunk number (for l10n) or en-US
Flags: needinfo?(hskupin)
Given that we wont extract the locales from the routing key anymore but having to download the manifest file, I don't really care about the CC'ed routing keys anymore. When I get a message I will directly download the the manifest for processing. So feel free to name it what you want. My proposal is at least to stay close with the build notifications if possible:

index.funsize.v1.{{ branch }}.{{ platform }}.latest.{{ update_number }}.{{ chunk_name }}.balrog

When we then get the update_number in the manifest (see bug 1198011) I'm happy.
Flags: needinfo?(hskupin)
> I'm not sure what imposes the 10 cc routes limit.
It's in TC docs. Enforced in JSON schema. 10 is an arbitrary limit that was added to prevent abuse.

Anyways, I think we resolved this on IRC.

Summary: Routing keys can be too specific, and sometimes it's sufficient to reduce candidate tasks from
         10k to a few hundred and then lookup artifacts on those tasks.
Flags: needinfo?(jopsen)
This will be live tomorrow.
(In reply to Jonas Finnemann Jensen (:jonasfj) from comment #20)
> > I'm not sure what imposes the 10 cc routes limit.
> It's in TC docs. Enforced in JSON schema. 10 is an arbitrary limit that was
> added to prevent abuse.

I should have spotted that! Sorry! Indeed it is on the docs page:

http://docs.taskcluster.net/queue/api-docs/#createTask

"Create New Task" -> "Request Payload": `routes` is defined as `array[0..10]`. This in turn is generated from:

https://github.com/taskcluster/taskcluster-queue/blob/master/schemas/create-task-request.yml#L63

I'm not sure how I missed this before, it should have been the first place to check...
Done
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
I can verify that works as expected. I was able to retrieve those messages today and handle them appropriately. Thanks Rail!
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.