Closed
Bug 1189267
Opened 9 years ago
Closed 9 years ago
Funsize should be able to generate multiple partials in a single run
Categories
(Release Engineering :: Release Automation: Other, defect)
Release Engineering
Release Automation: Other
Tracking
(Not tracked)
VERIFIED
FIXED
People
(Reporter: rail, Assigned: rail)
References
Details
This way we can support chunking and local cache!
Assignee | ||
Comment 1•9 years ago
|
||
CC'ing whimboo, he may be interested too.
Assignee | ||
Comment 2•9 years ago
|
||
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.
Comment 3•9 years ago
|
||
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.
Assignee | ||
Comment 4•9 years ago
|
||
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
Assignee | ||
Comment 5•9 years ago
|
||
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 :/
Comment 6•9 years ago
|
||
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)
Assignee | ||
Comment 7•9 years ago
|
||
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.
Comment 8•9 years ago
|
||
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.
Assignee | ||
Comment 9•9 years ago
|
||
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
Assignee | ||
Comment 10•9 years ago
|
||
Comment 11•9 years ago
|
||
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?
Comment 12•9 years ago
|
||
FYI the suffix '.updates' will break backward compatibility compared to '.balrog' before. But that's not a problem for us.
Assignee | ||
Comment 13•9 years ago
|
||
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).
Comment 14•9 years ago
|
||
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.
Assignee | ||
Comment 15•9 years ago
|
||
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.
Comment 16•9 years ago
|
||
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)
Comment 17•9 years ago
|
||
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
Assignee | ||
Comment 18•9 years ago
|
||
@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)
Comment 19•9 years ago
|
||
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)
Comment 20•9 years ago
|
||
> 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)
Assignee | ||
Comment 21•9 years ago
|
||
This will be live tomorrow.
Comment 22•9 years ago
|
||
(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...
Assignee | ||
Comment 23•9 years ago
|
||
Done
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Comment 24•9 years ago
|
||
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.
Description
•