1319824 - Enable funsize for date

Status: RESOLVED FIXED

(Release Engineering :: General, defect)

Description

[Rail Aliiev [:rail]]

Builds on date are TC based, so we'll need to modify funsize to treat them differently. We should track the signed binaries only.

Comment 1

[Simon Fraser [:sfraser] ⌚️GMT]

How do we track which jobs (or pulse messages) are related to the production of a signed binary?

Comment 2

[Simon Fraser [:sfraser] ⌚️GMT]

In order to easily track the signing jobs, adding a custom pulse message route to the task graph seems appropriate. Funsize doesn't have to have any special logic to work out if it it's a task that needs listening to - the signing tasks will be the only ones publishing on this route.

Based on https://docs.taskcluster.net/manual/devel/namespaces it seems like a route name of `project.releng.funsize.level-3` is appropriate. Would that be an acceptable route name for these pulse messages?

Comment 3

[Rail Aliiev [:rail]]

project.releng.funsize.level-3 sounds good to me. I'm not sure what our naming practise is, but maybe you need to add some other info into the route, like the branch?

Comment 4

[Simon Fraser [:sfraser] ⌚️GMT]

https://docs.taskcluster.net/manual/devel/namespaces has the naming guidelines. Adding in the branch sounds like a good idea. I'll put in a review request for 'project.releng.funsize.date.level-3'

Comment 5

[Simon Fraser [:sfraser] ⌚️GMT]

Attachment: bug 1319824 r? Enable funsize-specific pulse route for date's nightly

signing jobs.

Review commit: https://reviewboard.mozilla.org/r/100152/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/100152/

Comment 6

[Justin Wood (:Callek)]

