1279676 - Add --rebuild support for TaskCluster test jobs

Status: RESOLVED FIXED

(Taskcluster :: General, defect)

Description

[Joel Maher ( :jmaher ) (UTC -8)]

It seems that this feature is easy to break.  In bug 1268210 we reported it broken and then fixed it.  Last night I pushed to try:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=52b6078049d495b6ee3f366ee48d0fce843917ba

my jobs didn't rebuild at all, they all ran once!  So I have had to manually retrigger the jobs.

Comment 1

[Armen [:armenzg]]

The last was that we were in a half transition way where we wanted to use a more meaningful name than --rebuild.

I assume this was lost with the recent changes of how graphs are optimized.

Comment 2

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

I think the option is --trigger-tests, not --rebuild:

    # In order to run test jobs multiple times
    parser.add_argument('--trigger-tests', dest='trigger_tests', type=int, default=1)
    # Once bug 1250993 is fixed we can only use --trigger-tests
    parser.add_argument('--rebuild', dest='trigger_tests', type=int, default=1)

and this is supported in the new try parser.  It looks like that try push got re-runs scheduled, too.  WORKSFORME?

Comment 3

[Joel Maher ( :jmaher ) (UTC -8)]

the try push linked was manually retriggered.

As for what we support it appears that nothing support -trigger-tests, but --rebuild is supported for buildbot.

try push with --trigger-tests:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=b4d3b20aaace13b31cf61e8fe7dca9ffc5742610

try push with --rebuild:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=5fc9ed7bfd0b9662458ffcddeef93f2bba5b515d

as it stands we have a loss of functionality for linux64 debug by not supporting --rebuild.  Possibly the fix is to make --trigger-tests work for mozci, although I suspect there is other work required.

Comment 4

[Armen [:armenzg]]

The new file only contains trigger-tests:
https://dxr.mozilla.org/mozilla-central/source/taskcluster/taskgraph/try_option_syntax.py#188

The old file contains both:
https://dxr.mozilla.org/mozilla-central/source/taskcluster/taskgraph/util/legacy_commit_parser.py#

We can drop --trigger-tests completely. No one knows about it. It was an uncomplete attempt to start using a more meaningful flag.

Comment 5

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

Ah, I had assumed --rebuild had been dropped.

Comment 6

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

Attachment: Bug 1279676: use --rebuild instead of --trigger-tests;

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

Comment 7

[Armen [:armenzg]]

Comment on attachment 8763999 [details]
Bug 1279676: use --rebuild instead of --trigger-tests;

https://reviewboard.mozilla.org/r/60066/#review56968

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/cc9757f03c4a
use --rebuild instead of --trigger-tests; r=armenzg

Comment 9

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/cc9757f03c4a

Comment 10

[Geoff Brown [:gbrown]]

--rebuild still does not work for me. 

In https://treeherder.mozilla.org/#/jobs?repo=try&revision=ce499731e5ba555d1aea8fbbc1c2364ea583a05d you can see that buildbot-based Linux 64 opt M1 and M2 were repeated, but all of the taskcluster jobs ran only once.

Comment 11

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

I think the issue is that the legacy stuff, which is what generates the tasks, is always running 'try: -b do -p all -t all -u all' now, regardless of --rebuild.  This may need to wait for some real task kinds before it can be implemented again.

Comment 12

[Joel Maher ( :jmaher ) (UTC -8)]

hmm, can we prioritize this?  --rebuild is pretty critical- I would not want to migrate more jobs to tier-1 until we have more parity with buildbot.

Comment 13

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

You're welcome to land a hack..

Comment 14

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

So the way I'm thinking of doing this in the new 'test' kind is that the try_option_syntax target task method would set task.rebuild to the value of --rebuild, and then the task creation would re-submit the task (generating a new taskId in the process) that many times after first submitting under the assigned taskId.  It means all of the rebuild taskIds aren't included in task-graph.json, but that's probably OK (they would not be in there if the builds were manually retriggered, either).

I haven't thought too hard about this, though!

Comment 15

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

I feel like my being assigned here is preventing someone else from working on this..

Comment 17

[Boris Zbarsky [:bzbarsky]]

I have to echo what Joel says: this functionality is pretty critical when dealing with intermittents.  I'm planning to shift all my try pushes trying to diagnose those to whatever platforms still run on buildbot as a result of this bug..... :(

Comment 18

[Armen [:armenzg]]

I will grab this.

I will evaluate if to re-implement it through triggerbot functionality or in the gecko decision mach target.

Comment 19

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

Thanks, Armen!

Comment 20

