1550611 - Reduce number of builds triggered on push

Status: RESOLVED FIXED

(Thunderbird :: Build Config, task)

Description

[Rob Lemley [:rjl]]

Request to not run shippable builds on push for Thunderbird.

The idea is to have only opt and debug run on push, then shippable runs nightly, effectively restoring the old behavior.

Comment 1

[Rob Lemley [:rjl]]

Attachment: no_shippable_on_push_v1.patch

WIP!

TODO:

• Figure out what's going on with MacOS
• Should comm-beta and comm-esr be treated differently?

Comment 2

[Jorg K (CEST = GMT+2)]

(In reply to Rob Lemley [:rjl] from comment #0)

The idea is to have only opt and debug run on push, then shippable runs nightly, effectively restoring the old behavior.

I noticed that Nightly runs don't actually trigger a build even if the underlying M-C version has changes, see for example here:
https://treeherder.mozilla.org/#/jobs?repo=comm-central&revision=6bd11ad2949dc7454e6b98f9613064621ee319f3

The Nightly run only ran L10N and repack jobs, but based on the build that happened hours before.

For sheriffing that means that in order to get an up-to-date build for Nightly, we must push after the M-C merge or push before the M-C merge with DONTBUILD. In the former case the push will trigger a full build and the the Nightly run will just add the L10N bits, in the latter case, at least the shippable builds will run, like here:
https://treeherder.mozilla.org/#/jobs?repo=comm-central&revision=fc51c0e147cc1544f5fa61bb01b2cfd1ba87a89d

Unless I'm mistaken, the old behaviour was actually less work.

Comment 3

[Rob Lemley [:rjl]]

I'll see what I can do. Thanks for the input.

Comment 4

[Rob Lemley [:rjl]]

Attachment: remove include-push-tasks from .cron.yml [landed, comment #7]

include-push-tasks does not make sense for C-C workflow and should not have
been included.
Removing this will force a full build when the daily build triggers.

help='Whether tasks from the on-push graph should be re-used '
                          'in this graph. This allows cron graphs to avoid rebuilding '
                          'jobs that were built on-push.

Comment 5

[Jorg K (CEST = GMT+2)]

Comment on attachment 9064609 [details] [diff] [review]
remove include-push-tasks from .cron.yml [landed, comment #7]

OK, so this will force the Daily run to build new shippables since the M-C version could have changed since the last build.

I'm a bit surprised, since these lines were just added here
https://hg.mozilla.org/comm-central/rev/b58e5d1520e0#l1.12
to fix some sort of problem.

Comment 6

[Rob Lemley [:rjl]]

Yes I know. I should have backed it out as it didn't fix the problem as I expected.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/25aaacb02b1d
remove include-push-tasks from .cron.yml. r=jorgk DONTBUILD

Comment 8

[Rob Lemley [:rjl]]

Attachment: Do not run shippable builds on push [landed, comment #12; backed out, comment #14]

Comment 9

[Rob Lemley [:rjl]]

Attachment: Disable macosx64/opt build on push [landed, comment #12; backed out, comment #14]

macosx64-shippable will get run on push.

Comment 10

[Rob Lemley [:rjl]]

Attachment 9065287 [details] [diff] and 9065288 need landing.
My instinct is to just remove the macosx64/opt build altogether and only leave macosx64-shippable.
This disables it instead so if there are jobs further down the pipeline that depend on it, it will build and we don't see bustage.

Comment 11

[Jorg K (CEST = GMT+2)]

What's the reason to treat Linux/Windows and Mac differently? Why not just also disable Mac shippable by default? Oh, I see, that because on Linux/Windows the tests run on both "opt" and "shippable opt" and for Mac they only run on "shippable opt". So if we disabled "shippable opt", we'd lose the tests.

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/b206f4d87208
Do not run Linux/Windows shippable builds on push. r=darktrojan
https://hg.mozilla.org/comm-central/rev/6512afbb0b56
Disable macosx64/opt build on push. r=darktrojan DONTBUILD

Comment 13

[Rob Lemley [:rjl]]

(In reply to Jorg K (GMT+2) from comment #11)

What's the reason to treat Linux/Windows and Mac differently? Why not just also disable Mac shippable by default? Oh, I see, that because on Linux/Windows the tests run on both "opt" and "shippable opt" and for Mac they only run on "shippable opt". So if we disabled "shippable opt", we'd lose the tests.

Right, and I think that move has something to do with Notarized Builds but I'm not sure.

Comment 14

[Jorg K (CEST = GMT+2)]

Backout:
https://hg.mozilla.org/comm-central/rev/7321cfcf8b65cff0723ab5f01b2e9d993ab1557a
Backed out 2 changesets (bug 1550611) for breaking test runs on Daily/Nightly-only runs. a=backout

Comment 15

[Jorg K (CEST = GMT+2)]

Oh, I shouldn't have backed out the Mac part, since Mac test did run here:
https://treeherder.mozilla.org/#/jobs?repo=comm-central&revision=6512afbb0b5671acb2c4550b23f492d6cd2f5b72

Tests are back on Linux (and likely Windows when the time comes) here:
https://treeherder.mozilla.org/#/jobs?repo=comm-central&revision=4516cf2270f464e5e8bc19fbf2b0ca6fff10ca58

Comment 16

[Jorg K (CEST = GMT+2)]

This has sadly stalled for a month :-(

Some observations: SeaMonkey pushes for suite/ only still trigger builds:
https://treeherder.mozilla.org/#/jobs?repo=comm-central&revision=b26f815835648f3e0e80f9568d2adb33f5c97251

If the last push was a SM push for suite/ only, then Daily will not run tests, that's bad:
https://treeherder.mozilla.org/#/jobs?repo=comm-central&revision=12611e8ca4489c44a3bb6e8c3c685117e6afbd87

So we still want to reduce the number of build and build the right things at all times.

Comment 17

[Rob Lemley [:rjl]]

(In reply to Jorg K (GMT+2) from comment #16)

Some observations: SeaMonkey pushes for suite/ only still trigger builds:

I suspect the shippable builds require the repack tasks to run, and they're pulling in the shippable-build tasks.

If the last push was a SM push for suite/ only, then Daily will not run tests, that's bad:

Yes. This might be difficult to achieve given the requirement that we don't trigger builds on suite-only checkins, but I'll see what I can come up with.

So we still want to reduce the number of build and build the right things at all times.

Unfortunately, this shippable build thing is great for Firefox, but not so great for Thunderbird's workflow. The "nightly_desktop" target task that the cron task runs to build the Dailies assumes that tests already ran. That's just simply not the case for us.

I'll bump this up my priority queue.

[edit: fix formatting]

Comment 18

[Rob Lemley [:rjl]]

Quick update: I have some changes that I thought would work out, but there's some problems with tasks getting removed that we want to run, and there's some of the optimization algorithm jumps have me concerned that we will trigger Bug 1414043, and I really want to avoid that. Still working on this!

Comment 19

[Rob Lemley [:rjl]]

Bug 1422060 applies here as well.

Comment 20

[Rob Lemley [:rjl]]

Attachment: verbump_dontbuild.patch

This option is passed along to the treescript worker that actually
does the commit. The option is in use by Firefox, so I expect no issues
with it.
Please land upon r+.

Comment 21

[Rob Lemley [:rjl]]

Comment on attachment 9079151 [details] [diff] [review]
verbump_dontbuild.patch

># HG changeset patch
># User Rob Lemley <[e-mail]>
># Date 1563481842 14400
>#      Thu Jul 18 16:30:42 2019 -0400
># Node ID d442b810ccbfc53ef75ad471c1f43437b639706c
># Parent  2f19bf8b7bb8fa4fce1cd44783f734e2979a1eb7
>Bug 1550611 - Use dont-build commit when tagging release revisions. r?darktrojan
>
>Request from jorgk. This option is set in the Firefox config.
>
>diff --git a/taskcluster/ci/release-version-bump/kind.yml b/taskcluster/ci/release-version-bump/kind.yml
>--- a/taskcluster/ci/release-version-bump/kind.yml
>+++ b/taskcluster/ci/release-version-bump/kind.yml
>@@ -17,16 +17,17 @@ job-defaults:
>     run-on-projects: []
>     shipping-phase: ship
>     worker-type:
>         by-project:
>             comm-(beta|esr60|esr68): scriptworker-prov-v1/tb-treescript-comm-v1
>             default: scriptworker-prov-v1/tb-treescript-comm-dev
>     worker:
>         implementation: treescript
>+        dontbuild: true
>         tags: ['buildN', 'release']
>         bump: true
>         bump-files:
>             by-project:
>                 default: ["mail/config/version_display.txt"]
>                 comm-(beta|esr60|esr68):
>                     - "mail/config/version.txt"
>                     - "mail/config/version_display.txt"

Comment 22

[Rob Lemley [:rjl]]

Attachment: verbump_dontbuild.patch [landed, comment #23]

This option is passed along to the treescript worker that actually
does the commit. The option is in use by Firefox, so I expect no issues
with it.
Please land upon r+.

Version 2 removes DONTBUILD from the commit message so it doesn't
cause problems for the sheriffs.

Comment 23

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/852ab23d79ee
dont build on push when tagging release revisions. r=darktrojan

Comment 24

[Jorg K (CEST = GMT+2)]

Hmm, here's the grammar police: What is "dont"?

Comment 25

[Jorg K (CEST = GMT+2)]

Comment on attachment 9079190 [details] [diff] [review]
verbump_dontbuild.patch [landed, comment #23]

You could have requested uplift straight away.

Comment 26

[Jorg K (CEST = GMT+2)]

This is not really fixed since the main patches here got backed out. Overall, we're still creating too many unneeded tasks.

Comment 27

[Jorg K (CEST = GMT+2)]

TB 69 beta 2, just missed beta 1, but will be effective when tagging:
https://hg.mozilla.org/releases/comm-beta/rev/15913924cdc856474540c7ca7723e6aa7ebfc342

Comment 28

[Jorg K (CEST = GMT+2)]

TB 68.0 ESR:
https://hg.mozilla.org/releases/comm-esr68/rev/43c261a50c3e66bbac75c2866ffaf29274a53253

Comment 29

[Rob Lemley [:rjl]]

Attachment: fix_extra_builds_halloween.patch

This patch requires the patch waiting for review in bug 1592162.

And I think it will work. We won't really know until the conditions triggering
the undesired builds occur.

If my try job works out, this should address the issues raised in this bug
regarding too many build jobs.

Specifically;
- no more shippable builds on Seamonkey-only pushes
- always run the daily build even when the last push is seamonkey-only
- run the tests on seamonkey-only dailies.

Try is funny, so I'm checking that now.

I compared task lists with mach taskgraph to predict outcomes of the various
scenarios.

I grabbed parameters.yml files from three recent pushes to comm-central.
1) suite-only push, 2) regular thunderbird push, 3) thunderbird push for which
later got used for a Daily, 4) the parameters file for the Daily itself. 
With that I was able to create what the parameters.yml file would be if the
Suite-only push had been the one used for a Daily.

Then it was a matter of running "mach taskgraph optimized" with the various
paramters files and comparing output.

Source tests and toolchain builds may still run for Seamonkey-only pushes. As
toolchains are cached and reused, I don't have a problem with that. Source tests
are pretty small jobs. I can address in a follow-up if desired.

Linux32/opt tests don't exist. So no Linux32/opt tests on push, just on the
shippple variant on push. I can leave as is or bring them back.

Comment 30

[Geoff Lankow (:darktrojan)]

Comment on attachment 9105466 [details] [diff] [review]
fix_extra_builds_halloween.patch

This makes sense, although your comment on always_shippable isn't right. I guess you wrote it then changed what the code did.

Comment 31

[Jorg K (CEST = GMT+2)]

I guess you're referring to: "Sets "always_target" to true on shippable platforms when running a nightly build."

So let's fix this before landing this.

Comment 32

[Rob Lemley [:rjl]]

(In reply to Jorg K (GMT+2) from comment #31)

I guess you're referring to: "Sets "always_target" to true on shippable platforms when running a nightly build."

So let's fix this before landing this.

I will address the comment issue. The other thing is bug 1592162, I need to add to that so Geoff can review before this one works.

A corner case just occurred to me... I need to work on this some more.

When a release is promoted, all of those promotion tasks are going to depend on a "shippable" build. But it may or may not be a nightly build that gets promoted and with this function written like this I believe we are going to see a problem with tests. That should be limited to Seamonkey only pushes. Not likely to happen, but if it's not too hard I would like to address it.

Comment 33

[Rob Lemley [:rjl]]

Attachment: fix_extra_builds_halloween.patch

This is just a name change and comment fix. The corner case in mentioned in
comment 13 remains an issue for now. The workaround is to make sure that
any push thats to be promoted to a release is not a Seamonkey-only
code push.

Comment 34

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/d1762836541f
Add nightly attribute to shippable builds. r=darktrojan

Comment 35

[Jorg K (CEST = GMT+2)]

Last part landed on Thunderbird 72. I see 10 builds: Linux32, Linux64, Mac, Win32 and Win64 times two for debug/opt, yay \o/.

Let's file a new bug for any other issues.

Comment 36

[Jorg K (CEST = GMT+2)]

Spoke too early. Mac (and Linux32) tests aren't run.

Comment 37

[Jorg K (CEST = GMT+2)]

Attachment: mac-fix.patch

This is the original Mac bit and the current Mac bit removed:
https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=84d7e21a6662f8f182a63986d639207ad891fc78

And we can already see it doesn't work :-(

Comment 38

[Rob Lemley [:rjl]]

Attachment: bug1550611_mac_tests.patch

Should work as advertised.

Comment 39

[Jorg K (CEST = GMT+2)]

For the record: On IRC Geoff said to land the new patch. I would have only used the Linux32 part and reversed the Mac line here:
https://hg.mozilla.org/comm-central/rev/d1762836541f#l2.12

Comment 40

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/067c9af97ec9
Run macosx64/opt and linux/opt tests. r=darktrojan DONTBUILD