Navigation Menu

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

cmd/dist: remove arch-specific timeout scale heuristics; handle these in x/build as needed #57117

Closed
dmitshur opened this issue Dec 6, 2022 · 2 comments
Assignees
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@dmitshur
Copy link
Contributor

dmitshur commented Dec 6, 2022

Discussion in #56968 seems to suggest we have agreement to get cmd/dist out of the practice of maintaining a hardcoded timeout scale for a few architectures. Instead, dist users can set it for their own machine (if at all needed) and we can factor out this configuration into x/build/dashboard for builders.

That is, we should remove these lines from cmd/dist:

switch goarch {
case "arm":
	t.timeoutScale = 2
case "mips", "mipsle", "mips64", "mips64le":
	t.timeoutScale = 4
}

And adjust x/build as needed to avoid breaking any builders that might rely on that.

Breaking this off into this issue for tracking purposes.

CC @aclements, @bcmills.

@dmitshur dmitshur added the NeedsFix The path to resolution is known, but the work has not been done. label Dec 6, 2022
@dmitshur dmitshur added this to the Go1.21 milestone Dec 6, 2022
@dmitshur dmitshur self-assigned this Dec 6, 2022
@gopherbot
Copy link

Change https://go.dev/cl/455518 mentions this issue: cmd/dist: remove hardcoded timeout scale for arm and mips{,le,64,64le}

@gopherbot
Copy link

Change https://go.dev/cl/455521 mentions this issue: dashboard: apply cmd/dist's timeout scale for arm, mips* architectures

gopherbot pushed a commit to golang/build that referenced this issue Dec 6, 2022
These values come from cmd/dist doing it by default based on GOARCH.
We want to factor out that logic from cmd/dist, so apply it explicitly
in x/build to appropriate builders. CL 455518 removes it from cmd/dist.

The appropriate builders were found by iterating over all builders in
this package and filtering on their GOARCH value.

For golang/go#56968.
For golang/go#57117.

Change-Id: I1bccb7144d9ae13ca17e5f12169924d0fb89e341
Reviewed-on: https://go-review.googlesource.com/c/build/+/455521
Auto-Submit: Dmitri Shuralyov <dmitshur@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Reviewed-by: Bryan Mills <bcmills@google.com>
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
@golang golang locked and limited conversation to collaborators Jan 20, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
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

2 participants