Skip to content

Commit

Permalink
dashboard: include at most 3 ports per misc-compile TryBot
Browse files Browse the repository at this point in the history 
Between Go 1.15, 1.16 and tip, the following 29 ports don't have
a real TryBot and instead rely on misc-compile TryBots for their
pre-submit coverage:

	• aix/ppc64
	• darwin/amd64
	• darwin/arm64
	• dragonfly/amd64
	• freebsd/386
	• freebsd/arm
	• freebsd/arm64
	• illumos/amd64
	• linux/mips
	• linux/mips64
	• linux/mipsle
	• linux/mips64le
	• linux/ppc64
	• linux/ppc64le
	• linux/riscv64
	• linux/s390x
	• netbsd/386
	• netbsd/amd64
	• netbsd/arm
	• netbsd/arm64
	• openbsd/386
	• openbsd/arm
	• openbsd/arm64
	• openbsd/mips64
	• plan9/386
	• plan9/amd64
	• plan9/arm
	• solaris/amd64
	• windows/arm

The previous approach for misc-compile target selection was to break
them up primarily by GOOS value. However, as new architectures were
added over time, some misc-compile TryBots got to a point where they
were testing upwards of 5 ports (for example, misc-compile-openbsd
was testing 386, amd64, arm, arm64, and mips64 architectures).

Since each port is tested sequentially, allocating too many to one
misc-compile TryBot can cause it to become the bottleneck of an
entire TryBot run, exceeding the 10 minute completion time goal.

Arrange it so misc-compile TryBot target selection is done explicitly
in x/build, and pick 3 as max number of targets per TryBot for now.
Based on recent timing observations, that should strike a decent balance
between resource use (spinning up a builder) vs chance of a misc-compile
TryBot becoming a bottleneck. It will also give us an opportunity to
compare timing of 1, 2 and 3 targets per misc-compile in the future.
(When we start tracking timing for TryBot completion time holistically,
we'll be in a better position to refine this strategy further.)

Making misc-compile target selection explicit in x/build also enables
removing unnecessary duplicate misc-compile coverage from ports that
already have a real TryBot (for example, openbsd/amd64 was previously
tested via both the openbsd-amd64-68 TryBot and misc-compile-openbsd).
This shouldn't be needed, so it's no longer done.

For golang/go#17104.
Fixes golang/go#32632.

Change-Id: Iac918377b91af3e48780b38ffdf3153e213eeba2
Reviewed-on: https://go-review.googlesource.com/c/build/+/313210
Trust: Dmitri Shuralyov <[email protected]>
Run-TryBot: Dmitri Shuralyov <[email protected]>
TryBot-Result: Go Bot <[email protected]>
Reviewed-by: Heschi Kreinick <[email protected]>
Reviewed-by: Carlos Amedee <[email protected]>
  • Loading branch information
@dmitshur
dmitshur committed Apr 26, 2021
1 parent 2e5c6aa commit 646d48d
Show file tree
Hide file tree
Showing 2 changed files with 111 additions and 53 deletions.
62 changes: 39 additions & 23 deletions dashboard/builders.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1601,13 +1601,17 @@ func init() {
})
}

