Charmhub: Retry charmhub requests

The PR attempts to retry charmhub requests when the server returns the following status codes: - http.StatusBadGateway - http.StatusGatewayTimeout - http.StatusServiceUnavailable - http.StatusTooManyRequests
sdg0919 · Jun 24, 2021 · 1301b41 · 1301b41
1 parent 61df2f1
commit 1301b41
Show file tree

Hide file tree

Showing 5 changed files with 44 additions and 9 deletions.
diff --git a/apiserver/facades/client/charmhub/charmhub.go b/apiserver/facades/client/charmhub/charmhub.go
@@ -60,7 +60,7 @@ func NewFacade(ctx facade.Context) (*CharmHubAPI, error) {
 	}
 
 	return newCharmHubAPI(m, ctx.Auth(), charmHubClientFactory{
-		httpTransport: charmhub.RequestRecorderHTTPTransport(ctx.RequestRecorder()),
+		httpTransport: charmhub.RequestHTTPTransport(ctx.RequestRecorder(), charmhub.DefaultRetryPolicy()),
 	})
 }
 

diff --git a/apiserver/facades/client/charms/client.go b/apiserver/facades/client/charms/client.go
@@ -125,7 +125,7 @@ func NewFacadeV4(ctx facade.Context) (*API, error) {
 		return nil, errors.Trace(err)
 	}
 