[Armen [:armenzg]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=3226e89ae0ee

Comment 21

[Armen [:armenzg]]

Attachment: Bug 1279676 - Support --rebuild try flag to schedule tests N times.

If a developer adds '--rebuild N' to their try syntax they will get test jobs scheduled N times.
This is useful to determine intermittency rate.

This fixes a regression due to the recent refactoring on how we schedule tasks.

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

Comment 22

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

Comment on attachment 8775645 [details]
Bug 1279676 - Support --rebuild try flag to schedule tests N times.

https://reviewboard.mozilla.org/r/67796/#review64840

::: taskcluster/taskgraph/create.py:69
(Diff revision 1)
>              fs[task_id] = e.submit(_create_task, session, task_id,
>                                     taskid_to_label[task_id], task_def)
>  
> +            # Schedule tasks as many times as trigger_tests indicates
> +            for i in range(1, taskgraph.tasks[task_id].attributes.get('trigger_tests', 1)):
> +                # We use slugid() since we want a distinct task id

Please name this something a bit more generic, such as `task_duplicates`.  Also, the new attribute should be documented in `taskcluster/docs/attributes.rst`.

Comment 23

[Armen [:armenzg]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=8aa3df0c1029

Comment 24

[Armen [:armenzg]]

I decided to use 'unittest_task_duplicate'.
Let me know if this works for you.

Comment 25

[Armen [:armenzg]]

Comment on attachment 8775645 [details]
Bug 1279676 - Support --rebuild try flag to schedule tests N times.

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

Comment 26

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

Comment on attachment 8775645 [details]
Bug 1279676 - Support --rebuild try flag to schedule tests N times.

https://reviewboard.mozilla.org/r/67796/#review66042

::: taskcluster/docs/attributes.rst:62
(Diff revision 2)
>  
>  If a unittest suite has subdivisions, those are represented as flavors.  Not
>  all suites have flavors, in which case this attribute should be set to match
>  the suite.  Examples: ``mochitest-devtools-chrome-chunked`` or ``a11y``.
>  
> +unittest_task_duplicate

I'm sorry to do this to you, but.. the idea of my suggestion for another name was to be generic to any kind of task.  For example, we may want to retrigger a docker-image build task, or perhaps fuzzing.  So it's not specific to unittests.  I dislike seeing `unittest` appear in `create.py` since that file should be agnostic to the kinds of tasks it is creating.

I think `task_duplicates` is probably the best choice.

::: taskcluster/docs/attributes.rst:65
(Diff revision 2)
>  the suite.  Examples: ``mochitest-devtools-chrome-chunked`` or ``a11y``.
>  
> +unittest_task_duplicate
> +=======================
> +
> +This is used to indicate that we want multiple test tasks scheduled.

This is used to indicate that we want multiple copies of the task created.
This feature is used to track down intermittent job failures.

If this value is set to N, the task-creation machinery will create a total of N
copies of the task.  Only the first copy will be included in the taskgraph
output artifacts, although all tasks will be contained in the same taskGroup.

While most attributes are considered read-only, target task methods may alter
this attribute of tasks they include in the target set.

::: taskcluster/taskgraph/create.py:76
(Diff revision 2)
>  
>              fs[task_id] = e.submit(_create_task, session, task_id,
>                                     taskid_to_label[task_id], task_def)
>  
> +            # Schedule tasks as many times as unittest_task_duplicate indicates
> +            for i in range(1, taskgraph.tasks[task_id].attributes.get('unittest_task_duplicate', 1)):

nit: line is too long; try running `./mach lint -l flake8 -f treeherder` to find any other issues.

Comment 27

[Armen [:armenzg]]

Comment on attachment 8775645 [details]
Bug 1279676 - Support --rebuild try flag to schedule tests N times.

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

Comment 28

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

Comment on attachment 8775645 [details]
Bug 1279676 - Support --rebuild try flag to schedule tests N times.

https://reviewboard.mozilla.org/r/67796/#review66076

Comment 29

[Armen [:armenzg]]

Comment on attachment 8775645 [details]
Bug 1279676 - Support --rebuild try flag to schedule tests N times.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/67796/diff/3-4/

Comment 30

[Armen [:armenzg]]

Comment on attachment 8775645 [details]
Bug 1279676 - Support --rebuild try flag to schedule tests N times.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/67796/diff/4-5/

Comment 31

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/34cefe01d821
Support --rebuild try flag to schedule tests N times. r=dustin

Comment 32

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/34cefe01d821

Comment 33

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

https://hg.mozilla.org/integration/mozilla-inbound/rev/0329537bee24 fixes the tests.  Bug 1293397 looks at why the tests didn't run when this landed.