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
feat(internal/gensupport): add configurable retry #1324
Changes from all commits
0955a27
05e09f4
67f78e0
2629a72
f233fab
0568799
0bd85a2
fb2d239
b452afb
4c95618
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,106 @@ | ||
// Copyright 2021 Google LLC. | ||
// Use of this source code is governed by a BSD-style | ||
// license that can be found in the LICENSE file. | ||
|
||
package gensupport | ||
|
||
import ( | ||
"io" | ||
"time" | ||
|
||
"github.com/googleapis/gax-go/v2" | ||
"google.golang.org/api/googleapi" | ||
) | ||
|
||
// Backoff is an interface around gax.Backoff's Pause method, allowing tests to provide their | ||
// own implementation. | ||
type Backoff interface { | ||
Pause() time.Duration | ||
} | ||
|
||
// These are declared as global variables so that tests can overwrite them. | ||
var ( | ||
// Per-chunk deadline for resumable uploads. | ||
retryDeadline = 32 * time.Second | ||
// Default backoff timer. | ||
backoff = func() Backoff { | ||
return &gax.Backoff{Initial: 100 * time.Millisecond} | ||
} | ||
// syscallRetryable is a platform-specific hook, specified in retryable_linux.go | ||
syscallRetryable func(error) bool = func(err error) bool { return false } | ||
) | ||
|
||
const ( | ||
// statusTooManyRequests is returned by the storage API if the | ||
// per-project limits have been temporarily exceeded. The request | ||
// should be retried. | ||
// https://cloud.google.com/storage/docs/json_api/v1/status-codes#standardcodes | ||
statusTooManyRequests = 429 | ||
) | ||
|
||
// shouldRetry indicates whether an error is retryable for the purposes of this | ||
// package, unless a ShouldRetry func is specified by the RetryConfig instead. | ||
// It follows guidance from | ||
// https://cloud.google.com/storage/docs/exponential-backoff . | ||
func shouldRetry(status int, err error) bool { | ||
if 500 <= status && status <= 599 { | ||
return true | ||
} | ||
if status == statusTooManyRequests { | ||
return true | ||
} | ||
if err == io.ErrUnexpectedEOF { | ||
return true | ||
} | ||
// Transient network errors should be retried. | ||
if syscallRetryable(err) { | ||
return true | ||
} | ||
if err, ok := err.(interface{ Temporary() bool }); ok { | ||
if err.Temporary() { | ||
return true | ||
} | ||
} | ||
// If Go 1.13 error unwrapping is available, use this to examine wrapped | ||
// errors. | ||
if err, ok := err.(interface{ Unwrap() error }); ok { | ||
return shouldRetry(status, err.Unwrap()) | ||
} | ||
return false | ||
} | ||
|
||
// RetryConfig allows configuration of backoff timing and retryable errors. | ||
type RetryConfig struct { | ||
Backoff *gax.Backoff | ||
ShouldRetry func(err error) bool | ||
} | ||
|
||
// Get a new backoff object based on the configured values. | ||
func (r *RetryConfig) backoff() Backoff { | ||
if r == nil || r.Backoff == nil { | ||
return backoff() | ||
} | ||
return &gax.Backoff{ | ||
Initial: r.Backoff.Initial, | ||
Max: r.Backoff.Max, | ||
Multiplier: r.Backoff.Multiplier, | ||
} | ||
} | ||
|
||
// This is kind of hacky; it is necessary because ShouldRetry expects to | ||
// handle HTTP errors via googleapi.Error, but the error has not yet been | ||
// wrapped with a googleapi.Error at this layer, and the ErrorFunc type | ||
// in the manual layer does not pass in a status explicitly as it does | ||
// here. So, we must wrap error status codes in a googleapi.Error so that | ||
// ShouldRetry can parse this correctly. | ||
func (r *RetryConfig) errorFunc() func(status int, err error) bool { | ||
if r == nil || r.ShouldRetry == nil { | ||
return shouldRetry | ||
} | ||
return func(status int, err error) bool { | ||
if status >= 400 { | ||
return r.ShouldRetry(&googleapi.Error{Code: status}) | ||
} | ||
return r.ShouldRetry(err) | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -10,6 +10,8 @@ import ( | |
"errors" | ||
"net/http" | ||
"time" | ||
|
||
"github.com/googleapis/gax-go/v2" | ||
) | ||
|
||
// SendRequest sends a single HTTP request using the given client. | ||
|
@@ -50,7 +52,7 @@ func send(ctx context.Context, client *http.Client, req *http.Request) (*http.Re | |
// If ctx is non-nil, it calls all hooks, then sends the request with | ||
// req.WithContext, then calls any functions returned by the hooks in | ||
// reverse order. | ||
func SendRequestWithRetry(ctx context.Context, client *http.Client, req *http.Request) (*http.Response, error) { | ||
func SendRequestWithRetry(ctx context.Context, client *http.Client, req *http.Request, retry *RetryConfig) (*http.Response, error) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The only thing I think will be an issue is that the remote offset is not considered in the case Storage has progressed and the client is now unaligned. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, that work will have to come in a separate PR. This PR doesn't change the mechanics of the resumption strategy. |
||
// Disallow Accept-Encoding because it interferes with the automatic gzip handling | ||
// done by the default http.Transport. See https://github.com/google/google-api-go-client/issues/219. | ||
if _, ok := req.Header["Accept-Encoding"]; ok { | ||
|
@@ -59,10 +61,10 @@ func SendRequestWithRetry(ctx context.Context, client *http.Client, req *http.Re | |
if ctx == nil { | ||
return client.Do(req) | ||
} | ||
return sendAndRetry(ctx, client, req) | ||
return sendAndRetry(ctx, client, req, retry) | ||
} | ||
|
||
func sendAndRetry(ctx context.Context, client *http.Client, req *http.Request) (*http.Response, error) { | ||
func sendAndRetry(ctx context.Context, client *http.Client, req *http.Request, retry *RetryConfig) (*http.Response, error) { | ||
if client == nil { | ||
client = http.DefaultClient | ||
} | ||
|
@@ -72,7 +74,18 @@ func sendAndRetry(ctx context.Context, client *http.Client, req *http.Request) ( | |
|
||
// Loop to retry the request, up to the context deadline. | ||
var pause time.Duration | ||
bo := backoff() | ||
var bo Backoff | ||
if retry != nil && retry.Backoff != nil { | ||
bo = &gax.Backoff{ | ||
Initial: retry.Backoff.Initial, | ||
Max: retry.Backoff.Max, | ||
Multiplier: retry.Backoff.Multiplier, | ||
} | ||
} else { | ||
bo = backoff() | ||
} | ||
|
||
var errorFunc = retry.errorFunc() | ||
|
||
for { | ||
select { | ||
|
@@ -96,7 +109,7 @@ func sendAndRetry(ctx context.Context, client *http.Client, req *http.Request) ( | |
// Check if we can retry the request. A retry can only be done if the error | ||
// is retryable and the request body can be re-created using GetBody (this | ||
// will not be possible if the body was unbuffered). | ||
if req.GetBody == nil || !shouldRetry(status, err) { | ||
if req.GetBody == nil || !errorFunc(status, err) { | ||
break | ||
} | ||
var errBody error | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could the error codes be pulled from gax instead of string literals?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seeing as this is just a one-off comment and these values are not likely to change, I don't think it's work adding extra complexity to the templating in order to accomplish this.