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/cmd/coordinator: SlowBot requests are sometimes not recognized (seemingly when re-requested on same patch set) #42084

Closed
bcmills opened this issue Oct 20, 2020 · 7 comments
Labels
Builders x/build issues (builders, bots, dashboards) FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@bcmills
Copy link
Contributor

bcmills commented Oct 20, 2020

On https://golang.org/cl/263357, I repeatedly tried to trigger a nocgo SlowBot, and repeatedly failed to do so. (See the comment log there for details.)

I had thought that any builder at all could be triggered via a TRY= line, but now I'm not so sure. At the very least, it appears that the SlowBot configuration is missing a nocgo shorthand entry.

CC @golang/release

@gopherbot gopherbot added the Builders x/build issues (builders, bots, dashboards) label Oct 20, 2020
@gopherbot gopherbot added this to the Unreleased milestone Oct 20, 2020
@cagedmantis
Copy link
Contributor

/cc @dmitshur @toothrot

@dmitshur dmitshur added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Oct 20, 2020
@dmitshur
Copy link
Contributor

I've looked over relevant code and wasn't able to spot a problem that would explain this. The linux-amd64-nocgo builder should be available as a SlowBot, like all other builders. It's possible I missed something, or maybe the problem is something subtle and rare (for example, maybe maintner corpus was behind, and the code that handles that edge-case has a problem).

From looking over CL 263357's more recent history, I see that the linux-amd64-nocgo slowbot was eventually picked up, which confirms this was an intermittent problem.

We can keep this issue open to track similar occurrences, should they happen again, and investigate further.

Adding a nocgo → linux-amd64-nocgo alias sounds reasonable. I've broken it out into #42105.

@dmitshur dmitshur changed the title x/build: no SlowBot for "linux-amd64-nocgo" x/build: a SlowBot request for a specific builder is sometimes not recognized Oct 21, 2020
@laboger
Copy link
Contributor

laboger commented Oct 29, 2020

For ppc64 and variants the experience I've had is that even if a newer TRY alias list is requested, it will sometimes run the initial list specified for the CL. An example is in the second TRY list in https://go-review.googlesource.com/c/go/+/266202.

Another problem is that if an alias is mentioned in the TRY list which doesn't match anything, there is no message telling you that.

@dmitshur dmitshur changed the title x/build: a SlowBot request for a specific builder is sometimes not recognized x/build: SlowBot requests are sometimes not recognized (seemingly when re-requested on same patch set) Oct 29, 2020
@dmitshur
Copy link
Contributor

dmitshur commented Oct 29, 2020

Some experimenting in CL 266202 suggests there's a good chance there is a problem with determining the "latest" TRY= request on the same patch set. So an (unpleasant) workaround until this issue is resolved is to make a new patch set when needing to make a different SlowBot request.

@dmitshur dmitshur changed the title x/build: SlowBot requests are sometimes not recognized (seemingly when re-requested on same patch set) x/build/cmd/coordinator: SlowBot requests are sometimes not recognized (seemingly when re-requested on same patch set) May 11, 2021
@dmitshur
Copy link
Contributor

The <= in this segment is looks suspicious:

https://github.com/golang/build/blob/58cc22bdeadbabfebe33fa4dda4b08e14b036336/maintner/maintnerd/maintapi/api.go#L142-L144

Shouldn't it be <, else later TRY= comments on the same patch set are ignored?

@dmitshur dmitshur self-assigned this Jun 3, 2021
@dmitshur
Copy link
Contributor

dmitshur commented Jun 3, 2021

I suspect the fix may be as short as described in my previous comment. CL 324763 is a recent occurrence and a good test case.

@dmitshur dmitshur added NeedsFix The path to resolution is known, but the work has not been done. and removed NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Jun 4, 2021
@gopherbot
Copy link

Change https://golang.org/cl/325229 mentions this issue: cmd/coordinator, maintner/maintnerd/maintapi: use latest TRY=foo message

@dmitshur dmitshur added this to In Progress in Go Release Team Jun 4, 2021
Go Release Team automation moved this from In Progress to Done Jun 9, 2021
@golang golang locked and limited conversation to collaborators Jun 9, 2022
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.
Projects
Archived in project
Development

No branches or pull requests

5 participants