1384433 - The decision task should error out when tier n jobs depend on tier > n jobs

Status: RESOLVED FIXED

(Taskcluster :: General, enhancement)

Description

[Mike Hommey [:glandium]]

See for example bug 1384417, which, practically, makes many build jobs have a dependency on the toolchain-*-sccache jobs. Without the last patch in that bug, the sccache jobs are tier 2, but tier 1 jobs depend on it, which is not something that should be permitted.

While the consequence of a toolchain job failing would quickly be noticeable because a lot of build jobs wouldn't start, imagine a situation where a test is tier 1, but depends on a build that is tier 2. As a tier 2 build, the job can fail and be ignored by sheriffs, and the tier 1 test will just simply never run and be invisibly ignored, because its absence will be undetectable in the ocean of tests shown on treeherder.

Comment 1

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

This would make a good taskgraph morph.

Comment 2

[Mike Hommey [:glandium]]

So, I've implemented this, but a morph is either the wrong level to do it, or morphs should happen before optimization, because if that happens after optimization, then we can't check the tiers for optimized-out jobs (and wouldn't find that the windows clang-cl jobs have the wrong tier ; I had to disable optimization to get those errors)

Comment 3

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

Hm, that's a good point.  Maybe that's "good enough" though?

I think the alternative is to check this in the transforms.  But that will require that all tasks with tiers be generated in order by dependencies -- that is, that task A be generated before task B if B depends on A.  Kind dependencies ensure a little of that (from tests to builds) but aren't defined universally.  Maybe build -> test is enough?

Comment 4

[Mike Hommey [:glandium]]

I'm not sure if optimization shouldn't be happening last, actually. Currently, we don't have morphs that add tasks that would end up optimized, but since morph does have the side effect of adding tasks, it seems like it should happen before optimization. What do you think?

Comment 5

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

The checks you want to perform also need to be done before the target task selection, so just switching morphs and optimization isn't enough.

There's already some support for various kinds of verification in taskcluster/taskgraph/generator.py.  You could probably use that, perhaps generalizing it a bit so that verifiers are registered with a decorator or something for each phase of generation.

Another option might be to schedule a sub-task which performs post-facto checks on the taskgraph.  It could do this check against full-task-graph.json and other checks would be free to operate against the optimized task-graph.

Comment 6

[Mike Hommey [:glandium]]

> You could probably use that, perhaps generalizing it a bit so that verifiers are registered with a decorator or something for each phase of generation.

For verify_kinds and verify_parameters, there doesn't seem to be something generalizable, but it seems there's something nice we can do with the others, including verify_task_graph_symbol and verify_gecko_v2_routes.

Comment 7

[Mike Hommey [:glandium]]

Attachment: Bug 1384433 - Make Windows ASAN tests tier 3.

The Windows ASAN builds are tier 3, while the corresponding tests,
currently are tier 2. So we make them tier 3 for consistency.

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

Comment 8

[Mike Hommey [:glandium]]

Attachment: Bug 1384433 - Make clang-cl toolchain jobs tier 1.

Tier 1 Firefox builds are using those toolchain jobs for stylo, so they
need to be tier 1.

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

Comment 9

[Mike Hommey [:glandium]]

Attachment: Bug 1384433 - Generalize verifications done on task graphs.

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

Comment 10

[Mike Hommey [:glandium]]

Attachment: Bug 1384433 - Add a verification that tiers are consistent across dependencies.

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

Comment 11

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

Comment on attachment 8900979 [details]
Bug 1384433 - Make Windows ASAN tests tier 3.

https://reviewboard.mozilla.org/r/172432/#review178096

Better to get r? on this from someone familiar with the status of windows ASAN tests.  That's a lot of work to hide in tier 3 where nobody sees it, so if these truly are unimportant, I'd rather just disable.  Otherwise, if tier 2 was correct, let's raise the test tier.  Or, if there's an active effort to green these up for tier 2, then by all means land this at tier 3.

Comment 12

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

Comment on attachment 8900980 [details]
Bug 1384433 - Make clang-cl toolchain jobs tier 1.

https://reviewboard.mozilla.org/r/172434/#review178100

Comment 13

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

Comment on attachment 8900981 [details]
Bug 1384433 - Generalize verifications done on task graphs.

https://reviewboard.mozilla.org/r/172436/#review178104

Very nice!

Comment 14

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

Comment on attachment 8900982 [details]
Bug 1384433 - Add a verification that tiers are consistent across dependencies.

https://reviewboard.mozilla.org/r/172438/#review178106

Comment 15

[Ting-Yu Chou [:ting] (away)]

Comment on attachment 8900979 [details]
Bug 1384433 - Make Windows ASAN tests tier 3.

https://reviewboard.mozilla.org/r/172432/#review178340

Comment 16

[Mozilla Autoland]

We're sorry, Autoland could not rebase your commits for you automatically. Please manually rebase your commits and try again.

hg error in cmd: hg rebase -s 66a51c6d0241 -d 19142bd26374: rebasing 416395:66a51c6d0241 "Bug 1384433 - Make Windows ASAN tests tier 3. r=ting"
merging taskcluster/ci/test/tests.yml
warning: conflicts while merging taskcluster/ci/test/tests.yml! (edit, then use 'hg resolve --mark')
unresolved conflicts (see hg resolve, then hg rebase --continue)

Comment 17

[Mike Hommey [:glandium]]

Comment on attachment 8900979 [details]
Bug 1384433 - Make Windows ASAN tests tier 3.

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

Comment 18

[Mike Hommey [:glandium]]

Comment on attachment 8900980 [details]
Bug 1384433 - Make clang-cl toolchain jobs tier 1.

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

Comment 19

[Mike Hommey [:glandium]]

Comment on attachment 8900981 [details]
Bug 1384433 - Generalize verifications done on task graphs.

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

Comment 20

[Mike Hommey [:glandium]]

Comment on attachment 8900982 [details]
Bug 1384433 - Add a verification that tiers are consistent across dependencies.

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

Comment 21

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/dd03b1a72adb
Make Windows ASAN tests tier 3. r=ting
https://hg.mozilla.org/integration/autoland/rev/09a14a358ec3
Make clang-cl toolchain jobs tier 1. r=dustin
https://hg.mozilla.org/integration/autoland/rev/b4c2035d49b0
Generalize verifications done on task graphs. r=dustin
https://hg.mozilla.org/integration/autoland/rev/3cfb50b9a03a
Add a verification that tiers are consistent across dependencies. r=dustin

Comment 22

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0f9e6b660dce
Fix yaml lint job a=bustage CLOSED TREE

Comment 23

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/dd03b1a72adb
https://hg.mozilla.org/mozilla-central/rev/09a14a358ec3
https://hg.mozilla.org/mozilla-central/rev/b4c2035d49b0
https://hg.mozilla.org/mozilla-central/rev/3cfb50b9a03a
https://hg.mozilla.org/mozilla-central/rev/0f9e6b660dce

Comment 24

[[e-mail] (:bhearsum)]

This gave me some issues when trying to add dependencies between buildbot bridge and non-buildbot bridge tasks - I filed https://bugzilla.mozilla.org/show_bug.cgi?id=1419778 to deal with that.