// addMiscCompileGo1 adds a misc-compile TryBot that runs
// buildall.bash on a subset of platforms matching the egrep
// pattern rx. The pattern is matched against the "go tool
// dist list" name, but with hyphens instead of forward
// slashes ("linux-amd64", etc).
// addMiscCompileGo1 adds a misc-compile TryBot that
// runs buildall.bash on the specified target(s), up to 3 max.
// The targets are matched against the "go tool dist list" name,
// but with hyphens instead of forward slashes ("linux-amd64", etc).
// If min is non-zero, it specifies the minimum Go 1.x version.
addMiscCompileGo1 := func(min int, suffix, rx string) {
addMiscCompileGo1 := func(min int, suffix string, targets ...string) {
if len(targets) > 3 {
// This limit will do until we have better visibility
// into holistic TryBot completion times via metrics.
panic("at most 3 targets may be specified to avoid making TryBots slow; see issues 32632 and 17104")
}
var v types.MajorMinor
var alsoNote string
if min != 0 {
Expand All @@ -1624,30 +1628,42 @@ func init() {
tryOnly: true,
MinimumGoVersion: v,
CompileOnly: true,
Notes: "Runs buildall.bash to cross-compile & vet std+cmd packages for " + rx + ", but doesn't run any tests." + alsoNote,
Notes: "Runs buildall.bash to cross-compile & vet std+cmd packages for " + strings.Join(targets, " & ") + ", but doesn't run any tests." + alsoNote,
allScriptArgs: []string{
// Filtering pattern to buildall.bash:
rx,
"^(" + strings.Join(targets, "|") + ")$",
},
})
}
// addMiscCompile adds a misc-compile TryBot
// for all supported Go versions.
addMiscCompile := func(suffix, rx string) { addMiscCompileGo1(0, suffix, rx) }

addMiscCompile("-linuxarm", "^linux-arm-arm5$") // 1: linux/arm with GOARM=5
addMiscCompile("-darwin", "^darwin-(386|amd64)$") // 1: amd64
addMiscCompileGo1(16, "-darwinarm64", "^darwin-arm64$") // 1: arm64 (for Go 1.16 and newer)
addMiscCompile("-mips", "^linux-mips") // 4: mips, mipsle, mips64, mips64le
addMiscCompile("-ppc", "^(linux-ppc64|aix-)") // 3: linux-ppc64{,le}, aix-ppc64
addMiscCompile("-solaris", "^(solaris|illumos)") // 2: both amd64
addMiscCompile("-plan9", "^plan9-") // 3: amd64, 386, arm
addMiscCompile("-freebsd", `^freebsd-(386|arm|arm64)\b`) // 3: 386, arm, arm64 (amd64 already trybot)
addMiscCompile("-netbsd", "^netbsd-") // 4: amd64, 386, arm, arm64
addMiscCompile("-openbsd", "^openbsd-") // 4: amd64, 386, arm, arm64

// And 4 that don't fit above:
addMiscCompile("-other", "^(windows-arm|linux-s390x|linux-riscv64|dragonfly-amd64)$")
addMiscCompile := func(suffix string, targets ...string) { addMiscCompileGo1(0, suffix, targets...) }

// Arrange so that no more than 3 ports are tested sequentially in each misc-compile
// TryBot to avoid any individual misc-compile TryBot from becoming a bottleneck for
// overall TryBot completion time (currently 10 minutes; see golang.org/issue/17104).
//
// The TestTryBotsCompileAllPorts test is used to detect any gaps in TryBot coverage
// when new ports are added, and the misc-compile pairs below can be re-arranged.
//
// (In the past, we used flexible regexp patterns that matched all architectures
// for a given GOOS value. However, over time as new architectures were added,
// some misc-compile TryBot could become much slower than others.)
//
// See golang.org/issue/32632.
addMiscCompile("-mac-win", "darwin-amd64", "windows-arm")
addMiscCompileGo1(16, "-darwinarm64", "darwin-arm64") // darwin/arm64 (for Go 1.16 and newer) only.
addMiscCompile("-mips", "linux-mips", "linux-mips64")
addMiscCompile("-mipsle", "linux-mipsle", "linux-mips64le")
addMiscCompile("-ppc", "linux-ppc64", "linux-ppc64le", "aix-ppc64")
addMiscCompile("-freebsd", "freebsd-386", "freebsd-arm", "freebsd-arm64")
addMiscCompile("-netbsd", "netbsd-386", "netbsd-amd64")
addMiscCompile("-netbsd-arm", "netbsd-arm", "netbsd-arm64")
addMiscCompile("-openbsd", "openbsd-386", "openbsd-mips64")
addMiscCompile("-openbsd-arm", "openbsd-arm", "openbsd-arm64")
addMiscCompile("-plan9", "plan9-386", "plan9-amd64", "plan9-arm")
addMiscCompile("-other-1", "solaris-amd64", "illumos-amd64", "dragonfly-amd64")
addMiscCompile("-other-2", "linux-riscv64", "linux-s390x", "linux-arm-arm5") // 'linux-arm-arm5' is linux/arm with GOARM=5.

// TODO: Issue 25963, get the misc-compile trybots for Android/iOS.
// Then consider subrepos too, so "mobile" can at least be included
Expand Down
102 changes: 72 additions & 30 deletions dashboard/builders_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -99,20 +99,23 @@ func TestTrybots(t *testing.T) {
"linux-amd64-race",
"linux-arm-aws",
"linux-arm64-aws",
"misc-compile-darwin",
"openbsd-amd64-68",
"windows-386-2008",
"windows-amd64-2016",

"misc-compile-darwinarm64",
"misc-compile-freebsd",
"misc-compile-linuxarm",
"misc-compile-mac-win",
"misc-compile-mips",
"misc-compile-mipsle",
"misc-compile-netbsd",
"misc-compile-netbsd-arm",
"misc-compile-openbsd",
"misc-compile-other",
"misc-compile-openbsd-arm",
"misc-compile-plan9",
"misc-compile-ppc",
"misc-compile-solaris",
"openbsd-amd64-68",
"windows-386-2008",
"windows-amd64-2016",
"misc-compile-other-1",
"misc-compile-other-2",
},
},
{
Expand All @@ -127,20 +130,23 @@ func TestTrybots(t *testing.T) {
"linux-amd64-race",
"linux-arm-aws",
"linux-arm64-aws",
"misc-compile-darwin",
"openbsd-amd64-68",
"windows-386-2008",
"windows-amd64-2016",

"misc-compile-darwinarm64",
"misc-compile-freebsd",
"misc-compile-linuxarm",
"misc-compile-mac-win",
"misc-compile-mips",
"misc-compile-mipsle",
"misc-compile-netbsd",
"misc-compile-netbsd-arm",
"misc-compile-openbsd",
"misc-compile-other",
"misc-compile-openbsd-arm",
"misc-compile-plan9",
"misc-compile-ppc",
"misc-compile-solaris",
"openbsd-amd64-68",
"windows-386-2008",
"windows-amd64-2016",
"misc-compile-other-1",
"misc-compile-other-2",
},
},
{
Expand All @@ -155,20 +161,23 @@ func TestTrybots(t *testing.T) {
"linux-amd64-race",
"linux-arm-aws",
"linux-arm64-aws",
"misc-compile-darwin",
"openbsd-amd64-68",
"windows-386-2008",
"windows-amd64-2016",

"misc-compile-darwinarm64", // Starts with Go 1.16.
"misc-compile-freebsd",
"misc-compile-linuxarm",
"misc-compile-mac-win",
"misc-compile-mips",
"misc-compile-mipsle",
"misc-compile-netbsd",
"misc-compile-netbsd-arm",
"misc-compile-openbsd",
"misc-compile-other",
"misc-compile-openbsd-arm",
"misc-compile-plan9",
"misc-compile-ppc",
"misc-compile-solaris",
"openbsd-amd64-68",
"windows-386-2008",
"windows-amd64-2016",
"misc-compile-other-1",
"misc-compile-other-2",

// Include longtest builders on Go repo release branches. See issue 37827.
"linux-386-longtest",
Expand All @@ -188,19 +197,22 @@ func TestTrybots(t *testing.T) {
"linux-amd64-race",
"linux-arm-aws",
"linux-arm64-aws",
"misc-compile-darwin",
"openbsd-amd64-68",
"windows-386-2008",
"windows-amd64-2016",

"misc-compile-freebsd",
"misc-compile-linuxarm",
"misc-compile-mac-win",
"misc-compile-mips",
"misc-compile-mipsle",
"misc-compile-netbsd",
"misc-compile-netbsd-arm",
"misc-compile-openbsd",
"misc-compile-other",
"misc-compile-openbsd-arm",
"misc-compile-plan9",
"misc-compile-ppc",
"misc-compile-solaris",
"openbsd-amd64-68",
"windows-386-2008",
"windows-amd64-2016",
"misc-compile-other-1",
"misc-compile-other-2",

// Include longtest builders on Go repo release branches. See issue 37827.
"linux-386-longtest",
Expand Down Expand Up @@ -817,8 +829,10 @@ func TestCrossCompileConfigs(t *testing.T) {
}
}

// TestTryBotsCompileAllPorts verifies that each port (go tool dist list) is covered by
// either a real trybot or a misc-compile trybot.
// TestTryBotsCompileAllPorts verifies that each port (go tool dist list)
// is covered by either a real TryBot or a misc-compile TryBot.
//
// The special pseudo-port 'linux-arm-arm5' is tested in TestMiscCompileLinuxGOARM5.
func TestTryBotsCompileAllPorts(t *testing.T) {
out, err := exec.Command(filepath.Join(runtime.GOROOT(), "bin", "go"), "tool", "dist", "list").Output()
if err != nil {
Expand Down Expand Up @@ -892,6 +906,34 @@ func TestTryBotsCompileAllPorts(t *testing.T) {
}
}

// The 'linux-arm-arm5' pseduo-port is supported by src/buildall.bash
// and tests linux/arm with GOARM=5 set. Since it's not a normal port,
// the TestTryBotsCompileAllPorts wouldn't report if the misc-compile
// TryBot that covers is is accidentally removed. Check it explicitly.
func TestMiscCompileLinuxGOARM5(t *testing.T) {
var ok bool
for _, b := range Builders {
if !strings.HasPrefix(b.Name, "misc-compile-") {
continue
}
re, err := regexp.Compile(b.allScriptArgs[0])
if err != nil {
t.Fatalf("invalid misc-compile filtering pattern for builder %q: %q",
b.Name, b.allScriptArgs[0])
}
if re.MatchString("linux-arm-arm5") {
ok = true
break
}
}
if !ok {
// We get here if the linux-arm-arm5 port is no longer checked by
// a misc-compile TryBot. Report it as a failure in case the coverage
// was removed accidentally (e.g., as part of a refactor).
t.Errorf("no misc-compile TryBot coverage for the special 'linux-arm-arm5' pseudo-port")
}
}

// TestExpectedMacstadiumVMCount ensures that only 20 instances of macOS virtual machines
// are expected at MacStadium.
// TODO: remove once the scheduler allocates VMs based on demand https://golang.org/issue/35698
Expand Down

0 comments on commit 646d48d

Please sign in to comment.