Merge branch '10468-blob-storage-timeouts' closes #10468

diff --git a/services/keepstore/azure_blob_volume.go b/services/keepstore/azure_blob_volume.go
index d2163f6b490376768383b260444d6be90a9ca1ed..d2c4620066cf7f88d8374f5331e64e822df760be 100644 (file)
--- a/services/keepstore/azure_blob_volume.go
+++ b/services/keepstore/azure_blob_volume.go
@@ -8,6 +8,7 @@ import (
        "io"
        "io/ioutil"
        "log"
+       "net/http"
        "os"
        "regexp"
        "strconv"
@@ -15,9 +16,12 @@ import (
        "sync"
        "time"
 
+       "git.curoverse.com/arvados.git/sdk/go/arvados"
        "github.com/curoverse/azure-sdk-for-go/storage"
 )
 
+const azureDefaultRequestTimeout = arvados.Duration(10 * time.Minute)
+
 var (
        azureMaxGetBytes           int
        azureStorageAccountName    string
@@ -95,6 +99,7 @@ type AzureBlobVolume struct {
        ContainerName         string
        AzureReplication      int
        ReadOnly              bool
+       RequestTimeout        arvados.Duration
 
        azClient storage.Client
        bsClient storage.BlobStorageClient
@@ -108,6 +113,7 @@ func (*AzureBlobVolume) Examples() []Volume {
                        StorageAccountKeyFile: "/etc/azure_storage_account_key.txt",
                        ContainerName:         "example-container-name",
                        AzureReplication:      3,
+                       RequestTimeout:        azureDefaultRequestTimeout,
                },
        }
 }
@@ -133,6 +139,13 @@ func (v *AzureBlobVolume) Start() error {
        if err != nil {
                return fmt.Errorf("creating Azure storage client: %s", err)
        }
+
+       if v.RequestTimeout == 0 {
+               v.RequestTimeout = azureDefaultRequestTimeout
+       }
+       v.azClient.HTTPClient = &http.Client{
+               Timeout: time.Duration(v.RequestTimeout),
+       }
        v.bsClient = v.azClient.GetBlobService()
 
        ok, err := v.bsClient.ContainerExists(v.ContainerName)

diff --git a/services/keepstore/s3_volume.go b/services/keepstore/s3_volume.go
index caed35b670e9484e9978e771e0e8c2aea2532e52..ed1161c38d7bf977358de642aed390b1f410e089 100644 (file)
--- a/services/keepstore/s3_volume.go
+++ b/services/keepstore/s3_volume.go
@@ -19,6 +19,11 @@ import (
        "github.com/AdRoll/goamz/s3"
 )
 
+const (
+       s3DefaultReadTimeout    = arvados.Duration(10 * time.Minute)
+       s3DefaultConnectTimeout = arvados.Duration(time.Minute)
+)
+
 var (
        // ErrS3TrashDisabled is returned by Trash if that operation
        // is impossible with the current config.
@@ -134,6 +139,8 @@ type S3Volume struct {
        LocationConstraint bool
        IndexPageSize      int
        S3Replication      int
+       ConnectTimeout     arvados.Duration
+       ReadTimeout        arvados.Duration
        RaceWindow         arvados.Duration
        ReadOnly           bool
        UnsafeDelete       bool
@@ -147,24 +154,28 @@ type S3Volume struct {
 func (*S3Volume) Examples() []Volume {
        return []Volume{
                &S3Volume{
-                       AccessKeyFile: "/etc/aws_s3_access_key.txt",
-                       SecretKeyFile: "/etc/aws_s3_secret_key.txt",
-                       Endpoint:      "",
-                       Region:        "us-east-1",
-                       Bucket:        "example-bucket-name",
-                       IndexPageSize: 1000,
-                       S3Replication: 2,
-                       RaceWindow:    arvados.Duration(24 * time.Hour),
+                       AccessKeyFile:  "/etc/aws_s3_access_key.txt",
+                       SecretKeyFile:  "/etc/aws_s3_secret_key.txt",
+                       Endpoint:       "",
+                       Region:         "us-east-1",
+                       Bucket:         "example-bucket-name",
+                       IndexPageSize:  1000,
+                       S3Replication:  2,
+                       RaceWindow:     arvados.Duration(24 * time.Hour),
+                       ConnectTimeout: arvados.Duration(time.Minute),
+                       ReadTimeout:    arvados.Duration(5 * time.Minute),
                },
                &S3Volume{
-                       AccessKeyFile: "/etc/gce_s3_access_key.txt",
-                       SecretKeyFile: "/etc/gce_s3_secret_key.txt",
-                       Endpoint:      "https://storage.googleapis.com",
-                       Region:        "",
-                       Bucket:        "example-bucket-name",
-                       IndexPageSize: 1000,
-                       S3Replication: 2,
-                       RaceWindow:    arvados.Duration(24 * time.Hour),
+                       AccessKeyFile:  "/etc/gce_s3_access_key.txt",
+                       SecretKeyFile:  "/etc/gce_s3_secret_key.txt",
+                       Endpoint:       "https://storage.googleapis.com",
+                       Region:         "",
+                       Bucket:         "example-bucket-name",
+                       IndexPageSize:  1000,
+                       S3Replication:  2,
+                       RaceWindow:     arvados.Duration(24 * time.Hour),
+                       ConnectTimeout: arvados.Duration(time.Minute),
+                       ReadTimeout:    arvados.Duration(5 * time.Minute),
                },
        }
 }
@@ -203,8 +214,21 @@ func (v *S3Volume) Start() error {
        if err != nil {
                return err
        }
+
+       // Zero timeouts mean "wait forever", which is a bad
+       // default. Default to long timeouts instead.
+       if v.ConnectTimeout == 0 {
+               v.ConnectTimeout = s3DefaultConnectTimeout
+       }
+       if v.ReadTimeout == 0 {
+               v.ReadTimeout = s3DefaultReadTimeout
+       }
+
+       client := s3.New(auth, region)
+       client.ConnectTimeout = time.Duration(v.ConnectTimeout)
+       client.ReadTimeout = time.Duration(v.ReadTimeout)
        v.bucket = &s3.Bucket{
-               S3:   s3.New(auth, region),
+               S3:   client,
                Name: v.Bucket,
        }
        return nil