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

Jump to bottom

x/build/cmd/coordinator: triggering slowbots is somewhat confusing #59911

Open
rolandshoemaker opened this issue May 1, 2023 · 1 comment
Open

x/build/cmd/coordinator: triggering slowbots is somewhat confusing #59911

rolandshoemaker opened this issue May 1, 2023 · 1 comment
Labels
Builders x/build issues (builders, bots, dashboards) NeedsFix The path to resolution is known, but the work has not been done.
Milestone
Unreleased

Comments

@rolandshoemaker
Copy link
Member

Currently, the TRY= comment needs to happen at the same time as the Run-TryBot+1 vote, otherwise it is silently ignored, which can be incredibly confusing (at least to me), and requires a not insignificant number of actions to resolve (deleting votes, re-commenting and voting, etc).

It would seem somewhat preferable to simply use the last TRY= comment on the change to determine slowbots (perhaps this requires retrieving more information from gerrit than the coordinator currently has?). Reseting the selected slowbows would require an empty TRY= if you wanted to disable them, but overall that seems less confusing (at least to me) to the current behavior.

Given the rapid approach of LUCI, this may not be particularly important, but it may help trigger something in the back on my mind next time I forget how to do this.

cc @golang/release @bcmills
@rolandshoemaker rolandshoemaker added the Builders x/build issues (builders, bots, dashboards) label May 1, 2023
@gopherbot gopherbot added this to the Unreleased milestone May 1, 2023
@dmitshur
Copy link
Contributor

dmitshur commented May 1, 2023 •
edited

It would seem somewhat preferable to simply use the last TRY= comment on the change to determine slowbots (perhaps this requires retrieving more information from gerrit than the coordinator currently has?).

Yeah. The current suboptimal behavior is due to an implementation shortcut, see second sentence in the relevant comment. It was meant to be improved at some point so that "TRY= comments are ignored if they're not on the same comment that started the TryBots" could be deleted from https://go.dev/wiki/SlowBots#pitfalls.

At this point, as you said, LUCI's implementation of slowbots via a UI will likely supersede the TRY= comments and resolve this particular pitfall.

@dmitshur dmitshur added the NeedsFix The path to resolution is known, but the work has not been done. label May 1, 2023
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) NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Milestone
Unreleased
Development

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