[ARVADOS] updated: 1.3.0-826-g1f62181aa

Git user git at public.curoverse.com
Fri Apr 26 17:31:13 UTC 2019


Summary of changes:
 services/keepstore/azure_blob_volume.go      | 38 ++++++++++++++++++++++++----
 services/keepstore/azure_blob_volume_test.go | 24 +++++++++++++-----
 services/keepstore/handlers.go               | 20 ++++++++++-----
 3 files changed, 63 insertions(+), 19 deletions(-)

       via  1f62181aa86195fe76d27d351a9135e44940c8ef (commit)
       via  2f3aaf928716c35b1ca07803dce8623b65f2e114 (commit)
       via  6f18406060f2ac32d505db14ceab97b08431ce04 (commit)
      from  86f72edb9b3bc75af47475bf83da07ce3ce86231 (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.


commit 1f62181aa86195fe76d27d351a9135e44940c8ef
Merge: 86f72edb9 2f3aaf928
Author: Tom Clegg <tclegg at veritasgenetics.com>
Date:   Fri Apr 26 13:30:20 2019 -0400

    Merge branch '14399-azure-list-retry'
    
    fixes #14399
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tclegg at veritasgenetics.com>


commit 2f3aaf928716c35b1ca07803dce8623b65f2e114
Author: Tom Clegg <tclegg at veritasgenetics.com>
Date:   Thu Apr 25 13:54:40 2019 -0400

    14399: Don't insert error text in the middle of an index response.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tclegg at veritasgenetics.com>

diff --git a/services/keepstore/handlers.go b/services/keepstore/handlers.go
index 51dd73a51..9a4d02df8 100644
--- a/services/keepstore/handlers.go
+++ b/services/keepstore/handlers.go
@@ -277,13 +277,19 @@ func (rtr *router) IndexHandler(resp http.ResponseWriter, req *http.Request) {
 
 	for _, v := range vols {
 		if err := v.IndexTo(prefix, resp); err != nil {
-			// The only errors returned by IndexTo are
-			// write errors returned by resp.Write(),
-			// which probably means the client has
-			// disconnected and this error will never be
-			// reported to the client -- but it will
-			// appear in our own error log.
-			http.Error(resp, err.Error(), http.StatusInternalServerError)
+			// We can't send an error message to the
+			// client because we might have already sent
+			// headers and index content. All we can do is
+			// log the error in our own logs, and (in
+			// cases where headers haven't been sent yet)
+			// set a 500 status.
+			//
+			// If headers have already been sent, the
+			// client must notice the lack of trailing
+			// newline as an indication that the response
+			// is incomplete.
+			log.Printf("index error from volume %s: %s", v, err)
+			http.Error(resp, "", http.StatusInternalServerError)
 			return
 		}
 	}

commit 6f18406060f2ac32d505db14ceab97b08431ce04
Author: Tom Clegg <tclegg at veritasgenetics.com>
Date:   Thu Apr 25 13:54:03 2019 -0400

    14399: Retry Azure ListBlobs call after 503 errors.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tclegg at veritasgenetics.com>

diff --git a/services/keepstore/azure_blob_volume.go b/services/keepstore/azure_blob_volume.go
index 6b5b233c2..3c17b3bd0 100644
--- a/services/keepstore/azure_blob_volume.go
+++ b/services/keepstore/azure_blob_volume.go
@@ -26,7 +26,11 @@ import (
 	"github.com/prometheus/client_golang/prometheus"
 )
 
-const azureDefaultRequestTimeout = arvados.Duration(10 * time.Minute)
+const (
+	azureDefaultRequestTimeout       = arvados.Duration(10 * time.Minute)
+	azureDefaultListBlobsMaxAttempts = 12
+	azureDefaultListBlobsRetryDelay  = arvados.Duration(10 * time.Second)
+)
 
 var (
 	azureMaxGetBytes           int
@@ -108,6 +112,8 @@ type AzureBlobVolume struct {
 	ReadOnly              bool
 	RequestTimeout        arvados.Duration
 	StorageClasses        []string
+	ListBlobsRetryDelay   arvados.Duration
+	ListBlobsMaxAttempts  int
 
 	azClient  storage.Client
 	container *azureContainer
@@ -149,6 +155,12 @@ func (v *AzureBlobVolume) Type() string {
 
 // Start implements Volume.
 func (v *AzureBlobVolume) Start(vm *volumeMetricsVecs) error {
+	if v.ListBlobsRetryDelay == 0 {
+		v.ListBlobsRetryDelay = azureDefaultListBlobsRetryDelay
+	}
+	if v.ListBlobsMaxAttempts == 0 {
+		v.ListBlobsMaxAttempts = azureDefaultListBlobsMaxAttempts
+	}
 	if v.ContainerName == "" {
 		return errors.New("no container name given")
 	}
@@ -486,8 +498,8 @@ func (v *AzureBlobVolume) IndexTo(prefix string, writer io.Writer) error {
 		Prefix:  prefix,
 		Include: &storage.IncludeBlobDataset{Metadata: true},
 	}
-	for {
-		resp, err := v.container.ListBlobs(params)
+	for page := 1; ; page++ {
+		resp, err := v.listBlobs(page, params)
 		if err != nil {
 			return err
 		}
@@ -517,6 +529,22 @@ func (v *AzureBlobVolume) IndexTo(prefix string, writer io.Writer) error {
 	}
 }
 
+// call v.container.ListBlobs, retrying if needed.
+func (v *AzureBlobVolume) listBlobs(page int, params storage.ListBlobsParameters) (resp storage.BlobListResponse, err error) {
+	for i := 0; i < v.ListBlobsMaxAttempts; i++ {
+		resp, err = v.container.ListBlobs(params)
+		err = v.translateError(err)
+		if err == VolumeBusyError {
+			log.Printf("ListBlobs: will retry page %d in %s after error: %s", page, v.ListBlobsRetryDelay, err)
+			time.Sleep(time.Duration(v.ListBlobsRetryDelay))
+			continue
+		} else {
+			break
+		}
+	}
+	return
+}
+
 // Trash a Keep block.
 func (v *AzureBlobVolume) Trash(loc string) error {
 	if v.ReadOnly {
@@ -674,8 +702,8 @@ func (v *AzureBlobVolume) EmptyTrash() {
 	}
 
 	params := storage.ListBlobsParameters{Include: &storage.IncludeBlobDataset{Metadata: true}}
-	for {
-		resp, err := v.container.ListBlobs(params)
+	for page := 1; ; page++ {
+		resp, err := v.listBlobs(page, params)
 		if err != nil {
 			log.Printf("EmptyTrash: ListBlobs: %v", err)
 			break
diff --git a/services/keepstore/azure_blob_volume_test.go b/services/keepstore/azure_blob_volume_test.go
index cfad7577c..8d02def14 100644
--- a/services/keepstore/azure_blob_volume_test.go
+++ b/services/keepstore/azure_blob_volume_test.go
@@ -27,6 +27,7 @@ import (
 	"testing"
 	"time"
 
+	"git.curoverse.com/arvados.git/sdk/go/arvados"
 	"github.com/Azure/azure-sdk-for-go/storage"
 	"github.com/ghodss/yaml"
 	"github.com/prometheus/client_golang/prometheus"
@@ -65,8 +66,9 @@ type azBlob struct {
 
 type azStubHandler struct {
 	sync.Mutex
-	blobs map[string]*azBlob
-	race  chan chan struct{}
+	blobs      map[string]*azBlob
+	race       chan chan struct{}
+	didlist503 bool
 }
 
 func newAzStubHandler() *azStubHandler {
@@ -281,6 +283,11 @@ func (h *azStubHandler) ServeHTTP(rw http.ResponseWriter, r *http.Request) {
 		rw.WriteHeader(http.StatusAccepted)
 	case r.Method == "GET" && r.Form.Get("comp") == "list" && r.Form.Get("restype") == "container":
 		// "List Blobs" API
+		if !h.didlist503 {
+			h.didlist503 = true
+			rw.WriteHeader(http.StatusServiceUnavailable)
+			return
+		}
 		prefix := container + "|" + r.Form.Get("prefix")
 		marker := r.Form.Get("marker")
 
@@ -388,14 +395,17 @@ func NewTestableAzureBlobVolume(t TB, readonly bool, replication int) *TestableA
 			t.Fatal(err)
 		}
 	}
+	azClient.Sender = &singleSender{}
 
 	bs := azClient.GetBlobService()
 	v := &AzureBlobVolume{
-		ContainerName:    container,
-		ReadOnly:         readonly,
-		AzureReplication: replication,
-		azClient:         azClient,
-		container:        &azureContainer{ctr: bs.GetContainerReference(container)},
+		ContainerName:        container,
+		ReadOnly:             readonly,
+		AzureReplication:     replication,
+		ListBlobsMaxAttempts: 2,
+		ListBlobsRetryDelay:  arvados.Duration(time.Millisecond),
+		azClient:             azClient,
+		container:            &azureContainer{ctr: bs.GetContainerReference(container)},
 	}
 
 	return &TestableAzureBlobVolume{

-----------------------------------------------------------------------


hooks/post-receive
-- 




More information about the arvados-commits mailing list