Dmitri Shuralyov has uploaded this change for review.
buildlet: bump header timeout to 20 seconds
For consistency with "20 seconds" being chosen as an arbitrary timeout
for a network roundtrip and trivial processing in cmd/coordinator in
CL 406215, apply the same change to buildlet.
Change-Id: I889ff8b86d789a6763eb0a32b0ec40050020223e
---
M buildlet/buildletclient.go
1 file changed, 18 insertions(+), 5 deletions(-)
diff --git a/buildlet/buildletclient.go b/buildlet/buildletclient.go
index 2ca30d4..bd352db 100644
--- a/buildlet/buildletclient.go
+++ b/buildlet/buildletclient.go
@@ -549,9 +549,9 @@
req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
// The first thing the buildlet's exec handler does is flush the headers, so
- // 10 seconds should be plenty of time, regardless of where on the planet
- // (Atlanta, Paris, etc) the reverse buildlet is:
- res, err := c.doHeaderTimeout(req, 10*time.Second)
+ // 20 seconds should be plenty of time, regardless of where on the planet
+ // (Atlanta, Paris, Sydney, etc.) the reverse buildlet is:
+ res, err := c.doHeaderTimeout(req, 20*time.Second)
if err == errHeaderTimeout {
c.MarkBroken()
return nil, errors.New("buildlet: timeout waiting for exec header response")
@@ -693,7 +693,7 @@
return Status{}, err
}
req = req.WithContext(ctx)
- resp, err := c.doHeaderTimeout(req, 10*time.Second) // plenty of time
+ resp, err := c.doHeaderTimeout(req, 20*time.Second) // plenty of time
if err != nil {
return Status{}, err
}
@@ -719,7 +719,7 @@
return "", err
}
req = req.WithContext(ctx)
- resp, err := c.doHeaderTimeout(req, 10*time.Second) // plenty of time
+ resp, err := c.doHeaderTimeout(req, 20*time.Second) // plenty of time
if err != nil {
return "", err
}
To view, visit change 407554. To unsubscribe, or for help writing mail filters, visit settings.
Attention is currently required from: Carlos Amedee.
Patch set 1:Run-TryBot +1Auto-Submit +1
Attention is currently required from: Dmitri Shuralyov.
Patch set 2:Code-Review +2
1 comment:
File buildlet/buildletclient.go:
Patch Set #2, Line 554: 20*time.Second
Suggestion that can be ignored: consider making this value a constant.
To view, visit change 407554. To unsubscribe, or for help writing mail filters, visit settings.
Patch set 2:Auto-Submit +1
1 comment:
File buildlet/buildletclient.go:
Patch Set #2, Line 554: 20*time.Second
Suggestion that can be ignored: consider making this value a constant.
Thanks. I've considered it, and didn't find a way to make this change that I thought worked well. Making it a constant requires coming up with a good name and rearranging comments, which seemed to help less compared to just leaving it nearest to the context where it's used in the 3 places. Happy to make the change later when I see a good way (I suspect more progress will happen as part of improving the overall state of timeouts during test execution).
To view, visit change 407554. To unsubscribe, or for help writing mail filters, visit settings.
Patch set 2:Code-Review +1
Gopher Robot submitted this change.
buildlet: bump header timeout to 20 seconds
For consistency with "20 seconds" being chosen as an arbitrary timeout
for a network roundtrip and trivial processing in cmd/coordinator in
CL 406215, apply the same change to buildlet.
Change-Id: I889ff8b86d789a6763eb0a32b0ec40050020223e
Reviewed-on: https://go-review.googlesource.com/c/build/+/407554
Auto-Submit: Dmitri Shuralyov <dmit...@golang.org>
Run-TryBot: Dmitri Shuralyov <dmit...@golang.org>
TryBot-Result: Gopher Robot <go...@golang.org>
Reviewed-by: Carlos Amedee <car...@golang.org>
Reviewed-by: Dmitri Shuralyov <dmit...@google.com>
---
M buildlet/buildletclient.go
1 file changed, 24 insertions(+), 5 deletions(-)
To view, visit change 407554. To unsubscribe, or for help writing mail filters, visit settings.