Skip to content

Commit 34b995b

Browse files
committed
cmd/coordinator, dashboard: scale test exec timeouts by GO_TEST_TIMEOUT_SCALE
So slow builders can finish their cgo tests, etc. Updates golang/go#29177 etc Change-Id: If5ee8d2201dc53ab506d4f5e31406d6aae240352 Reviewed-on: https://go-review.googlesource.com/c/162540 Run-TryBot: Brad Fitzpatrick <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> Reviewed-by: Dmitri Shuralyov <[email protected]>
1 parent ff5c28c commit 34b995b

File tree

3 files changed

+87
-6
lines changed

3 files changed

+87
-6
lines changed

cmd/coordinator/coordinator.go

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2836,11 +2836,6 @@ func parseOutputAndBanner(b []byte) (banner string, out []byte) {
28362836
// (A communication error)
28372837
const maxTestExecErrors = 3
28382838

2839-
func execTimeout(testNames []string) time.Duration {
2840-
// TODO(bradfitz): something smarter probably.
2841-
return 20 * time.Minute
2842-
}
2843-
28442839
// runTestsOnBuildlet runs tis on bc, using the optional goroot & gopath environment variables.
28452840
func (st *buildStatus) runTestsOnBuildlet(bc *buildlet.Client, tis []*testItem, goroot, gopath string) {
28462841
names := make([]string, len(tis))
@@ -2871,7 +2866,7 @@ func (st *buildStatus) runTestsOnBuildlet(bc *buildlet.Client, tis []*testItem,
28712866
args = append(args, names...)
28722867
var buf bytes.Buffer
28732868
t0 := time.Now()
2874-
timeout := execTimeout(names)
2869+
timeout := st.conf.DistTestsExecTimeout(names)
28752870
var remoteErr, err error
28762871
if ti := tis[0]; ti.bench != nil {
28772872
pbr, perr := st.parentRev()

dashboard/builders.go

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import (
1212
"sort"
1313
"strconv"
1414
"strings"
15+
"time"
1516

1617
"golang.org/x/build/buildenv"
1718
"golang.org/x/build/types"
@@ -681,6 +682,8 @@ type BuildConfig struct {
681682

682683
env []string // extra environment ("key=value") pairs
683684
allScriptArgs []string
685+
686+
testHostConf *HostConfig // override HostConfig for testing, at least for now
684687
}
685688

686689
func (c *BuildConfig) Env() []string {
@@ -720,7 +723,42 @@ func (c *BuildConfig) FilePathJoin(x ...string) string {
720723
return strings.Join(x, "/")
721724
}
722725

726+
// DistTestsExecTimeout returns how long the coordinator should wait
727+
// for a cmd/dist test execution to run the provided dist test names.
728+
func (c *BuildConfig) DistTestsExecTimeout(distTests []string) time.Duration {
729+
// TODO: consider using distTests? We never did before, but
730+
// now we have the TestStats in the coordinator. Pass in a
731+
// *buildstats.TestStats and use historical data times some
732+
// fudge factor? For now just use the old 20 minute limit
733+
// we've used since 2014, but scale it by the
734+
// GO_TEST_TIMEOUT_SCALE for the super slow builders which
735+
// struggle with, say, the cgo tests. (which should be broken
736+
// up into separate dist tests or shards, like the test/ dir
737+
// was)
738+
d := 20 * time.Minute
739+
d *= time.Duration(c.timeoutScale())
740+
return d
741+
}
742+
743+
// timeoutScale returns this builder's GO_TEST_TIMEOUT_SCALE value, or 1.
744+
func (c *BuildConfig) timeoutScale() int {
745+
const pfx = "GO_TEST_TIMEOUT_SCALE="
746+
for _, env := range [][]string{c.env, c.hostConf().env} {
747+
for _, kv := range env {
748+
if strings.HasPrefix(kv, pfx) {
749+
if n, err := strconv.Atoi(kv[len(pfx):]); err == nil && n > 0 {
750+
return n
751+
}
752+
}
753+
}
754+
}
755+
return 1
756+
}
757+
723758
func (c *BuildConfig) hostConf() *HostConfig {
759+
if c.testHostConf != nil {
760+
return c.testHostConf
761+
}
724762
if c, ok := Hosts[c.HostType]; ok {
725763
return c
726764
}

dashboard/builders_test.go

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ package dashboard
77
import (
88
"strings"
99
"testing"
10+
"time"
1011
)
1112

1213
func TestOSARCHAccessors(t *testing.T) {
@@ -21,6 +22,53 @@ func TestOSARCHAccessors(t *testing.T) {
2122
}
2223
}
2324

25+
func TestDistTestsExecTimeout(t *testing.T) {
26+
tests := []struct {
27+
c *BuildConfig
28+
want time.Duration
29+
}{
30+
{
31+
&BuildConfig{
32+
env: []string{},
33+
testHostConf: &HostConfig{},
34+
},
35+
20 * time.Minute,
36+
},
37+
{
38+
&BuildConfig{
39+
env: []string{"GO_TEST_TIMEOUT_SCALE=2"},
40+
testHostConf: &HostConfig{},
41+
},
42+
40 * time.Minute,
43+
},
44+
{
45+
&BuildConfig{
46+
env: []string{},
47+
testHostConf: &HostConfig{
48+
env: []string{"GO_TEST_TIMEOUT_SCALE=3"},
49+
},
50+
},
51+
60 * time.Minute,
52+
},
53+
// BuildConfig's env takes precedence:
54+
{
55+
&BuildConfig{
56+
env: []string{"GO_TEST_TIMEOUT_SCALE=2"},
57+
testHostConf: &HostConfig{
58+
env: []string{"GO_TEST_TIMEOUT_SCALE=3"},
59+
},
60+
},
61+
40 * time.Minute,
62+
},
63+
}
64+
for i, tt := range tests {
65+
got := tt.c.DistTestsExecTimeout(nil)
66+
if got != tt.want {
67+
t.Errorf("%d. got %v; want %v", i, got, tt.want)
68+
}
69+
}
70+
}
71+
2472
func TestListTrybots(t *testing.T) {
2573
forProj := func(proj string) {
2674
t.Run(proj, func(t *testing.T) {

0 commit comments

Comments
 (0)