From 8d2eca842c7289b0b1d243f564af19645d2d6249 Mon Sep 17 00:00:00 2001 From: Chris Cotter Date: Mon, 13 Dec 2021 15:48:45 -0500 Subject: [PATCH] feat(internal/gensupport): add configurable retry (#1324) Extend SendAndRetryRequest to allow retry options (backoff and ErrorFunc) to be passed in. Add a surface to the generated code for storage to allow the manual layer to choose whether the request is retried. I tested this out via the manual layer by verifying that the ObjectsInsertCall only retries now when WithRetry() is applied. --- google-api-go-generator/gen.go | 40 +++++++++++- internal/gensupport/resumable.go | 67 +++---------------- internal/gensupport/retry.go | 106 +++++++++++++++++++++++++++++++ internal/gensupport/send.go | 23 +++++-- internal/gensupport/send_test.go | 2 +- storage/v1/storage-gen.go | 31 ++++++++- storage/v1beta2/storage-gen.go | 2 +- 7 files changed, 203 insertions(+), 68 deletions(-) create mode 100644 internal/gensupport/retry.go diff --git a/google-api-go-generator/gen.go b/google-api-go-generator/gen.go index 10eb88ed0e4..2e337c9e1d8 100644 --- a/google-api-go-generator/gen.go +++ b/google-api-go-generator/gen.go @@ -699,6 +699,9 @@ func (a *API) GenerateCode() ([]byte, error) { pn(" %q", imp) } pn("") + if a.Name == "storage" { + pn(" %q", "github.com/googleapis/gax-go/v2") + } for _, imp := range []struct { pkg string lname string @@ -1838,6 +1841,9 @@ func (meth *Method) generateCode() { if meth.supportsMediaUpload() { pn(" mediaInfo_ *gensupport.MediaInfo") + if meth.api.Name == "storage" { + pn(" retry *gensupport.RetryConfig") + } } pn(" ctx_ context.Context") pn(" header_ http.Header") @@ -1986,6 +1992,32 @@ func (meth *Method) generateCode() { pn("}") } + if meth.supportsMediaUpload() && meth.api.Name == "storage" { + comment := "WithRetry causes the library to retry the initial request of the upload" + + "(for resumable uploads) or the entire upload (for multipart uploads) if" + + "a transient error occurs. This is contingent on ChunkSize being > 0 (so" + + "that the input data may be buffered). The backoff argument will be used to" + + "determine exponential backoff timing, and the errorFunc is used to determine" + + "which errors are considered retryable. By default, exponetial backoff will be" + + "applied using gax defaults, and the following errors are retried:" + + "\n\n" + + "- HTTP responses with codes 429, 502, 503, and 504." + + "\n\n" + + "- Transient network errors such as connection reset and io.ErrUnexpectedEOF." + + "\n\n" + + "- Errors which are considered transient using the Temporary() interface." + + "\n\n" + + "- Wrapped versions of these errors." + p("\n%s", asComment("", comment)) + pn("func (c *%s) WithRetry(bo *gax.Backoff, errorFunc func(err error) bool) *%s {", callName, callName) + pn(" c.retry = &gensupport.RetryConfig{") + pn(" Backoff: bo,") + pn(" ShouldRetry: errorFunc,") + pn(" }") + pn(" return c") + pn("}") + } + comment := "Fields allows partial responses to be retrieved. " + "See https://developers.google.com/gdata/docs/2.0/basics#PartialResponse " + "for more information." @@ -2106,7 +2138,10 @@ func (meth *Method) generateCode() { pn(`})`) } if meth.supportsMediaUpload() && meth.api.Name == "storage" { - pn("return gensupport.SendRequestWithRetry(c.ctx_, c.s.client, req)") + pn("if c.retry != nil {") + pn(" return gensupport.SendRequestWithRetry(c.ctx_, c.s.client, req, c.retry)") + pn("}") + pn("return gensupport.SendRequest(c.ctx_, c.s.client, req)") } else { pn("return gensupport.SendRequest(c.ctx_, c.s.client, req)") } @@ -2172,6 +2207,9 @@ func (meth *Method) generateCode() { pn("if rx != nil {") pn(" rx.Client = c.s.client") pn(" rx.UserAgent = c.s.userAgent()") + if meth.api.Name == "storage" { + pn(" rx.Retry = c.retry") + } pn(" ctx := c.ctx_") pn(" if ctx == nil {") // TODO(mcgreevy): Require context when calling Media, or Do. diff --git a/internal/gensupport/resumable.go b/internal/gensupport/resumable.go index edc87ec24f6..0fb74606c3a 100644 --- a/internal/gensupport/resumable.go +++ b/internal/gensupport/resumable.go @@ -12,32 +12,6 @@ import ( "net/http" "sync" "time" - - gax "github.com/googleapis/gax-go/v2" -) - -// 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 ( - retryDeadline = 32 * time.Second - backoff = func() Backoff { - return &gax.Backoff{Initial: 100 * time.Millisecond} - } - // isRetryable 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 ) // ResumableUpload is used by the generated APIs to provide resumable uploads. @@ -57,6 +31,9 @@ type ResumableUpload struct { // Callback is an optional function that will be periodically called with the cumulative number of bytes uploaded. Callback func(int64) + + // Retry optionally configures retries for requests made against the upload. + Retry *RetryConfig } // Progress returns the number of bytes uploaded at this point. @@ -176,13 +153,15 @@ func (rx *ResumableUpload) Upload(ctx context.Context) (resp *http.Response, err } return resp, nil } + // Configure retryable error criteria. + errorFunc := rx.Retry.errorFunc() // Send all chunks. for { var pause time.Duration - // Each chunk gets its own initialized-at-zero retry. - bo := backoff() + // Each chunk gets its own initialized-at-zero backoff. + bo := rx.Retry.backoff() quitAfter := time.After(retryDeadline) // Retry loop for a single chunk. @@ -206,7 +185,7 @@ func (rx *ResumableUpload) Upload(ctx context.Context) (resp *http.Response, err } // Check if we should retry the request. - if !shouldRetry(status, err) { + if !errorFunc(status, err) { break } @@ -226,33 +205,3 @@ func (rx *ResumableUpload) Upload(ctx context.Context) (resp *http.Response, err return prepareReturn(resp, err) } } - -// shouldRetry indicates whether an error is retryable for the purposes of this -// package, following 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 -} diff --git a/internal/gensupport/retry.go b/internal/gensupport/retry.go new file mode 100644 index 00000000000..4a4861b1b1a --- /dev/null +++ b/internal/gensupport/retry.go @@ -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) + } +} diff --git a/internal/gensupport/send.go b/internal/gensupport/send.go index 276d6f6963a..dab64aef367 100644 --- a/internal/gensupport/send.go +++ b/internal/gensupport/send.go @@ -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) { // 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 diff --git a/internal/gensupport/send_test.go b/internal/gensupport/send_test.go index 289a7bc2b5b..d6af483e66e 100644 --- a/internal/gensupport/send_test.go +++ b/internal/gensupport/send_test.go @@ -24,7 +24,7 @@ func TestSendRequestWithRetry(t *testing.T) { // Setting Accept-Encoding should give an error immediately. req, _ := http.NewRequest("GET", "url", nil) req.Header.Set("Accept-Encoding", "") - _, err := SendRequestWithRetry(context.Background(), nil, req) + _, err := SendRequestWithRetry(context.Background(), nil, req, nil) if err == nil { t.Error("got nil, want error") } diff --git a/storage/v1/storage-gen.go b/storage/v1/storage-gen.go index 73869a8d2db..d67c6a99b34 100644 --- a/storage/v1/storage-gen.go +++ b/storage/v1/storage-gen.go @@ -55,6 +55,7 @@ import ( "strconv" "strings" + "github.com/googleapis/gax-go/v2" googleapi "google.golang.org/api/googleapi" gensupport "google.golang.org/api/internal/gensupport" option "google.golang.org/api/option" @@ -10099,6 +10100,7 @@ type ObjectsInsertCall struct { mediaInfo_ *gensupport.MediaInfo ctx_ context.Context header_ http.Header + retry *gensupport.RetryConfig } // Insert: Stores a new object and metadata. @@ -10265,6 +10267,29 @@ func (c *ObjectsInsertCall) ProgressUpdater(pu googleapi.ProgressUpdater) *Objec return c } +// WithRetry causes the library to retry the initial request of the upload +// (for resumable uploads) or the entire upload (for multipart uploads) if +// a transient error occurs. This is contingent on ChunkSize being > 0 (so +// that the input data may be buffered). The backoff argument will be used to +// determine exponential backoff timing, and the errorFunc is used to determine +// which errors are considered retryable. By default, exponetial backoff will be +// applied using gax defaults, and the following errors are retried: +// +// - HTTP responses with codes 429, 502, 503, and 504. +// +// - Transient network errors such as connection reset and io.ErrUnexpectedEOF. +// +// - Errors which are considered transient using the Temporary() interface. +// +// - Wrapped versions of these errors. +func (c *ObjectsInsertCall) WithRetry(bo *gax.Backoff, errorFunc func(err error) bool) *ObjectsInsertCall { + c.retry = &gensupport.RetryConfig{ + Backoff: bo, + ShouldRetry: errorFunc, + } + return c +} + // Fields allows partial responses to be retrieved. See // https://developers.google.com/gdata/docs/2.0/basics#PartialResponse // for more information. @@ -10328,7 +10353,10 @@ func (c *ObjectsInsertCall) doRequest(alt string) (*http.Response, error) { googleapi.Expand(req.URL, map[string]string{ "bucket": c.bucket, }) - return gensupport.SendRequestWithRetry(c.ctx_, c.s.client, req) + if c.retry != nil { + return gensupport.SendRequestWithRetry(c.ctx_, c.s.client, req, c.retry) + } + return gensupport.SendRequest(c.ctx_, c.s.client, req) } // Do executes the "storage.objects.insert" call. @@ -10361,6 +10389,7 @@ func (c *ObjectsInsertCall) Do(opts ...googleapi.CallOption) (*Object, error) { if rx != nil { rx.Client = c.s.client rx.UserAgent = c.s.userAgent() + rx.Retry = c.retry ctx := c.ctx_ if ctx == nil { ctx = context.TODO() diff --git a/storage/v1beta2/storage-gen.go b/storage/v1beta2/storage-gen.go index 090c2e0d321..b077f128a9b 100644 --- a/storage/v1beta2/storage-gen.go +++ b/storage/v1beta2/storage-gen.go @@ -6285,7 +6285,7 @@ func (c *ObjectsInsertCall) doRequest(alt string) (*http.Response, error) { googleapi.Expand(req.URL, map[string]string{ "bucket": c.bucket, }) - return gensupport.SendRequestWithRetry(c.ctx_, c.s.client, req) + return gensupport.SendRequest(c.ctx_, c.s.client, req) } // Do executes the "storage.objects.insert" call.