1879118 - Coalesce beta CI runs

Status: RESOLVED FIXED

(Firefox Build System :: Task Configuration, task)

Description

[:glob ✱]

Some time ago Release Management along with Aryx were chatting about load/backlog related concerns following the changes to the uplift process.

The legacy process involved Release Managers manually coalescing patches into a single push.
The new process has each uplift request in an individual push, increasing the total number of pushes and therefore demands on the CI infrastructure.

Aryx suggested the following: we could stop running all jobs on beta, instead have a cron that runs only after there hasn't been a push within the last N minutes.

Given the large backlog encountered recently (5000+ macOS jobs queued!) we should enact Aryx's suggestion.

Comment 1

[Ryan VanderMeulen [:RyanVM]]

We should keep builds on every push at a minimum. The main issue is the hardware pools, so we could also conceivably keep many Linux/Windows jobs around and restrain this only to the most constrained pools. But I guess that would also depend on the frequency of the cron.

Thinking out loud, we'd also want this to work with TH's "Trigger missing jobs" functionality so we can expedite in the event of a chemspill-type situation.

Comment 2

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

Yesterday I waited 8 hours for windows testers (azure based) on try, so this doesn't affect only hardware. I agree with the need for having builds.

Possibly we could use the bugbug scheduling like we do on autoland and every 5th push do all tests otherwise only relevant tests?

Comment 3

[Ryan VanderMeulen [:RyanVM]]

We need to make sure we always have the full set of tests on the most recent non-DONTBUILD push, though. Sheriffs have more flexibility around choosing what commit to use as a merge candidate while we need to build releases off specific commits and don't have the luxury of waiting for a later push with full CI triggered.

I'd be OK with using bugbug to handle initial test scheduling, but I think we'd still want the cron forcing full CI to happen on a regular interval.

Comment 4

[Connor Sheehan [:sheehan]]

Joel, would your team be best suited to address this bug? I discussed this at length with RelMan at our recent work week. It is one of the major things blocking adoption of Phab/Lando uplifts as the default, since in the new workflow each uplift is landed as a single push, instead of multiple uplifts being pushed at once after manually applying the patches in a local checkout. The increase in the number of pushes causes much higher load on CI given our current scheduling strategy, which is a risk for releases.

RelMan needs to be able to have full CI runs for things we release. We discussed a few options for ways we could make this happen:

• A cron which schedules full CI on release trains every N hours/minutes/etc, with only partial CI running on most pushes.
• Bugbug/autoland style scheduling on beta/release/etc. A hard blocker for this would be the ability to schedule a full CI run with a button on Treeherder.

If there are other options that we haven't considered it would be great to hear those too. We did consider attempting to implement coalescing uplifts together into a single landing in Lando, but it seems like this is more of a CI scheduling problem, and we would like to keep the uplift workflow as similar to the workflow for regular landings as possible.

Comment 5

[Ryan VanderMeulen [:RyanVM]]

