X-Git-Url: https://git.arvados.org/arvados.git/blobdiff_plain/9d33848deed3a044f5e6d086a19215b29e3c2847..da5858d5f794c14cf00b830166bb34b1bcd79ba5:/sdk/go/arvados/client.go

diff --git a/sdk/go/arvados/client.go b/sdk/go/arvados/client.go
index 735a44d24c..7bc3d5bc42 100644
--- a/sdk/go/arvados/client.go
+++ b/sdk/go/arvados/client.go
@@ -26,6 +26,7 @@ import (
 	"regexp"
 	"strconv"
 	"strings"
+	"sync"
 	"sync/atomic"
 	"time"
 
@@ -76,6 +77,11 @@ type Client struct {
 	// context deadline to establish a maximum request time.
 	Timeout time.Duration
 
+	// Maximum disk cache size in bytes or percent of total
+	// filesystem size. If zero, use default, currently 10% of
+	// filesystem size.
+	DiskCacheSize ByteSizeOrPercent
+
 	dd *DiscoveryDocument
 
 	defaultRequestID string
@@ -88,7 +94,10 @@ type Client struct {
 	// differs from an outgoing connection limit (a feature
 	// provided by http.Transport) when concurrent calls are
 	// multiplexed on a single http2 connection.
-	requestLimiter requestLimiter
+	//
+	// getRequestLimiter() should always be used, because this can
+	// be nil.
+	requestLimiter *requestLimiter
 
 	last503 atomic.Value
 }
@@ -150,7 +159,8 @@ func NewClientFromConfig(cluster *Cluster) (*Client, error) {
 		APIHost:        ctrlURL.Host,
 		Insecure:       cluster.TLS.Insecure,
 		Timeout:        5 * time.Minute,
-		requestLimiter: requestLimiter{maxlimit: int64(cluster.API.MaxConcurrentRequests / 4)},
+		DiskCacheSize:  cluster.Collections.WebDAVCache.DiskCacheSize,
+		requestLimiter: &requestLimiter{maxlimit: int64(cluster.API.MaxConcurrentRequests / 4)},
 	}, nil
 }
 
@@ -238,6 +248,8 @@ var reqIDGen = httpserver.IDGenerator{Prefix: "req-"}
 
 var nopCancelFunc context.CancelFunc = func() {}
 
+var reqErrorRe = regexp.MustCompile(`net/http: invalid header `)
+
 // Do augments (*http.Client)Do(): adds Authorization and X-Request-Id
 // headers, delays in order to comply with rate-limiting restrictions,
 // and retries failed requests when appropriate.
@@ -274,6 +286,7 @@ func (c *Client) Do(req *http.Request) (*http.Response, error) {
 	var lastResp *http.Response
 	var lastRespBody io.ReadCloser
 	var lastErr error
+	var checkRetryCalled int
 
 	rclient := retryablehttp.NewClient()
 	rclient.HTTPClient = c.httpClient()
@@ -287,11 +300,20 @@ func (c *Client) Do(req *http.Request) (*http.Response, error) {
 		rclient.RetryMax = 0
 	}
 	rclient.CheckRetry = func(ctx context.Context, resp *http.Response, respErr error) (bool, error) {
-		if c.requestLimiter.Report(resp, respErr) {
+		checkRetryCalled++
+		if c.getRequestLimiter().Report(resp, respErr) {
 			c.last503.Store(time.Now())
 		}
 		if c.Timeout == 0 {
-			return false, err
+			return false, nil
+		}
+		// This check can be removed when
+		// https://github.com/hashicorp/go-retryablehttp/pull/210
+		// (or equivalent) is merged and we update go.mod.
+		// Until then, it is needed to pass
+		// TestNonRetryableStdlibError.
+		if respErr != nil && reqErrorRe.MatchString(respErr.Error()) {
+			return false, nil
 		}
 		retrying, err := retryablehttp.DefaultRetryPolicy(ctx, resp, respErr)
 		if retrying {
@@ -314,21 +336,29 @@ func (c *Client) Do(req *http.Request) (*http.Response, error) {
 	}
 	rclient.Logger = nil
 
-	c.requestLimiter.Acquire(ctx)
+	limiter := c.getRequestLimiter()
+	limiter.Acquire(ctx)
 	if ctx.Err() != nil {
-		c.requestLimiter.Release()
+		limiter.Release()
 		cancel()
 		return nil, ctx.Err()
 	}
 	resp, err := rclient.Do(rreq)
 	if (errors.Is(err, context.DeadlineExceeded) || errors.Is(err, context.Canceled)) && (lastResp != nil || lastErr != nil) {
-		resp, err = lastResp, lastErr
+		resp = lastResp
+		err = lastErr
+		if checkRetryCalled > 0 && err != nil {
+			// Mimic retryablehttp's "giving up after X
+			// attempts" message, even if we gave up
+			// because of time rather than maxretries.
+			err = fmt.Errorf("%s %s giving up after %d attempt(s): %w", req.Method, req.URL.String(), checkRetryCalled, err)
+		}
 		if resp != nil {
 			resp.Body = lastRespBody
 		}
 	}
 	if err != nil {
-		c.requestLimiter.Release()
+		limiter.Release()
 		cancel()
 		return nil, err
 	}
@@ -338,7 +368,7 @@ func (c *Client) Do(req *http.Request) (*http.Response, error) {
 	resp.Body = cancelOnClose{
 		ReadCloser: resp.Body,
 		cancel: func() {
-			c.requestLimiter.Release()
+			limiter.Release()
 			cancel()
 		},
 	}
@@ -352,6 +382,30 @@ func (c *Client) Last503() time.Time {
 	return t
 }
 
+// globalRequestLimiter entries (one for each APIHost) don't have a
+// hard limit on outgoing connections, but do add a delay and reduce
+// concurrency after 503 errors.
+var (
+	globalRequestLimiter     = map[string]*requestLimiter{}
+	globalRequestLimiterLock sync.Mutex
+)
+
+// Get this client's requestLimiter, or a global requestLimiter
+// singleton for c's APIHost if this client doesn't have its own.
+func (c *Client) getRequestLimiter() *requestLimiter {
+	if c.requestLimiter != nil {
+		return c.requestLimiter
+	}
+	globalRequestLimiterLock.Lock()
+	defer globalRequestLimiterLock.Unlock()
+	limiter := globalRequestLimiter[c.APIHost]
+	if limiter == nil {
+		limiter = &requestLimiter{}
+		globalRequestLimiter[c.APIHost] = limiter
+	}
+	return limiter
+}
+
 // cancelOnClose calls a provided CancelFunc when its wrapped
 // ReadCloser's Close() method is called.
 type cancelOnClose struct {
@@ -619,7 +673,11 @@ func (c *Client) apiURL(path string) string {
 	if scheme == "" {
 		scheme = "https"
 	}
-	return scheme + "://" + c.APIHost + "/" + path
+	// Double-slash in URLs tend to cause subtle hidden problems
+	// (e.g., they can behave differently when a load balancer is
+	// in the picture). Here we ensure exactly one "/" regardless
+	// of whether the given APIHost or path has a superfluous one.
+	return scheme + "://" + strings.TrimSuffix(c.APIHost, "/") + "/" + strings.TrimPrefix(path, "/")
 }
 
 // DiscoveryDocument is the Arvados server's description of itself.