1334917 - Stop double-running PGO on aurora and below, where the normal build is PGO without asking for PGO

Status: RESOLVED WORKSFORME

(Release Engineering :: General, defect)

Description

[Phil Ringnalda (:philor)]

Because reasons, mozharness forces every build of platforms which are capable of doing PGO to be PGO on aurora/beta/release/esr, in https://dxr.mozilla.org/mozilla-central/source/testing/mozharness/configs/builds/branch_specifics.py#253

For the taskcluster budget, that means you are spending the money to do four builds instead of two, linux32-which-is-PGO and linux32-PGO and linux64-which-is-PGO and linux64-PGO, and three sets of tests instead of two (since we didn't bother with linux32-PGO tests), but because both linux64 builds trigger talos which runs on a far too small pool of hardware, this is contributing to having 12 hour backlogs of talos tests on try and 3 hour backlogs of talos tests on integration branches every weekday, thus the severity.

Comment 1

[Phil Ringnalda (:philor)]

And yes, dropping the PGO build/tests and relying on just the unlabelled PGO build/tests is likely to cause confusion, but then, we've had builds-not-labelled-PGO and tests-labelled-PGO for as long as we've had the whole idea of periodic PGO on trunk and on-push PGO on release branches, so it's just new confusion added to existing confusion.

Comment 2

[Kim Moir [:kmoir] ET]

Alin or Andrei, could you investigate a solution for this issue?

Comment 3

[Alin Selagea [:aselagea]]

Trying to put things together a bit here (with respect to the release-stabilization branches):

1. We're indeed doing PGO builds for both linux32/64 and linux32/64-pgo on these branches. Taking a look at Treeherder, a PGO build generally needs twice the time a normal build would require, so that results in a waste of resources. 
For the tests running, we have the following:
    - mozilla-aurora: 
        - linux32: all tests running in TC
        - linux32-pgo: no tests
        - linux64: all tests running in TC
        - linux64-pgo: all tests running in TC
    - mozilla-beta:
        - linux32: no tests
        - linux32-pgo: BB-only tests [*]
        - linux64: all tests running in TC
        - linux64-pgo:  BB + TC tests [*]
    - mozilla-release:
        - linux32: no tests
        - linux32-pgo: disabled 
        - linux64: all tests running in TC
        - linux64-pgo: all tests running in TC
    - mozilla-esr45:
        - linux32: no tests
        - linux32-pgo: BB-only tests
        - linux64: no tests
        - linux64-pgo: BB-only tests

2. As for the talos tests, they run the following way (on linux):
    - mozilla-aurora: on *both* linux64-opt and linux64-pgo => 18 talos tests/push
    - mozilla-beta: on linux64-pgo *only* => 9 talos tests/push
    - mozilla-release => no talos tests
    - mozilla-esr45 => no talos tests

One solution would be removing linux64-pgo tests on these branches - they already run in TC for the linux64 build (which is also PGO), so it makes sense to avoid running them twice. That will also save 9 talos tests/push on aurora and beta. I haven't considered esr45 here (where BB tests are still running).

If we want to also disable linux32-pgo tests, that will result in no tests for beta (once it reaches 53). Would that be an issue? 

To end with, I'm not sure if it's worth disabling linux32/64-pgo builds for now, noticed they do run on development branches so maybe it wouldn't harm to let them as they are. 
More opinions are welcome here though. 

* BB builds&tests will be disabled once mozilla-beta will reach Firefox 53.
https://dxr.mozilla.org/build-central/source/buildbot-configs/mozilla-tests/config.py#2982
https://dxr.mozilla.org/build-central/source/buildbot-configs/mozilla/config.py#2699

Comment 4

[Kim Moir [:kmoir] ET]

Coop would it be okay to disable the duplicate linux64-pgo tests to alleviate the backlog until we have the SETA issues resolved as described here?

https://bugzilla.mozilla.org/show_bug.cgi?id=1334929#c8

Comment 5

[Chris Cooper [:coop] (he/him)]

(In reply to Kim Moir [:kmoir] from comment #4)
> Coop would it be okay to disable the duplicate linux64-pgo tests to
> alleviate the backlog until we have the SETA issues resolved as described
> here?
> 
> https://bugzilla.mozilla.org/show_bug.cgi?id=1334929#c8

Yes, absolutely. The sooner, the better.

Comment 6

[Chris Cooper [:coop] (he/him)]

(In reply to Phil Ringnalda (:philor) from comment #1)
> And yes, dropping the PGO build/tests and relying on just the unlabelled PGO
> build/tests is likely to cause confusion, but then, we've had
> builds-not-labelled-PGO and tests-labelled-PGO for as long as we've had the
> whole idea of periodic PGO on trunk and on-push PGO on release branches, so
> it's just new confusion added to existing confusion.

Ideally we should drop the regular builds/tests that are PGO but aren't billed as such.

Perhaps we could re-use an existing flag in the bb-configs like 'pgo_strategy' to indicate whether we should bother adding non-PGO default builds? For branches from aurora and downstream, we could check for 'per-checkin' and not add the basic opt build in those branches.

Comment 7

[Kim Moir [:kmoir] ET]

Attachment: npgo.patch

not sure about this patch, it would in theory limit non-pgo builds to non-integraion branches

http://gecko.readthedocs.io/en/latest/taskcluster/taskcluster/attributes.html#run-on-projects

Comment 8

[Kim Moir [:kmoir] ET]

aselagea: you asked about this earlier today, I think we just need to do what coop indicated in comment #6

>> Ideally we should drop the regular builds/tests that are PGO but aren't billed as such.

Comment 9

[Phil Ringnalda (:philor)]

That's not a "just" option, since it means:

* turn on Linux32 PGO Taskcluster tests on Try
* turn on Linux32 PGO Taskcluster tests on trunk
* turn on Linux32 PGO Taskcluster tests on aurora and let them ride the trains
* teach taskcluster not to do Linux32 "opt" builds (and thus not do tests) on release branches

which is maybe slightly less daunting than the original task of just doing tests on Linux32 PGO Taskcluster builds would have been, since accidentally doing them on aurora has shown us that they'll run just fine, but it still has to be done.

Comment 10

[Kim Moir [:kmoir] ET]

Rail, I forgot to ask you about this last week. Coop has suggested in comment #6 

>>Ideally we should drop the regular builds/tests that are PGO but aren't billed as such.

He mentioned that I should run this by someone more familiar with releases if this would be a problem, do you have any concerns? I believe he mentioned this since a different binary would be promoted.

Comment 11

[Rail Aliiev [:rail]]

relpro uses the opt variant, but it won't be hard to change that behaviour. https://hg.mozilla.org/build/tools/file/tip/lib/python/kickoff/__init__.py#l161 is where we define those. Probably we would need to move those somewhere it can ride trains. Could be bb-configs (eeew) or some in-tree config. I filed bug 1339151 to make this possible (required in any case).

Comment 12

[Chris AtLee [:catlee]]

See also bugs 700924 and 806787 where the request was to make the buildername on aurora/beta/release indicate that these were in fact PGO builds.

Comment 13

[Kim Moir [:kmoir] ET]

aselagea you should investigate the work to implement the what is described in comment 6 given the caveats in comment 9

Comment 14

[Chris AtLee [:catlee]]

Are we still doing this?

Comment 15

[Phil Ringnalda (:philor)]

Yes.

Comment 16

[Kim Moir [:kmoir] ET]

So I talked to aki re his shippable build proposal
https://groups.google.com/forum/#!topic/mozilla.release.engineering/MRfHnhaVI48

and it doesn't conflict with disabling opt builds here.   

So we need to find a solution, Alin can you describe the where you got stuck on the bug, I'll investigate as well.  Do you have any work in progress patches?

Comment 17

[Alin Selagea [:aselagea]]

I'm sorry about this, but it's still not clear to me what we really need here :|

(In reply to Chris Cooper [:coop] from comment #6)

> Ideally we should drop the regular builds/tests that are PGO but aren't
> billed as such.
That would make me think we want to drop the opt-labelled builds/tests (which are pgo in reality).

Phil mentioned in comment 9 that we would need to enable Linux32 pgo TC tests on trunk+try+aurora and also stop running Linux32 opt TC builds for the release branches.

Yet https://bugzilla.mozilla.org/show_bug.cgi?id=1352113#c0 mentions that we would need to drop 'pgo' builds as well.

Am I missing something here?

Comment 18

[Kim Moir [:kmoir] ET]

I apologize for the confusion.

We don't have anyone assigned to implementing bug 1352113 yet (shippable builds).  We are still working on migrating taskcluster nightlies on platforms other than Android and Linux which is a requirement for shippable builds.

From the conversation in #tcmigration yesterday
kmoir> Kim Moir ak.i: with your shippable build proposal, at what cadence will these shippable builds run? Do we have agreement that this proposal will go forward? The reason I'm asking is that buildduty has a bug to enable more PGO builds and disable the associated opt builds https://bugzilla.mozilla.org/show_bug.cgi?id=1334917 and don't want to create work that will be
11:23 AM undone
11:24 AM 
<aki-away> kmoir: yes, the shippable builds would essentially make pgo builds obsolete
11:25 AM 
<catlee> in the meanwhile, should we stop running the "PGO" builds on aurora/beta/release?
11:25 AM 
<Callek> aki-away: welllll, nothing says that shippable builds mean that shippable builds will happen on every CI push
11:25 AM 
<catlee> since the opt builds are already PGO right now
11:25 AM 
<Callek> (for all trees)
11:25 AM 
<catlee> Callek: they should be for release trees
11:27 AM 
<kmoir> Kim Moir yeah, I guess the proposal doesn't negate the work needed in that bug. I'll go see again how it can be implemented to help it get unstuck
11:27 AM 
<aki-away> my current proposal is 4hr shippables on integration, backfillable, and per push on m-c through m-r and esr

Aki, would it make more sense to disable the pgo build on aurora/beta/release/esr and leave the opt ones which are really pgo?  As apposed to adding more pgo builds which will go away when shippable builds are impelemented?

Comment 19

[Aki Sasaki (not active)]

(In reply to Kim Moir [:kmoir] from comment #18)
> Aki, would it make more sense to disable the pgo build on
> aurora/beta/release/esr and leave the opt ones which are really pgo?

+1

> As
> apposed to adding more pgo builds which will go away when shippable builds
> are impelemented?

I think we can skip these for now.  To be clear, that means we'll be leaving things opt-as-pgo until shippable builds, which we plan on working after tc nightlies are migrated.  I'm ok with that.

Comment 20

[Alin Selagea [:aselagea]]

Okay, thank you both for looking into this. 

I'd have one more question regarding the way we should drop the TaskCluster pgo builds and tests for mozilla-aurora and below.
    - [mozilla-beta] - in bug 1344321 we removed all pgo builds and tests from m-b. That was done using a method like target_tasks_mozilla_beta() to filter the pgo entries [1]

    - [mozilla-aurora] - we don't have such a method for m-a, so the default filter is used in this case [2].
That means we should probably either:
        - write a similar method for aurora
        - don't mind it since aurora is going away soon

    - [mozilla-release] - uses the default task filter [3], but we already have a specific filter for it defined in m-b and above which should limit the builds and test to be the same as on m-b [4]. So if my understanding is correct, we should get rid of the pgo builds and tests on mozilla-release when we do the next uplift.

    - [mozilla-esr52] - used the default task filter [5], I think a specific method is needed here to filter the pgo tasks

    - [mozilla-esr45] - no TC pgo tasks

So my guess here would be that we need to:
    - write a patch for esr52 to disable pgo
    - write a patch for mozilla-release to include the method defined in m-b [4]. That would run the same tasks on release and beta and thus, drop pgo.

I apologize for the long comment. Any thoughts?

[1] https://dxr.mozilla.org/mozilla-beta/source/taskcluster/taskgraph/target_tasks.py#149
[2] e.g. https://public-artifacts.taskcluster.net/XHl6RhWjSzWlgk0kjOVDIg/0/public/parameters.yml
[3] e.g. https://public-artifacts.taskcluster.net/fbvwyyZCS428Mlc6bjZrBA/0/public/parameters.yml
[4] https://dxr.mozilla.org/mozilla-beta/source/taskcluster/taskgraph/target_tasks.py#178
[5] https://public-artifacts.taskcluster.net/RP86tS-yTjaxmf8zhaIeRw/0/public/parameters.yml

Comment 21

[Aki Sasaki (not active)]

Defining run-on-projects for pgo builds might make sense.  Really, what we want is `all` minus `release`, which might be tough.

Maybe here [1] we could add a check for 'non-release', e.g.

    if 'non-release' in run_on_projects:
        if project not in RELEASE_PROJECTS:
            return True

and we'd be able to set pgo run-on-projects to `non-release`.

[1] https://hg.mozilla.org/integration/mozilla-inbound/file/tip/taskcluster/taskgraph/util/attributes.py#l74

Comment 22

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

why are we running opt builds/tests/talos on aurora/beta/release ?  historically we have just done PGO on those branches.

Comment 23

[Aki Sasaki (not active)]

(In reply to Joel Maher ( :jmaher) from comment #22)
> why are we running opt builds/tests/talos on aurora/beta/release ? 
> historically we have just done PGO on those branches.

Answered in IRC: essentially, opt is PGO, so we're running it twice.

Comment 24

[Alin Selagea [:aselagea]]

(In reply to Aki Sasaki [:aki] from comment #21)

> Maybe here [1] we could add a check for 'non-release', e.g.
> 
>     if 'non-release' in run_on_projects:
>         if project not in RELEASE_PROJECTS:
>             return True

I've tried doing something similar on my local copy of m-c repo.
As stated here, I created the check for the non-release branches and then added a 'run-on-projects' parameter in the definition of the linux/macosx/windows pgo builders. [1]

The parameters.yml file used was one from a m-a push [2]

I'm not sure why the builders are not removed from aurora since it uses the default filter (which in turn relies on the filter_for_project filter) [3]

I'm not sure why, 
[1] https://dxr.mozilla.org/mozilla-central/source/taskcluster/ci/build
[2] https://public-artifacts.taskcluster.net/KgOGSKH9RMqoEL7eULM9OQ/0/public/parameters.yml
[3] https://dxr.mozilla.org/mozilla-central/source/taskcluster/taskgraph/target_tasks.py#30

Comment 25

[Alin Selagea [:aselagea]]

Well, ignore the second 'uncertainty' statement ^

Comment 26

[Chris AtLee [:catlee]]

I'm confused about the current state of this bug. Now that aurora is gone, is there anything left to do here?

Comment 27

[Phil Ringnalda (:philor)]

It's confusing because somebody fixed it in some other bug in the most sideways way possible, by running Linux "nightly" builds (despite the fact that we don't do nightlies on either branch) on every push to mozilla-beta and mozilla-release, and not running any other builds and thus not running any other tests.

So on Linux we build (PGO) nightly on every push despite not having nightlies, on Mac we build opt (because Mac doesn't do PGO), and on Windows we build PGO labelled as not being PGO with tests that are labelled as being PGO.