Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

x/build: decide status of Android TryBots #53377

Closed
bcmills opened this issue Jun 14, 2022 · 8 comments
Closed

x/build: decide status of Android TryBots #53377

bcmills opened this issue Jun 14, 2022 · 8 comments
Labels
Builders x/build issues (builders, bots, dashboards) FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. release-blocker
Milestone

Comments

@bcmills
Copy link
Contributor

bcmills commented Jun 14, 2022

The android-amd64-emu builder currently runs as a TryBot in the default set. However, android/amd64 is not a first-class port, and has some open issues that can cause test failures in TryBot runs (notably #51001, #52724, #42212).

Discussion #53060 proposes to run only first-class ports as TryBots, but I think we should reevaluate the status of android TryBot in particular independent of that discussion, because we have specific known failure modes affecting the android TryBot today.

CL 407615 would remove this TryBot for the go repo, and #25963 discusses misc-compile TryBots that could plausibly take its place.

In order to avoid TryBot failure noise when the tree reopens for Go 1.20, we should decide on the status of this TryBot (and update cmd/coordinator accordingly) before completing the Go 1.19 release cycle.

(CC @eliben @golang/android @golang/release)

@bcmills bcmills added Builders x/build issues (builders, bots, dashboards) release-blocker labels Jun 14, 2022
@bcmills bcmills added this to the Go1.19 milestone Jun 14, 2022
@bcmills bcmills added the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label Jun 14, 2022
@gopherbot
Copy link

Change https://go.dev/cl/412174 mentions this issue: dashboard: add known issues for android-*-emu

@gopherbot
Copy link

Change https://go.dev/cl/407615 mentions this issue: dashboard: exclude android-amd64-emu from main go repo's TryBot set

@dmitshur

This comment was marked as resolved.

@bcmills

This comment was marked as resolved.

gopherbot pushed a commit to golang/build that referenced this issue Jun 14, 2022
Issue golang/go#42212 manifests as test timeouts, and is by far the most
frequent of these known issues.

Issue golang/go#51001 causes failures with "systemstack called from unexpected
goroutine". It seems to have been introduced sometime last year, but
it isn't clear to me whether it is a regression or an older (latent)
bug unearthed by some other change.

Issue golang/go#52724 appears to be a bug or race in the Android emulator
itself. It might require a builder image update and/or escalation to
the maintainers of the emulator proper.

Updates golang/go#53377.

Change-Id: I677915b1ff02dd02e0f14c63b0d25caf11e27a72
Reviewed-on: https://go-review.googlesource.com/c/build/+/412174
Reviewed-by: David Chase <drchase@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Bryan Mills <bcmills@google.com>
@heschi
Copy link
Contributor

heschi commented Jun 15, 2022

We think that this should be removed from the TryBot set: it's pretty noisy and there isn't a clear path to fixing it. If nobody disagrees we'll do that.

cc @eliasnaur, who may be interested

@dmitshur
Copy link
Contributor

I think in the current state it makes sense for the android-amd64-emu builder to be post-submit only.

If I understand correctly, @cherrymui may support that too, based on this comment on CL 407615. (I'll update that CL once this issue moves to NeedsFix state.)

@changkun
Copy link
Member

Just clarify questions:
What's the exact consequence of removing a builder from TryBots?
Does it mean that this port will never be tested unless explicitly requested?
Is it going to be similar to how iOS living today? (i.e. unless try=ios, the builder will only be tested after a CL is submitted?)
Instead of removing it from TryBots will at least require doing a proper upgrade, is that correct?

@dmitshur
Copy link
Contributor

dmitshur commented Jun 16, 2022

Is it going to be similar to how iOS living today? (i.e. unless try=ios, the builder will only be tested after a CL is submitted?)

Yes, this is correct. "Removing the builder from the TryBot set" would mean it will not run automatically before CL submission when Run-TryBot+1 vote is set, but can be requested via SlowBots syntax (i.e., TRY=android or TRY=android-amd64). The android-amd64-emu builder will run after a CL is submitted (aka post-submit builds, as visible on the build dashboard), that's not changing here. This is indeed very similar to the iOS situation which doesn't have any builders in the default TryBot set, only as post-submit builders.

I think this answers other questions, but let me know if I missed something. (I didn't quite understand what "a proper upgrade" refers to in the last question.)

@cagedmantis cagedmantis added this to In Progress in Go Release Team Jun 21, 2022
@dmitshur dmitshur added NeedsFix The path to resolution is known, but the work has not been done. and removed NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. labels Jun 22, 2022
Go Release Team automation moved this from In Progress to Done Jun 22, 2022
@golang golang locked and limited conversation to collaborators Jun 22, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Builders x/build issues (builders, bots, dashboards) FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. release-blocker
Projects
Archived in project
Development

No branches or pull requests

5 participants