1386857 - Remove tc() from Treeherder symbols

Status: RESOLVED FIXED

(Firefox Build System :: Task Configuration, task)

Description

[Gregory Szorc [:gps]]

TaskCluster has taken over the Firefox CI world. Pretty much every result on Treeherder is from TaskCluster. I don't think we need "tc()" to denote items as belonging to TaskCluster any more.

sheriffs: any opinions?

Comment 1

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

No strong opposition to this from me.

Treeherder includes symbol and job group in the job signature hashes, which are then used for things like visibility profiles and not-in-tree tier profiles, so dropping/changing these groups could mean a bunch of jobs that were previously hidden are suddenly visible again until someone tracks them all down and updates the signatures. 

I think perfherder might also use the signatures to track changes over time, so might be good to get feedback from them.

We might be able to do something within Treeherder where it just stops showing the tc/tc- prefixes, but leaves them within the job model so the signatures stay unchanged? 

Once we get all of the stragglers off of buildbot and onto taskcluster, things will be easier (probably completely rework the concept of job visibility/tier so they're defined entirely in-tree, dropping a big usage of the signatures.

Comment 2

[William Lachance (:wlach)]

This isn't the first time this has come up, though I don't know of any bugs filed as of yet. e.g. this was discussed on #treeherder a few days ago:

http://logs.glob.uno/?c=mozilla%23treeherder&s=28+Jul+2017&e=28+Jul+2017&h=tc#c137950

This would be no problem from a perfherder perspective, it doesn't use the job group/type symbols anywhere. 

The treeherder story is more complicated, as :KWierso indicates. My personal inclination would be to just leave things be at least until buildbot is turned off altogether and the visibility issues are taken care of, but :camd/:emorley would probably be the best people to ask.

Comment 3

[Phil Ringnalda (:philor)]

Bug 1277837.

Comment 5

[Dustin J. Mitchell [:dustin] (he/him)]

I was going to work on bug 1277837, but it sounds like there are reasons *not* to do this, so I'll hold off.  Actually doing it will be a pretty easy diff (with a lot of hunks..)

Comment 6

[Andrew Halberstadt [:ahal]]

Wes, would it be possible to use the 'build_system_type' filter in the exclusion profiles instead of the group name? E.g:
https://treeherder.mozilla.org/#/jobs?repo=mozilla-central&filter-build_system_type=taskcluster

If so we could update all the exclusion profiles beforehand to use this, and then land the group-name change without breaking anything.

At the very least, I think it's important we get the 'tc' group name off of build tasks so they aren't hiding in the tc group anymore. Though I'd strongly prefer we don't depend on buildbot being retired to make this change everywhere.

Comment 7

[William Lachance (:wlach)]

(In reply to Andrew Halberstadt [:ahal] from comment #6)

> At the very least, I think it's important we get the 'tc' group name off of
> build tasks so they aren't hiding in the tc group anymore. 

I am not sure, but I suspect we could make this change just for the build tasks without breaking too many things -- I doubt there are many (any?) exclusion profiles which apply to them.

Comment 8

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

I'm going to redirect that question to Ed.

Comment 9

[Ed Morley [:emorley]]

I'm sure doing this for builds is fine :-)

Comment 10

[Andrew Halberstadt [:ahal]]

Sorry, the question I was asking was in comment 6. Can the 'build_system_type' filter be used as part of the exclusion profiles?

Comment 11

[Ed Morley [:emorley]]

I have no idea - Cameron should know :-)

Comment 12

[Cameron Dawson [:camd]]

tl;dr - Go ahead and change your task definitions to no longer have "tc" in them.  And set the Tier appropriately while you're at it.  Tier-3 is hidden.

More info:
This has an, unfortunately, complex answer.  It's not very easy to modify the exclusion profiles.  They're hashes built off of all the values used, so if you change the value, you SHOULD update the hash (the signature) which is what we search on to see if something should be hidden.  But you change the signature hash and then it might orphan existing jobs.  But I don't think we need to go down that path.  It's a bit of a mess...  However, I think there's a better alternative.

Jobs can be hidden in 2 ways:

1. An exclusion profile
2. Being Tier-3

In an ideal world, any TaskCluster job that is hidden is already set as Tier-3 in the task definition.  Maybe we already LIVE in an ideal world and all the TaskCluster jobs have their Tier set appropriately.  I wouldn't be terribly surprised by that.  :)  If some are not, we should make that change to do so.  If they are, then you can remove the "tc" prefix on everything and it'll just go where it should and be hidden when it should.

The work toward making this the official policy is tracked in Bug 1387640.  It's still a work in progress, and I want to get more feedback on it.  But eventually, I want to get rid of the exclusion profiles completely and use Tiers only.

If you want to see what jobs an exclusion profile applies to, you can add these parameters to your Treeherder URL:

exclusion_profile=default&visibility=excluded&filter-tier=1&filter-tier=2&filter-tier=3&filter-build_system_type=taskcluster

like 

https://treeherder.mozilla.org/#/jobs?repo=mozilla-central&exclusion_profile=default&visibility=excluded&filter-tier=1&filter-tier=2&filter-tier=3&filter-build_system_type=buildbot

I don't see any builds that are hidden, fwiw.

Comment 13

[Cameron Dawson [:camd]]

Hmm, but I DO see some hidden jobs that are not Tier-3 on m-c:

https://treeherder.mozilla.org/#/jobs?repo=mozilla-central&exclusion_profile=default&visibility=excluded&filter-build_system_type=taskcluster&filter-tier=1&filter-tier=2&filter-tier=3

Comment 14

[Dustin J. Mitchell [:dustin] (he/him)]

Ideally everything has a tier defined in-tree these days.  It seems a few things were missed, from Cam's note, but that shouldn't be hard to fix..

Comment 15

[(not currently active) Ted Mielczarek]

OK, with that dependent bug fixed I think we should be fine to just remove all the tc groupings. The other jobs that show up in that link in comment 13 are not in tc groupings, so won't be affected.

Comment 16

[William Lachance (:wlach)]

(In reply to Ted Mielczarek [:ted.mielczarek] from comment #15)
> OK, with that dependent bug fixed I think we should be fine to just remove
> all the tc groupings. The other jobs that show up in that link in comment 13
> are not in tc groupings, so won't be affected.

Bikeshed: While we're at it, sould we remove all the "executed by taskcluster" stuff in the job descriptions?

http://searchfox.org/mozilla-central/search?q=by+TaskCluster&case=false

Comment 17

[Andrew Halberstadt [:ahal]]

Yes, let's also remove 'executed by taskcluster'.

Comment 18

[Andrew Halberstadt [:ahal]]

Attachment: Bug 1386857 - [taskgraph] Add path of offending config file to the UNKNOWN_GROUP_NAME error message,

When trying to remove an ubiquitous group like tc(), it's hard to tell where the
error was located without grepping my filesystem. This makes it a bit easier to
find and fix these errors.

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

Comment 19

[Andrew Halberstadt [:ahal]]

Attachment: Bug 1386857 - [ci] Remove the 'tc' treeherder group prefix from all tasks,

There are now only a handful of buildbot jobs remaining and the concern over
outdated treeherder exclusion profiles has largely been resolved.

This does remove the tc() group from a substantial number of tasks which will
now show up as top level tasks, potentially adding clutter. In some cases, we
might want to re-add a new group (e.g group builds or compiled tests together).
However rather than try to predict the best group names for tasks I'm unfamiliar
with, I think it's best to land this as is. Then if things are looking too
cluttered at the root namespace, file follow-up bugs as needed.

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

Comment 20

[Andrew Halberstadt [:ahal]]

If you'd rather I start off with just builds and/or tests, I can do that too, though I figured we might as well just get rid of the prefix wholesale. As far as exclusion profiles go, worst case scenario some tasks show up that shouldn't and we update the profile.

If this looks good to the people here, I'll send out a note to the relevant lists and wait a day before landing.

Comment 21

[Andrew Halberstadt [:ahal]]

As a reminder, if you need to see all the tasks still scheduled with buildbot, you can do this:
https://treeherder.mozilla.org/#/jobs?repo=mozilla-central&filter-build_system_type=buildbot

So I don't think a 'bb' prefix is necessary (and it would involve large out-of-tree changes anyway).

Comment 22

[Dustin J. Mitchell [:dustin] (he/him)]

Comment on attachment 8942336 [details]
Bug 1386857 - [ci] Remove the 'tc' treeherder group prefix from all tasks,

https://reviewboard.mozilla.org/r/212612/#review219032

Comment 23

[Dustin J. Mitchell [:dustin] (he/him)]

Comment on attachment 8942335 [details]
Bug 1386857 - [taskgraph] Add path of offending config file to the UNKNOWN_GROUP_NAME error message,

https://reviewboard.mozilla.org/r/212610/#review219026

Comment 24

[Tom Prince [:tomprince]]

Comment on attachment 8942336 [details]
Bug 1386857 - [ci] Remove the 'tc' treeherder group prefix from all tasks,

https://reviewboard.mozilla.org/r/212612/#review219062

Right now, everything with the `tc` group is grouped, and gets collapsed. Removing the prefix will mean that everything like this will no longer be collapsed, making treeherder somewhat more cluttered.

I wonder if there should still be one (or perhaps more) groups for all the things that are currently grouped under `tc`.

Comment 25

[(not currently active) Ted Mielczarek]

That's a reasonable idea for a follow-up, but I think getting important things like builds broken back out is useful enough to get this landed without fixing that.

Comment 26

[Tom Prince [:tomprince]]

If builds do want to be broken out, then landing this and improving incrementally is definitely the right choice.

Comment 27

[Andrew Halberstadt [:ahal]]

Yeah, I mentioned this in the commit message briefly. I do think we'll want to group some things that become ungrouped (like the misc test jobs maybe), but I think it'll be better to see how it looks first and then figure out the best grouping in follow ups. This way we can also let the task owners make the decision.

Even with more tasks at the top level, treeherder is going to look much less cluttered than it does now.

Comment 28

[Andrew Halberstadt [:ahal]]

Comment on attachment 8942335 [details]
Bug 1386857 - [taskgraph] Add path of offending config file to the UNKNOWN_GROUP_NAME error message,

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

Comment 29

[Andrew Halberstadt [:ahal]]

Comment on attachment 8942336 [details]
Bug 1386857 - [ci] Remove the 'tc' treeherder group prefix from all tasks,

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

Comment 30

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f924db10f84f
[taskgraph] Add path of offending config file to the UNKNOWN_GROUP_NAME error message, r=dustin
https://hg.mozilla.org/integration/autoland/rev/47cfae837c49
[ci] Remove the 'tc' treeherder group prefix from all tasks, r=dustin

Comment 31

[Andrew Halberstadt [:ahal]]

Note to sheriffs:
There's a good chance this might fail when it gets merged to central (e.g if someone adds a new task on inbound). If this happens please let me know (or dustin if I'm not around). It'll be easy to fix directly on central.

Comment 32

[Dorel Luca [:dluca]]

https://hg.mozilla.org/mozilla-central/rev/f924db10f84f
https://hg.mozilla.org/mozilla-central/rev/47cfae837c49

Comment 34

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/be4b8df49597
Port Bug 1386857 to comm-central:  Remove the 'tc' treeherder group prefix from all tasks, r=me