Closed Bug 1319824 Opened 3 years ago Closed 3 years ago

Enable funsize for date

Categories

(Release Engineering :: General, defect)

Product:
Release Engineering
Component:
General
Type:
defect
Priority:
Not set

Tracking

(Not tracked)

Status:
RESOLVED FIXED

People

(Reporter: rail, Assigned: sfraser)

Details

Attachments

(3 files, 2 obsolete files)

bug 1319824 Announce signing jobs to funsize with
59 bytes, text/x-review-board-request
Callek
: review+
Details
based on review, change comment and add config-based level in the route
59 bytes, text/x-review-board-request
Details
bug 1319824 Bump funsize version for pulse route name reordering
59 bytes, text/x-review-board-request
aki
: review+
mtabara
: checked-in+
Details 
Builds on date are TC based, so we'll need to modify funsize to treat them differently. We should track the signed binaries only.
How do we track which jobs (or pulse messages) are related to the production of a signed binary?
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?
Flags: needinfo?(rail)
Flags: needinfo?(bugspam.Callek)
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?
Flags: needinfo?(rail)
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'
(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.
Flags: needinfo?(bugspam.Callek)
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.
Attachment #8820676 - Attachment is obsolete: true
Attachment #8820755 - Attachment is obsolete: true
(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 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 on attachment 8820793 [details]
bug 1319824  Announce signing jobs to funsize with

https://reviewboard.mozilla.org/r/100232/#review100724
Attachment #8820793 - Flags: review?(bugspam.Callek) → review+
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
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
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
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
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Assignee: nobody → sfraser
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 on attachment 8825060 [details]
bug 1319824 Bump funsize version for pulse route name reordering

https://hg.mozilla.org/build/puppet/rev/0b685510abdf
Attachment #8825060 - Flags: checked-in+
Comment on attachment 8825060 [details]
bug 1319824 Bump funsize version for pulse route name reordering

https://reviewboard.mozilla.org/r/103312/#review105320
Attachment #8825060 - Flags: review?(aki) → review+
Anything left to do here?
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
\o/ \o/ \o/
Thank you Simon!
Status: REOPENED → RESOLVED
Closed: 3 years ago → 3 years ago
Resolution: --- → FIXED
Component: General Automation → General
You need to log in before you can comment on or make changes to this bug.