-	httpTransport := charmhub.RequestRecorderHTTPTransport(ctx.RequestRecorder())
+	httpTransport := charmhub.RequestHTTPTransport(ctx.RequestRecorder(), charmhub.DefaultRetryPolicy())
 
 	return &API{
 		CharmsAPI:            commonCharmsAPI,

diff --git a/charmhub/http.go b/charmhub/http.go
@@ -30,6 +30,30 @@ const (
 	JSON MIME = "application/json"
 )
 
+const (
+	// DefaultRetryAttempts defines the number of attempts that a default http
+	// transport will retry before giving up.
+	// Retries are only performed on certain status codes, nothing in the 200 to
+	// 400 range and a select few from the 500 range (deemed retryable):
+	//
+	// - http.StatusBadGateway
+	// - http.StatusGatewayTimeout
+	// - http.StatusServiceUnavailable
+	// - http.StatusTooManyRequests
+	//
+	// See: juju/http package.
+	DefaultRetryAttempts = 3
+
+	// DefaultRetryDelay holds the amount of time after a try, a new attempt
+	// will wait before another attempt.
+	DefaultRetryDelay = time.Second * 10
+
+	// DefaultRetryMaxDelay holds the amount of time before a giving up on a
+	// request. This values includes any server response from the header
+	// Retry-After.
+	DefaultRetryMaxDelay = time.Minute * 10
+)
+
 // Transport defines a type for making the actual request.
 type Transport interface {
 	// Do performs the *http.Request and returns a *http.Response or an error
@@ -39,9 +63,19 @@ type Transport interface {
 
 // DefaultHTTPTransport creates a new HTTPTransport.
 func DefaultHTTPTransport(logger Logger) Transport {
-	return RequestRecorderHTTPTransport(loggingRequestRecorder{
+	return RequestHTTPTransport(loggingRequestRecorder{
 		logger: logger.Child("request-recorder"),
-	})(logger)
+	}, DefaultRetryPolicy())(logger)
+}
+
+// DefaultRetryPolicy returns a retry policy with sane defaults for most
+// requests.
+func DefaultRetryPolicy() jujuhttp.RetryPolicy {
+	return jujuhttp.RetryPolicy{
+		Attempts: DefaultRetryAttempts,
+		Delay:    DefaultRetryDelay,
+		MaxDelay: DefaultRetryMaxDelay,
+	}
 }
 
 type loggingRequestRecorder struct {
@@ -62,12 +96,13 @@ func (r loggingRequestRecorder) RecordError(method string, url *url.URL, err err
 	}
 }
 
-// RequestRecorderHTTPTransport creates a new HTTPTransport that records the
+// RequestHTTPTransport creates a new HTTPTransport that records the
 // requests.
-func RequestRecorderHTTPTransport(recorder jujuhttp.RequestRecorder) func(logger Logger) Transport {
+func RequestHTTPTransport(recorder jujuhttp.RequestRecorder, policy jujuhttp.RetryPolicy) func(logger Logger) Transport {
 	return func(logger Logger) Transport {
 		return jujuhttp.NewClient(
 			jujuhttp.WithRequestRecorder(recorder),
+			jujuhttp.WithRequestRetrier(policy),
 			jujuhttp.WithLogger(logger),
 		)
 	}

diff --git a/go.mod b/go.mod
@@ -58,7 +58,7 @@ require (
 	github.com/juju/gnuflag v0.0.0-20171113085948-2ce1bb71843d
 	github.com/juju/gojsonschema v0.0.0-20150312170016-e1ad140384f2
 	github.com/juju/gomaasapi/v2 v2.0.0-20210323144809-92beddd020fe
-	github.com/juju/http/v2 v2.0.0-20210616081525-ede00d07798a
+	github.com/juju/http/v2 v2.0.0-20210623153641-418e83b0dab4
 	github.com/juju/idmclient/v2 v2.0.0-20210309081103-6b4a5212f851
 	github.com/juju/jsonschema v0.0.0-20210422141032-b0ff291abe9c
 	github.com/juju/jsonschema-gen v0.0.0-20200416014454-d924343d72b2

diff --git a/go.sum b/go.sum
@@ -425,8 +425,8 @@ github.com/juju/gosigma v0.0.0-20200420012028-063911838a9e h1:JoXBbhRrYNw6EIPGcM
 github.com/juju/gosigma v0.0.0-20200420012028-063911838a9e/go.mod h1:OPBu48GcIJ30kNTA1cm+VbZb6GkQ6vthnr5v6NJ49eM=
 github.com/juju/http v0.0.0-20201019013536-69ae1d429836 h1:F+KhYWcKHSUf/R7Ovoz+s6VnK1wGFibaCrPXPYijFa4=
 github.com/juju/http v0.0.0-20201019013536-69ae1d429836/go.mod h1:lbZ9zbaOw9vMW7XMHGxYTgFadDDfzc4r8Aa7gP8GOYo=
-github.com/juju/http/v2 v2.0.0-20210616081525-ede00d07798a h1:Moqm99huPhL1lffDs8uNZ6VJ3kAPDxPPu/kbZlMAy98=
-github.com/juju/http/v2 v2.0.0-20210616081525-ede00d07798a/go.mod h1:y1PngvubGWeSpDBI4kxdaTc9f3HEThd2A5mv0MKBi74=
+github.com/juju/http/v2 v2.0.0-20210623153641-418e83b0dab4 h1:rG3LBPCjMbyJS49DmiTG3tn/taVkaDiN898aDf+6fdk=
+github.com/juju/http/v2 v2.0.0-20210623153641-418e83b0dab4/go.mod h1:1cSFAPYQcBMP3uzXzAkTrX756GmMM/Dw0WxcnhNNntc=
 github.com/juju/httpprof v0.0.0-20141217160036-14bf14c30767 h1:COsaGcfAONDdIDnGS8yFdxOyReP7zKQEr7jFzCHKDkM=
 github.com/juju/httpprof v0.0.0-20141217160036-14bf14c30767/go.mod h1:+MaLYz4PumRkkyHYeXJ2G5g5cIW0sli2bOfpmbaMV/g=
 github.com/juju/httprequest v1.0.1/go.mod h1:K+CyYVHU/NcfbMpK7YIVobh4U4Fci3EUB2AqIRtl+xs=
-Original file line number
+Diff line change
@@ Expand Up / @@ -60,7 +60,7 @@ func NewFacade(ctx facade.Context) (*CharmHubAPI, error) { @@
     	}
     	return newCharmHubAPI(m, ctx.Auth(), charmHubClientFactory{
-    		httpTransport: charmhub.RequestRecorderHTTPTransport(ctx.RequestRecorder()),
+    		httpTransport: charmhub.RequestHTTPTransport(ctx.RequestRecorder(), charmhub.DefaultRetryPolicy()),
     	})
     }
@@ Expand Down @@