(In reply to Simon Fraser [:sfraser] ⌚️GMT from comment #2)
> In order to easily track the signing jobs, adding a custom pulse message
> route to the task graph seems appropriate. Funsize doesn't have to have any
> special logic to work out if it it's a task that needs listening to - the
> signing tasks will be the only ones publishing on this route.
> 
> Based on https://docs.taskcluster.net/manual/devel/namespaces it seems like
> a route name of `project.releng.funsize.level-3` is appropriate. Would that
> be an acceptable route name for these pulse messages?

I think we should probably meet about this, I'm not sure why we need a route/namespace, or why it makes sense for this case.

That said, I am planning on coercing the index routes to be on the signing task and not the unsigned task at some point in the (near) future though.

Comment 7

[Simon Fraser [:sfraser] ⌚️GMT]

If there's a different, reliable way of identifying just the signing tasks from the pulse messages, we could use that. I'm trying to avoid having the funsize scheduler perform lots of API calls in order to identify which messages it needs to act on.

Comment 8

[Simon Fraser [:sfraser] ⌚️GMT]

Attachment: bug 1319824 r? Enable funsize-specific pulse route for date's en-US nightly

Review commit: https://reviewboard.mozilla.org/r/100192/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/100192/

Comment 9

[Justin Wood (:Callek)]

(In reply to Justin Wood (:Callek) from comment #6)
> (In reply to Simon Fraser [:sfraser] ⌚️GMT from comment #2)
> > In order to easily track the signing jobs, adding a custom pulse message
> > route to the task graph seems appropriate. Funsize doesn't have to have any
> > special logic to work out if it it's a task that needs listening to - the
> > signing tasks will be the only ones publishing on this route.
> > 
> > Based on https://docs.taskcluster.net/manual/devel/namespaces it seems like
> > a route name of `project.releng.funsize.level-3` is appropriate. Would that
> > be an acceptable route name for these pulse messages?
> 
> I think we should probably meet about this, I'm not sure why we need a
> route/namespace, or why it makes sense for this case.

We chatted in IRC about this today, takeaways:

* partials are tier1 blocking
* this is one of the easier ways we envision being able to identify which tasks funsize needs to act on.

Because of that, I'm not going to block on a thought-project on if there is a better way to go about this, a route isn't that bad of a thing.

Comment 10

[Simon Fraser [:sfraser] ⌚️GMT]

Attachment: bug 1319824 Announce signing jobs to funsize with

a new pulse route.

Review commit: https://reviewboard.mozilla.org/r/100232/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/100232/

Comment 11

[Justin Wood (:Callek)]

Comment on attachment 8820793 [details]
bug 1319824  Announce signing jobs to funsize with

https://reviewboard.mozilla.org/r/100232/#review100722

Also, please put the commit message on one line, otherwise `hg log` and summary of such won't show the whole thing, leaving a shortened actual message in consoles.

::: taskcluster/taskgraph/transforms/signing.py:67
(Diff revision 1)
>      # taskcluster/taskgraph/transforms/task.py for the schema details, and the
>      # below transforms for defaults of various values.
>      Optional('treeherder'): task_description_schema['treeherder'],
> +
> +    # If True, announce to a specific route when done, so that funsize
> +    # can start its partial mar generation. For nightly builds only.

very nitpicky:

"If True, adds a route which funsize uses to schedule generation of partial mar files for updates. Expected to be added on nightly builds only"

::: taskcluster/taskgraph/transforms/signing.py:124
(Diff revision 1)
>              'attributes': attributes,
>              'run-on-projects': dep_job.attributes.get('run_on_projects'),
>              'treeherder': treeherder,
>          }
>  
> +        if job.get('use_funsize_route', False):

can omit the ", False" but doesn't hurt to have it.

::: taskcluster/taskgraph/transforms/signing.py:125
(Diff revision 1)
>              'run-on-projects': dep_job.attributes.get('run_on_projects'),
>              'treeherder': treeherder,
>          }
>  
> +        if job.get('use_funsize_route', False):
> +            task['routes'] = ["project.releng.funsize.{project}.level-3".format(project=config.params['project'])]

nit: also format "level" so:

"project.releng.funsize.{project}.level-{level}".format(... level=config.params['level'])

Comment 12

[Justin Wood (:Callek)]

Comment on attachment 8820793 [details]
bug 1319824  Announce signing jobs to funsize with

https://reviewboard.mozilla.org/r/100232/#review100724

Comment 13

[Simon Fraser [:sfraser] ⌚️GMT]

Attachment: based on review, change comment and add config-based level in the route

Review commit: https://reviewboard.mozilla.org/r/100234/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/100234/

Comment 14

[Justin Wood (:Callek)]

sfraser:

in the future you can do `hg commit --amend` to modify an existing commit with new content and/or change the commit message.

You can also use `hg histedit` to do things like fold two commits together and select certain commits to modify a commit message for as well, incase that workflow works better for you.

In this case, I landed to date, but I folded the two commits together, and fixed the commit message while I was at it...

https://hg.mozilla.org/projects/date/rev/a331041b33dab6eceaafd29e4666042d9a5def37

Comment 15

[Simon Fraser [:sfraser] ⌚️GMT]

Comment on attachment 8820815 [details]
based on review, change comment and add config-based level in the route

https://reviewboard.mozilla.org/r/100234/#review102238

All done

Comment 16

[Simon Fraser [:sfraser] ⌚️GMT]

https://github.com/mozilla-releng/funsize/pull/42 up for the main piece of work. 

I didn't notice this had been marked as resolved/fixed. Reopened until the work is done

Comment 17

[Simon Fraser [:sfraser] ⌚️GMT]

Attachment: bug 1319824 Bump funsize version for pulse route name reordering

Review commit: https://reviewboard.mozilla.org/r/103314/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/103314/

Comment 18

[Mihai Tabara [:mtabara]⌚️GMT]

Comment on attachment 8825060 [details]
bug 1319824 Bump funsize version for pulse route name reordering

https://reviewboard.mozilla.org/r/103312/#review104130

Didn't follow-up carefully the pulse BB/TC conversation but from puppet standpoint looks good to me.

Comment 19

[Mihai Tabara [:mtabara]⌚️GMT]

Comment on attachment 8825060 [details]
bug 1319824 Bump funsize version for pulse route name reordering

https://hg.mozilla.org/build/puppet/rev/0b685510abdf

Comment 20

[Simon Fraser [:sfraser] ⌚️GMT]

Comment on attachment 8825060 [details]
bug 1319824 Bump funsize version for pulse route name reordering

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/103314/diff/1-2/

Comment 21

[Aki Sasaki (not active)]

Comment on attachment 8825060 [details]
bug 1319824 Bump funsize version for pulse route name reordering

https://reviewboard.mozilla.org/r/103312/#review105320

Comment 22

[Chris Cooper [:coop] (he/him)]

Anything left to do here?

Comment 23

[Simon Fraser [:sfraser] ⌚️GMT]

With the funsize breakage the other day this is the first day I've been able to verify that the partials actually work, and they do. As far as I can see this is all done

Comment 24

[Rail Aliiev [:rail]]

\o/ \o/ \o/
Thank you Simon!