In bug 1632946 we're going to be switching to dynamically assigning what manifests run on which tasks on every push.

It is not fully decided the way to implement that, however, I believe we can land some code in preparation for it and use "Custom action" to try it out the new behaviour.

(writing this here since it feels strange to have two bugs for the same functionality, feel free to copy this to implement this in a separate bug

I chatted with armenzg a bit about this, and I have the following proposal for the implementation. It address both manifest scheduling and Bug 1585757 [1]

The current implementation is as follows:

• When a backfill is requested, it triggers a taskgraph task action, which ends up
  calling taskgraph.actions.backfill.backfill_action, passing the task_id to
  be backfilled. That function has the following parts:

  • query hg.mozilla.org to find the previous N pushes
  • for each push in the recent pushes:
    • load the task graph from the corresponding decision task
    • find the label corresponding the tasked that was passed in
    • depending on the action input, use deep knowledge of what a test task looks like
      to mutate the task
    • create the task and potentially its dependencies.

I propose we split this up into two parts, which can be implemented mostly independently[2]:

• When a backfill is requested, it triggers a taskgraph task action, passing the task_id to be backfilled
  • the function identifies which test manifests and test configuration of the task that was passed in (potentially by looking at task.extra, or pulling the attributes from the graph)
  • querying hg.mozilla.org for the previous N pushes
  • for each push in the recent pushes:
    • trigger a (new) push action on that push, passing as input the test manifests and test configuration.
      • There is not an example of this in-tree, but there is some code in treeherder that does something similar.
• When the new backfill-manifests actions is triggered on each previous push, it is passed the input from the previous stage, but not a task_id:
  • It will combine the parameters from the push, with some new parameters populated from the action input and then run the decision task logic:
    • The merge_automation is an example of doing this, as is the release promotion action.
    • It will need something like this code to avoid triggering new builds, but that is something we can land without and iterate on.
    • the new parameters should include something like target_tasks_method: backfill and optimize_target_tasks: false
  • The taskgraph.transforms.test transform will look at the newly introduced parameters and:
    • create tasks for the requested manifests and test configuration, instead of all manifests, or those indicated by bugbug
    • mark the new tasks as being backfills (not sure what all makes sense but probably change the label, add a new tag and attribute, and adjust the treeherder symbol, at a minimum)
  • The backfill target method will select all tasks with the new backfill attribute that the test transform added to the new tasks
  • The decision logic will create the new tasks and all there dependencies.

If we do take this approach, the implementation of the two actions can be split up. I would recommend:

1. Implementing the second action first, this can be tested on try in isolation, on a single push, by running a custom push action on the push.
2. Implementing the first action in stages:
   a. Implement the logic to extract the test manifest and test configuration, and directly call the implementation of the second action. (This can be tested on a single push, by triggering a custom task action)
   b. Implement the logic to call an action on a given push and use that to trigger the second action on the same push as the first action. (This can also be tested on a single push)
   c. Implement the logic to get pushes from hg.mozilla.org, and call the second action on different pushes. (This is the point where testing will need multiple pushes).

Incidentally, I realized the logic in the second actions implements a significant chunk of the logic that would be needed to implement "add new tasks by manifest" as requested in Bug 1632946 comment 1.

[1] Addressing Bug 1585757 makes the implementation of my proposal easier. Also, addressing that bug becomes more critical if the parallel consideration of optimizing away builds that have tests is undertaken.
[2] This description proposed here assumes that we are backfilling tests. We can either limit the new behavior to tests by setting the action context to be limited to {"kind": "test"}, or handle the non-test tasks in a way that does not try to change the test manifests. Long-term we want to do the later, but that does not need to block this work.

This proposal makes sense to me! Regarding the step around making the transforms load manifests a different way, I already have a patch to implement a mechanism to do this easily:
https://hg.mozilla.org/try/rev/80dc824e69e22a69001427e369a9c4e45c4beb43

I'll get it up for review shortly.

I also wanted to note that Armen had a mostly working PoC for the old method already. I suspect this proposal will be much cleaner than the old backfill function and something we should tackle anyway. But if it's going to be a large amount of work, we'll probably want to de-couple it from the manifest-scheduling project and find a way to tackle them independently.

For my own tracking, here's the PoC work:
https://hg.mozilla.org/try/pushloghtml?changeset=9db545aa28482a955b1cc133156b2dc72075978a

Depends on D78092

Depends on D78093

Depends on D78094

Depends on D78095

Depends on D78096

Depends on D78097

Depends on D78098

Depends on D78099

Depends on D78101

Depends on D78102

Depends on D78103

Depends on D78104

Depends on D78105

This new backfill action takes a specific task and if it is a test
task it will extract the test manifests and backfill the task
with that set of manifests.

This also adds the ability of testing a backfill action without having
to push to Try and to use Treeherder to schedule it. This can be achieved
via Taskcluster temporary client generation through the browser.

Comment on attachment 9154185 [details]
WIP - Bug 1639873 - Backfill test tasks by test manifests

Hi Tom,
This is not ready, however, it shows the direction in which I'm heading + it shows that it can be done since I can schedule from the CLI as well as from the automation.

I still need to do a bunch of other changes and I need to prove what modifying the manifests would look like. I will tackle that next.

I'm interested for your general feedback. No need to go over every detail. I want to know if there's anything outrageous OR if I'm completely off the mark.

See that I'm using an action "new-backfill" and schedule another action "add-new-jobs" for each push.
We actually don't need to schedule other actions from the first action to schedule what we need.

See that I was able to retrigger the tasks that the "add-new-jobs" scheduled. I believe this is something you mentioned mentioned on another bug.

• Retriggered task for push A
• Retriggered task for push B

What do I do with all the other set of Phabricator links attached to this bug?
How do I go about for following changes? Do I edit the history and squash future commits into a single one and then use moz-phab with reorg?

(In reply to Armen [:armenzg] from comment #20)

I still need to do a bunch of other changes and I need to prove what modifying the manifests would look like. I will tackle that next.

I'm interested for your general feedback. No need to go over every detail. I want to know if there's anything outrageous OR if I'm completely off the mark.

Yeah, this is definitely heading in the right direction.

See that I'm using an action "new-backfill" and schedule another action "add-new-jobs" for each push.
We actually don't need to schedule other actions from the first action to schedule what we need.

I'm a little confused by second sentence here, as the first one describes triggering actions from another action.

See that I was able to retrigger the tasks that the "add-new-jobs" scheduled. I believe this is something you mentioned mentioned on another bug.

Yeah, given the the add-new-jobs is running on the right push, that is unsurprising. That said interesting failure of Bug 1585757 is[3] that:

• retriggers of tests on the push where the backfill was requested was using builds from pushes that were backfilled to.
• if a backfill of a test triggered new builds on a push, those builds would not be reused by other backfills needing that build on that push, or add-new-jobs of tests needing that build on that push

That said, while it would not hurt to verify that these work (and did not work as described above before the patch), the new design avoids that behavior by construction.

What do I do with all the other set of Phabricator links attached to this bug?

You can "abandon" all those revisions in phabricator (it is one of the option in the drop down above the comment box at the bottom).

How do I go about for following changes? Do I edit the history and squash future commits into a single one and then use moz-phab with reorg?

I can't speak to the details of moz-phab in particular, as I don't use it. You will want to edit the history before submitting again, though I think you should end up with multiple commits, which will show up as multiple revisions in phabriactor.

There isn't a hard-and-fast rule on how to divide things; my general rule of thumb is that each commit be reviewable on its own and contain a single idea. I generally try to make it so each point in the series could stand alone (i.e. tests pass and lint clean and don't have structural dependencies on later patches). Often my stacks end up with a bunch of refactoring and and adding infrastructure, and then a couple of small commits that make use of those changes.

[1] I haven't actually dug into the failures to verify, and I'm not sure that the tasks still exists, I'm fairly certain that this is what is going on, as it is the behavior I would expect from the current implementation.

(In reply to Tom Prince [:tomprince] from comment #21)

(In reply to Armen [:armenzg] from comment #20)

See that I'm using an action "new-backfill" and schedule another action "add-new-jobs" for each push.
We actually don't need to schedule other actions from the first action to schedule what we need.

I'm a little confused by second sentence here, as the first one describes triggering actions from another action.

The first sentence implies the following:

new-backfill action executes
- for each push, *schedule* add-new-jobs *action*

however, we could execute the work of the add-new-jobs within the execution of new-backfill like this:

new-backfill action executes
- for each push, *call* the add-new-jobs *code*

(In reply to Armen [:armenzg] from comment #22)

(In reply to Tom Prince [:tomprince] from comment #21)

(In reply to Armen [:armenzg] from comment #20)

See that I'm using an action "new-backfill" and schedule another action "add-new-jobs" for each push.
We actually don't need to schedule other actions from the first action to schedule what we need.

I'm a little confused by second sentence here, as the first one describes triggering actions from another action.

The first sentence implies the following:

new-backfill action executes
- for each push, *schedule* add-new-jobs *action*

however, we could execute the work of the add-new-jobs within the execution of new-backfill like this:

new-backfill action executes
- for each push, *call* the add-new-jobs *code*

Yeah, the current implementation of backfill is essentially is essentially the second thing there (not in terms of lines of code executed, but equivalent observable behavior). While that seems more efficient, the way we discover what existing tasks there are for a push means that all the tasks created by the action appear associated to the push where the backfill was requested.[1] The split that is your first options is how we fix Bug 1585757.

[1] It *might be possible to change this, but the way we currently discover existing tasks is by looking at (among other tihngs) all the action tasks on a given push. We'd need to add:

• a new way to discover which actions to look at for tasks
• annotate the metadata of those tasks to associate them with the right pushes

I think the added complexity of that outweighs the complexity that needing to task has.

Hi Tom,
Since I was doing my development from my localhost I did not envision the problem of improper scopes in automation. The failing task.

The backfill action has "assume:repo:hg.mozilla.org/try:action:generic", however, it needs the scope hooks:trigger-hook:project-gecko/in-tree-action-1-generic/95e3d0a9c5.

I see the the role does not have hooks:trigger-hook:project-gecko/in-tree-action-1-generic/95e3d0a9c5.

What options do we have?

[task 2020-06-11T19:50:14.339Z] Traceback (most recent call last):
[task 2020-06-11T19:50:14.339Z]   File "/builds/worker/checkouts/gecko/taskcluster/mach_commands.py", line 276, in action_callback
[task 2020-06-11T19:50:14.339Z]     test=False)
[task 2020-06-11T19:50:14.339Z]   File "/builds/worker/checkouts/gecko/taskcluster/taskgraph/actions/registry.py", line 319, in trigger_action_callback
[task 2020-06-11T19:50:14.339Z]     cb(Parameters(**parameters), graph_config, input, task_group_id, task_id)
[task 2020-06-11T19:50:14.339Z]   File "/builds/worker/checkouts/gecko/taskcluster/taskgraph/actions/new_backfill.py", line 143, in new_backfill_action
[task 2020-06-11T19:50:14.339Z]     new_action_input,
[task 2020-06-11T19:50:14.339Z]   File "/builds/worker/checkouts/gecko/taskcluster/taskgraph/actions/util.py", line 64, in trigger_action
[task 2020-06-11T19:50:14.340Z]     trigger_hook(action['hookGroupId'], action['hookId'], hook_payload)
[task 2020-06-11T19:50:14.340Z]   File "/builds/worker/checkouts/gecko/taskcluster/taskgraph/actions/util.py", line 76, in trigger_hook
[task 2020-06-11T19:50:14.340Z]     hooks.triggerHook(hook_group_id, hook_id, hook_payload)
[task 2020-06-11T19:50:14.340Z]   File "/builds/worker/checkouts/gecko/third_party/python/taskcluster/taskcluster/hooks.py", line 173, in triggerHook
[task 2020-06-11T19:50:14.340Z]     return self._makeApiCall(self.funcinfo["triggerHook"], *args, **kwargs)
[task 2020-06-11T19:50:14.340Z]   File "/builds/worker/checkouts/gecko/third_party/python/taskcluster/taskcluster/client.py", line 270, in _makeApiCall
[task 2020-06-11T19:50:14.340Z]     response = self._makeHttpRequest(entry['method'], _route, payload)
[task 2020-06-11T19:50:14.340Z]   File "/builds/worker/checkouts/gecko/third_party/python/taskcluster/taskcluster/client.py", line 542, in _makeHttpRequest
[task 2020-06-11T19:50:14.340Z]     superExc=None
[task 2020-06-11T19:50:14.340Z] TaskclusterRestFailure: This request requires Taskcluster credentials that satisfy the following scope expression:
[task 2020-06-11T19:50:14.340Z] 
[task 2020-06-11T19:50:14.340Z] ```
[task 2020-06-11T19:50:14.340Z] hooks:trigger-hook:project-gecko/in-tree-action-1-generic/95e3d0a9c5
[task 2020-06-11T19:50:14.340Z] ```

This new backfill action takes a specific task and if it is a test
task it will extract the test manifests and backfill the task
with that set of manifests.

This also adds the ability of testing a backfill action without having
to push to Try and to use Treeherder to schedule it.

From our conversation yesterday.

Tom's actions:

• Figure out how to fix missing scopes from comment 24.
• Review current patch
• Follow up a refactoring to use the taskgraph logic. Related code:
  • https://searchfox.org/mozilla-central/source/taskcluster/taskgraph/actions/add_new_jobs.py#48-49
  • https://searchfox.org/mozilla-central/rev/2c1092dc68c63f7bad6da6a03c5883a5ab5ff2ca/taskcluster/taskgraph/actions/merge_automation.py#78-103
  • https://searchfox.org/mozilla-central/source/taskcluster/taskgraph/util/chunking.py#141-167

Armen's actions:

• Move info from docstrings into oficial documentation
• Move logic added to add-new-jobs to new action
• Handle where original chunked task does not exist on current push
• Move input_for_add_new_jobs to intermediary action

This allows testing backfill functionality on Try without adding jobs
to the pushes of others.

Depends on D79351

Depends on D79999

Depends on D80530

Depends on D80531

Depends on D80532

Depends on D80533

Depends on D80534

Depends on D80535

An attempt was made to make all moved code not to use the Taskcluster
client library, however, making an authenticated post request would
be reimplementing the logic from the Taskcluster client, thus, leaving
the logic of triggering a hook still using the Taskcluster library.

Depends on D80536

When trying to test the backfill_by_manifest action we will only
require the task-id and make task-group-id optional.

When the task-group-id is not specified by the group we will
default to the one associated to the task-id, thus, the scheduled
task will be on the same push.

Depends on D80537

Depends on D80538

Depends on D80538

Depends on D80728

I'm going to group various commits that are logically related and discard the ones unwanted.

Since we look at all action on a push to find tasks associated with that push, we
want to split the backfill action in two, with the action that schedules jobs on
a given push being an action on that push. This adds a new action permission to
allow the first action to trigger the hook for the second action.

We currently base this on the cb_name, but it would make be better to make this
explicit, so that different actions can share a permisssion.

Depends on D80533

Depends on D79999

https://hg.mozilla.org/mozilla-central/rev/71961455f5a1

Depends on D79999

Depends on D81379

I've got this working and I believe I've restructured the patches to make the review process easier.

There's two hacks on my Try pushes:

• On Try it only iterate over patches of the same author
• Force backfill requests of -2 chunks to be -9
  • When the -9 label is not found it will fallback to existing -1 or not-numbered labels from the task graph

Steps on automation:

• Select this bc2 task
  • All other pushes won't work
• Open hamburger menu bottom left, access "Custom action" and select the "new-backfill" action
• Change these values depth: 1 inclusive: true of the payload and "Submit"

What will happen:

• You will see a "Bk" (backfill) task on the same push
• That task will trigger a hook per push and an "add-test-task" action will get scheduled
  • The action task will be in charge of scheduling the task on that push

You can see that the originally backfilled bc2 task and the finally scheduled bc1 task share the same MOZHARNESS_TEST_PATHS value.

If you checkout the code locally you can do the same from the CLI (It won't schedule the backfill command but it will everything else):

• Follow the authentication docs
• Run:

./mach taskgraph test-action-callback new-backfill --input taskcluster/taskgraph/actions/sample_input.yml --task-id <task_id>

NOTE: It won't quite work right this moment for all my pushes because I've renamed few things in the last few pushes.

TODO: I still need to rename the symbol to be something easily recognizable but that will a very minor change. I will tackle that on Monday

I've scheduled a task with a modified symbol.

bc2-short_revision instead of bc2.

This makes it a bit more clear as to what was the originating backfilled task.

Depends on D81379

This shows the same task being backfilled across pushes and they make reference to the originating revision:
https://treeherder.mozilla.org/#/jobs?repo=try&group_state=expanded&collapsedPushes=739362&author=armenzg%40mozilla.com&searchStr=linux%2C18.04%2Cx64%2Cdebug%2Cmochitests%2Ctest-linux1804-64%2Fdebug-mochitest-browser-chrome-e10s-1%2Cm(bc1-d68de04a896)&fromchange=4849460bcbaf7cce4c49a1411d94e952efc4af2a

Nice! Maybe we should put backfills in a different group symbol as well? E.g, M-backfill or something like that. Just thought it might make it cleaner.

Depends on D81384

Depends on D81754

I'm not modifying the groupName since there's a UI trick I can take advantage of that changing it would prevent me to use.

I backfilled wpt from the top push with these settings (depth:2 inclusive: true).

This link shows originally scheduled task + backfilled ones.

Depends on D82064

It seems the new-backfill action does not work on autoland.

[task 2020-07-03T20:51:55.571Z] https://hg.mozilla.org/integration/autoland/json-pushes?version=2&startID=120735&endID=120737
[task 2020-07-03T20:51:56.773Z] Failure to trigger action for 120736
[task 2020-07-03T20:51:56.773Z] __call__() got an unexpected keyword argument 'use_proxy'
[task 2020-07-03T20:51:57.363Z] Failure to trigger action for 120737
[task 2020-07-03T20:51:57.363Z] __call__() got an unexpected keyword argument 'use_proxy'
[taskcluster 2020-07-03 20:51:57.954Z] === Task Finished ===
[taskcluster 2020-07-03 20:51:58.036Z] Successful task run with exit code: 0 completed in 14.508 seconds

I will look into it on Monday.

https://hg.mozilla.org/mozilla-central/rev/3f093b7c209c
https://hg.mozilla.org/mozilla-central/rev/3ddf49b60369

https://hg.mozilla.org/mozilla-central/rev/596728ec69ca

The new backfill action works on autoland.

• Scheduled tasks
• Action tasks

Both the original and scheduled tasks share the same MOZHARNESS_TEST_PATHS.
The next step is replacing the current backfill on bug 1650224.

"MOZHARNESS_TEST_PATHS": "{\"mochitest-browser-chrome\": [\"accessible/tests/browser/events/browser.ini\", \"accessible/tests/browser/states/browser.ini\", \"browser/base/content/test/captivePortal/browser.ini\", \"browser/components/contextualidentity/test/browser/browser.ini\", \"browser/components/enterprisepolicies/tests/browser/disable_developer_tools/browser.ini\", \"browser/components/pioneer/test/browser/browser.ini\", \"browser/components/search/test/browser/browser.ini\", \"browser/modules/test/browser/formValidation/browser.ini\", \"docshell/test/browser/browser.ini\", \"dom/security/test/cors/browser.ini\", \"toolkit/components/cleardata/tests/browser/browser.ini\", \"toolkit/components/passwordmgr/test/browser/browser.ini\", \"toolkit/components/reader/test/browser.ini\"]}"