Bugzilla

BugBot [:suhaib / :marco/ :calixte]

Comment 5

•

5 years ago

Looking into this now..

Comment 6

•

5 years ago

Bugbug thinks this bug is a regression, but please revert this change in case of error.

Keywords: regression

Comment 7

•

5 years ago

Did we see this once, on the Try push in comment 0? It seems to be an intermittent issue that affects certain revisions intermittently, see also bug 1543326.

I just did a beta simulation build on top of inbound tip and it's green: https://treeherder.mozilla.org/#/jobs?repo=try&group_state=expanded&revision=093979c5bbf2322a1e7bf41e986ebfc322a5548e

Flags: needinfo?(jdemooij)

Comment 8

•

5 years ago

Also, I think there are multiple reds in that Try push in comment 0 because they all depend on the same Bpgo(instr) and Bpgo(run) jobs. If these jobs upload corrupt profiling data, then all PGO/Nightly builds will fail when retriggered.

Comment 9

•

5 years ago

mshal, do you know if we have plans to address the issue in comment 8 somehow? (Ryan forwarded me to you..)

Flags: needinfo?(mshal)

Assignee

Comment 10

•

5 years ago

(In reply to Jan de Mooij [:jandem] from comment #9)

mshal, do you know if we have plans to address the issue in comment 8 somehow? (Ryan forwarded me to you..)

I'll try to look into moving the profdata merge into the run task, so that the run task can't upload corrupted data. But yeah, I don't believe this is an issue with your patch, it's just that the profile writing in Android in the "run" task is not robust enough, so if it uploads corrupted data then all downstream tasks will fail.

:tomprince, is there a workaround to be able to retrigger the run task and have downstream build tasks use the profile data from that run task? Retriggering the builds doesn't work in this case since they re-use the bad data from the same run task.

Flags: needinfo?(mshal) → needinfo?(mozilla)

Assignee

Updated

•

5 years ago

Assignee: nobody → mshal

Assignee

Comment 11

•

5 years ago

Attached file Bug 1560755 - Merge Android PGO profile data in run task; r?#firefox-build-system-reviewers (deleted) — Details

Android profile runs don't always fully write out the profile data. In
this case, the corrupted profile data is successfully uploaded, but
future profile-use PGO builds try to merge the data and fail. Retrying
the profile-use builds doesn't help, since they all pull from the same
job that published the corrupt data.

We can detect this in the run task by using llvm-profdata merge, and if
the merge fails the task can automatically be retried. Note that the
data gets redundantly merged in the profile-use build, but it may not be
possible to run the merge in the run task on all platforms (eg: OSX), so
we have to keep the merge there as well.

Early Beta: https://treeherder.mozilla.org/#/jobs?repo=try&selectedJob=254145065&resultStatus=success%2Ctestfailed%2Cbusted%2Cexception%2Cretry%2Cusercancel%2Crunnable&revision=24f4ab13539d9b779350817522c079779de84633&searchStr=android

Comment 12

•

5 years ago

This seems like something which could bite us on 68 as well. Tracking for both releases.

status-firefox68: unaffected → affected

tracking-firefox68: --- → +

tracking-firefox69: ? → +

Andreea Pavel [:apavel]

Reporter

Comment 13

•

5 years ago

Importing the patch makes the failures go away:

Late Beta: https://treeherder.mozilla.org/#/jobs?repo=try&resultStatus=success%2Ctestfailed%2Cbusted%2Cexception%2Cretry%2Cusercancel%2Crunnable&revision=f26f9acf755459db69c03aeff65f8499f6180077&searchStr=android&selectedJob=254145566

https://hg.mozilla.org/mozilla-central/rev/5a867f13f12e

Assignee

Updated

•

5 years ago

See Also: → https://bugzilla.mozilla.org/show_bug.cgi?id=1563053

Pulsebot

Comment 14

•

5 years ago

Pushed by mshal@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/5a867f13f12e Merge Android PGO profile data in run task; r=firefox-build-system-reviewers,chmanchester

Andrei Ciure[:aciure]

Comment 15

•

5 years ago

bugherder

Status: NEW → RESOLVED

Closed: 5 years ago

status-firefox69: affected → fixed

Resolution: --- → FIXED

Target Milestone: --- → mozilla69

Tom Prince [:tomprince]

Comment 16

•

5 years ago

(In reply to Michael Shal [:mshal] from comment #10)

:tomprince, is there a workaround to be able to retrigger the run task and have downstream build tasks use the profile data from that run task? Retriggering the builds doesn't work in this case since they re-use the bad data from the same run task.

Nothing straightforward, but if you re-run the generate-profile task, and then rerun the downstream tasks once the first one is finished, the downstream tasks should work.

Flags: needinfo?(mozilla)

Comment 17

•

5 years ago

Is this something we should consider uplifting to ESR68 since that's where we're shipping Fennec from now? If so, please request approval.

status-firefox68: affected → wontfix

status-firefox-esr68: --- → affected

tracking-firefox-esr68: --- → ?

Flags: needinfo?(mshal)

Assignee

Comment 18

•

5 years ago

Comment on attachment 9074702 [details]
Bug 1560755 - Merge Android PGO profile data in run task; r?#firefox-build-system-reviewers

ESR Uplift Approval Request

If this is not a sec:{high,crit} bug, please state case for ESR consideration: If an android "run" task fails to generate a proper PGO profile, the downstream Android build will fail to build, along with any attempts to retrigger the build task. It is not immediately obvious that the "run" task needs to be retriggered, resulting in lost sheriff and dev time investigating the task failures. This patch is intended to mitigate the issue by automatically retrying the "run" tasks that fail to generate a valid profile.
User impact if declined: No impact to end users, only impact to sheriffs and our developers. Without this patch, a release for Android may be delayed due to the extra round trip involved with sheriff interaction.
Fix Landed on Version:
Risk to taking this patch: Low
Why is the change risky/not risky? (and alternatives if risky): The change only affects the Android PGO profile generation stage of the build, and is currently running in mozilla-central.
String or UUID changes made by this patch:

Flags: needinfo?(mshal)

Attachment #9074702 - Flags: approval-mozilla-esr68?

Comment 19

•

5 years ago

Comment on attachment 9074702 [details]
Bug 1560755 - Merge Android PGO profile data in run task; r?#firefox-build-system-reviewers

Reliability improvement for Fennec PGO runs. Approved for 68.1esr.

Attachment #9074702 - Flags: approval-mozilla-esr68? → approval-mozilla-esr68+

Updated

•

5 years ago

tracking-firefox-esr68: ? → 69+

Comment 20

•

5 years ago

Looks like this needs a rebased patch for ESR68 (or has another dep needing uplift).

Flags: needinfo?(mshal)

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

Assignee

Comment 21

•

5 years ago

Looks like the spacing got messed up somehow in the yml file when importing the patch - "toolchains" should be at the same level as "run", not inside "run". This patch on top of the commit in esr68 works (or at least, the decision task completes) -

diff --git a/taskcluster/ci/generate-profile/kind.yml b/taskcluster/ci/generate-profile/kind.yml
index 4e9c296bb2f68..dbcd7826ed82d 100644
--- a/taskcluster/ci/generate-profile/kind.yml
+++ b/taskcluster/ci/generate-profile/kind.yml
@@ -91,5 +91,5 @@ jobs:
                 - android/android_common.py
                 - android/androidarm_4_3.py
                 - android/android_pgo.py
-            toolchains:
-                - linux64-clang
+        toolchains:
+            - linux64-clang

Flags: needinfo?(mshal) → needinfo?(ryanvm)

Comment 22

•

5 years ago

bugherder uplift

https://hg.mozilla.org/releases/mozilla-esr68/rev/aae0c4405b05

status-firefox-esr68: affected → fixed