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.

Sign up for GitHub

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

Jump to bottom

x/build/cmd/coordinator: failed to add requested SlowBot #46106

Open
bcmills opened this issue May 11, 2021 · 4 comments
Open

x/build/cmd/coordinator: failed to add requested SlowBot #46106

bcmills opened this issue May 11, 2021 · 4 comments
Labels
Builders x/build issues (builders, bots, dashboards) NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone
Unreleased

Comments

@bcmills
Copy link
Contributor

bcmills commented May 11, 2021

In https://go-review.googlesource.com/c/go/+/318570/1#message-c05120c76c77004b072d84595dbb73a0392f90a4, I added a TRY=longtest comment and Run-TryBot+1 to a CL.

I had previously removed a Run-TryBot+1 and a TryBot-Result for the same patch set after the TryBot run had flaked.

Unfortunately, the coordinator appears not to have added the requested SlowBot. It replied with “TryBots beginning”, and the list of builders did not include any longtest builders:
image

CC @golang/release @FiloSottile
@bcmills bcmills added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label May 11, 2021
@gopherbot gopherbot added the Builders x/build issues (builders, bots, dashboards) label May 11, 2021
@gopherbot gopherbot added this to the Unreleased milestone May 11, 2021
@bcmills
Copy link
Contributor Author

bcmills commented May 11, 2021

I removed the +1's and added another TRY=longtest comment and it seems to have picked it up now (https://go-review.googlesource.com/c/go/+/318570/1#message-1c0e6fb5c05da64da7aecd49fc36339c51f54907). I don't know why one comment was ignored while another was accepted.

@dmitshur
Copy link
Contributor

This looks likely to be the same as #42084. At least the investigation there could explain this instance too. Should we close this in favor of that issue or keep it open?

@dmitshur
Copy link
Contributor

Actually, I think there are at least 2 underlying problems. Let's keep #42084 about it ignoring re-requests on same patch set with a different TRY= message, which might be a simple logic fix.

One possible explanation for the exact problem observed here is a data inconsistency race between the votes and comments. In goFindTryWork, perhaps it's possible the "label:Run-TryBot=1 label:TryBot-Result=0 status:open" Gerrit search query returns the CL where a Run-TryBot vote was made sooner than the ListChangeComments starts to return the comment that was left. I hope it's something else, but until proven otherwise this remains a possibility.

Issue #20806 is related here, in that if we can get rid of Gerrit API as a data source, we don't need to worry about its consistency guarantees. However, maintner currently has its own consistency problem to worry about...

gopherbot pushed a commit to golang/build that referenced this issue May 9, 2023
@dmitshur @gopherbot
maintner/maintnerd/maintapi: use inline comment posted on older patchset
84feada 
A valid TRY= request can end up being left in a patchset-level comment
associated with an older patch set than the patch set that the message
with a Run-TryBot+1 vote is posted on. Fix the oversight of dismissing
such SlowBot requests.

For golang/go#46106.

Change-Id: If3ff0bbdddca5bffa5b4279755765eca2fcb0cca
Reviewed-on: https://go-review.googlesource.com/c/build/+/493855
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
Auto-Submit: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/493855 mentions this issue: maintner/maintnerd/maintapi: use inline comment posted on older patchset

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Builders x/build issues (builders, bots, dashboards) NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Milestone
Unreleased
Development

No branches or pull requests
3 participants
@dmitshur @bcmills @gopherbot