(In reply to Connor Sheehan [:sheehan] from comment #4)

• Bugbug/autoland style scheduling on beta/release/etc. A hard blocker for this would be the ability to schedule a full CI run with a button on Treeherder.

As a quick follow-on to this point, it's my understanding that the existing "Trigger missing jobs" functionality on Treeherder does not trigger the same complete set of jobs that a backstop push schedules. This is the crux of the concern regarding this point.

Comment 6

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

it sounds like action items are:

1. add a "run ALL tests" button on treeherder for the sheriffs
2. use bugbug for reduced test selection

Both of these items should be solvable, although I wonder why current tools are not working for #1, so this might not be as straightforward as we think.

Comment 7

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

:aryx, we have a custom push action "run missing tests": https://searchfox.org/mozilla-central/source/taskcluster/gecko_taskgraph/actions/run_missing_tests.py

does this not work in general? does this only work on central/autoland?

Comment 8

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

The issue with "Run Missing Tests" on autoland is it will schedule all tasks but not all manifests - a bc1 task with one manifest will still contain that one manifest. Fixing this moved to backlog.

If there is not manifest optimization but tasks with full manifest sets have not been scheduled, this is not an issue.

Comment 9

[Connor Sheehan [:sheehan]]

(In reply to Joel Maher ( :jmaher ) (UTC -8) from comment #6)

it sounds like action items are:

1. add a "run ALL tests" button on treeherder for the sheriffs
2. use bugbug for reduced test selection

Both of these items should be solvable, although I wonder why current tools are not working for #1, so this might not be as straightforward as we think.

Joel, does comment 8 clarify why tools aren't working for #1? Is fixing this more or less straightforward?

Comment 10

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

I am pretty sure the items listed are accurate, and #2 should be easy to fix. as for #1, I am not sure how to fix it. Possibly a few days of hacking and creative work could solve it. It wouldn't be a priority as it isn't straightforward and we haven't been able to solve it in the last 4 years. There are no developers who work on treeherder even half time, it is way understaffed. Luckily we do have contractors we can hire to do a lot of work on it, that takes budget.

We made a decision to run every commit standalone on beta/release, I assume that meant planning for increased budget and need for increasing pool sizes. If that calculus has changed, we need to look at all options.

I see:

1. using what we do on autoland (which is what we have been discussing)
2. going back to the original method of coalescing patches, probably not ideal
3. using a cron job or backstop pushes that run ALL jobs, that wouldn't guarantee every release would have a full set of tests
4. finding the gap in budget/capacity difference and filling it.

Personally I would prefer to see #1 solved as it would help scale for future projects.

Comment 11

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

in order to do what we do on autoland, yet have the ability for "add new jobs", we would need to modify the decision task artifacts. Specifically, full-task-graph.json as this has only the 'test-manifests' which bugbug determines are needed.

I am not sure if there would be problems with having task-graph.json have a subset of test-manifests, and the full-task-graph.json having the entire set of manifests

Comment 12

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

it appears that full-task-graph.json has ALL the tasks, but test tasks have reduced manifests based on bugbug/try filters.

then target-tasks filter out tasks that are not relevant, creating a much shorter list.

the change I think we need is to keep ALL the manifests in full-task-graph.json, but then when we reduce down to target-tasks, optimized-tasks, etc. we reduce the manifests.

This will cause some confusion in tests, because if we run task mda with 1 manifest in target_tasks, then we "run all jobs", mda will have 10 manifests- but be called the same thing- how would we know what is filtered and what is full?

Effectively we cannot do a "run all missing jobs", we would need to do a "run all jobs". But we can reuse the builds and other dependencies (probably an edge case to consider).

maybe if we do a "run all jobs" it would have different groups visible on treeherder- or move existing tests to tier-3, or some other group?

:aryx, if you could give some input here, I would appreciate it, this might not be so easy to solve by hacking the decision task + treeherder/taskcluster action, but it seems possible. what am I missing?

Comment 13

[Julien Cristau [:jcristau]]

the change I think we need is to keep ALL the manifests in full-task-graph.json, but then when we reduce down to target-tasks, optimized-tasks, etc. we reduce the manifests.

From a taskgraph point of view this is quite awkward / best avoided IMO.

Comment 14

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

Attachment: Bug 1879118 - optimize beta test tasks: run complete set only for every 10th push, else every 10th test. r=#taskgraph-reviewers

Comment 15

[Andrew Halberstadt [:ahal]]

Attachment: Bug 1879118 - Run constrained tests on beta only every 5th push, r?#taskgraph-reviewers!,marco!

Comment 16

[Phabricator Automation]

firefox-beta Uplift Approval Request

• User impact if declined: capacity issues on mac pools
• Code covered by automated testing: yes
• Fix verified in Nightly: no
• Needs manual QE test: no
• Steps to reproduce for manual QE testing:
• Risk associated with taking this patch: low
• Explanation of risk level: automation only
• String changes made/needed: N/A
• Is Android affected?: no

Comment 17

[Andrew Halberstadt [:ahal]]

Attachment: Bug 1879118 - Run constrained tests on beta only every 5th push,

Original Revision: https://phabricator.services.mozilla.com/D285838

Comment 18

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/3fa7d30a5e3b
https://hg.mozilla.org/integration/autoland/rev/1dbb9e6b176a
Run constrained tests on beta only every 5th push, r=marco,taskgraph-reviewers,release-managers,diannaS,bhearsum

Comment 19

[Iulian Moraru]

https://hg.mozilla.org/mozilla-central/rev/1dbb9e6b176a

Comment 20

[Pulsebot]

https://github.com/mozilla-firefox/firefox/commit/1c4ee256854f
https://hg.mozilla.org/releases/mozilla-beta/rev/2e358e889ff5

Comment 21

[Pascal Chevrel (relman team) -> :pascalc]

Should D285804 be closed now that the bug is resolved? It is marked for beta uplift but not accepted and the other patch in this bug was uplifted to beta. Thanks.

Comment 22

[Ryan VanderMeulen [:RyanVM]]

Will this avoid including DONTBUILD pushes in the "is this the 5th push?" calculation? If not, can it be made to? :-)

Comment 23

[Andrew Halberstadt [:ahal]]

No and yes*

*I know this is how backstops work so it's definitely possible.. but I'll need to re-learn this and unsure when that can be.