[ARVADOS] created: 1.3.0-386-g601764a10
Git user
git at public.curoverse.com
Fri Feb 22 14:03:40 EST 2019
at 601764a10575232ada34a7c0dc5ec61195094e7f (commit)
commit 601764a10575232ada34a7c0dc5ec61195094e7f
Author: Lucas Di Pentima <ldipentima at veritasgenetics.com>
Date: Fri Feb 22 15:59:32 2019 -0300
14804: Returns 503 status (instead of 404) on GET when a volume does the same.
Also, translates Azure's "...StatusCode=503..." error to VolumeBusyError.
Arvados-DCO-1.1-Signed-off-by: Lucas Di Pentima <ldipentima at veritasgenetics.com>
diff --git a/services/keepstore/azure_blob_volume.go b/services/keepstore/azure_blob_volume.go
index 5da2055b7..4f7339fac 100644
--- a/services/keepstore/azure_blob_volume.go
+++ b/services/keepstore/azure_blob_volume.go
@@ -603,6 +603,9 @@ func (v *AzureBlobVolume) translateError(err error) error {
switch {
case err == nil:
return err
+ case strings.Contains(err.Error(), "StatusCode=503"):
+ // "storage: service returned error: StatusCode=503, ErrorCode=ServerBusy, ErrorMessage=The server is busy" (See #14804)
+ return VolumeBusyError
case strings.Contains(err.Error(), "Not Found"):
// "storage: service returned without a response body (404 Not Found)"
return os.ErrNotExist
diff --git a/services/keepstore/handlers.go b/services/keepstore/handlers.go
index e079b9678..2a1bbc972 100644
--- a/services/keepstore/handlers.go
+++ b/services/keepstore/handlers.go
@@ -20,11 +20,10 @@ import (
"sync"
"time"
- "github.com/gorilla/mux"
-
"git.curoverse.com/arvados.git/sdk/go/arvados"
"git.curoverse.com/arvados.git/sdk/go/health"
"git.curoverse.com/arvados.git/sdk/go/httpserver"
+ "github.com/gorilla/mux"
)
type router struct {
@@ -669,6 +668,11 @@ func GetBlock(ctx context.Context, hash string, buf []byte, resp http.ResponseWr
if !os.IsNotExist(err) {
log.Printf("%s: Get(%s): %s", vol, hash, err)
}
+ // If some volume returns a transient error, return it to the caller
+ // instead of "Not found" so it can retry.
+ if err == VolumeBusyError {
+ errorToCaller = err.(*KeepError)
+ }
continue
}
// Check the file checksum.
commit a986c70c083ed53f9b2d0d112853d1a1155eedea
Author: Lucas Di Pentima <ldipentima at veritasgenetics.com>
Date: Fri Feb 22 15:06:30 2019 -0300
14804: Exposes the bug with a test.
Arvados-DCO-1.1-Signed-off-by: Lucas Di Pentima <ldipentima at veritasgenetics.com>
diff --git a/services/keepstore/handler_test.go b/services/keepstore/handler_test.go
index c37a4d112..ac75cf10f 100644
--- a/services/keepstore/handler_test.go
+++ b/services/keepstore/handler_test.go
@@ -151,6 +151,23 @@ func TestGetHandler(t *testing.T) {
ExpectStatusCode(t,
"Authenticated request, expired locator",
ExpiredError.HTTPCode, response)
+
+ // Authenticated request, signed locator
+ // => 503 Server busy (transient error)
+
+ // Set up the block owning volume to respond with errors
+ vols[0].(*MockVolume).Bad = true
+ vols[0].(*MockVolume).BadVolumeError = VolumeBusyError
+ response = IssueRequest(&RequestTester{
+ method: "GET",
+ uri: signedLocator,
+ apiToken: knownToken,
+ })
+ // A transient error from one volume while the other doesn't find the block
+ // should make the service return a 503 so that clients can retry.
+ ExpectStatusCode(t,
+ "Volume backend busy",
+ 503, response)
}
// Test PutBlockHandler on the following situations:
diff --git a/services/keepstore/keepstore.go b/services/keepstore/keepstore.go
index 6ae414bf9..a6c8cd995 100644
--- a/services/keepstore/keepstore.go
+++ b/services/keepstore/keepstore.go
@@ -50,6 +50,7 @@ var (
DiskHashError = &KeepError{500, "Hash mismatch in stored data"}
ExpiredError = &KeepError{401, "Expired permission signature"}
NotFoundError = &KeepError{404, "Not Found"}
+ VolumeBusyError = &KeepError{503, "Volume backend busy"}
GenericError = &KeepError{500, "Fail"}
FullError = &KeepError{503, "Full"}
SizeRequiredError = &KeepError{411, "Missing Content-Length"}
diff --git a/services/keepstore/keepstore_test.go b/services/keepstore/keepstore_test.go
index 26d49946a..d1d380466 100644
--- a/services/keepstore/keepstore_test.go
+++ b/services/keepstore/keepstore_test.go
@@ -7,6 +7,7 @@ package main
import (
"bytes"
"context"
+ "errors"
"fmt"
"io/ioutil"
"os"
@@ -165,6 +166,7 @@ func TestPutBlockOneVol(t *testing.T) {
vols := KeepVM.AllWritable()
vols[0].(*MockVolume).Bad = true
+ vols[0].(*MockVolume).BadVolumeError = errors.New("Bad volume")
// Check that PutBlock stores the data as expected.
if n, err := PutBlock(context.Background(), TestBlock, TestHash); err != nil || n < 1 {
diff --git a/services/keepstore/volume_test.go b/services/keepstore/volume_test.go
index 43ddd090c..046f3fac2 100644
--- a/services/keepstore/volume_test.go
+++ b/services/keepstore/volume_test.go
@@ -40,7 +40,8 @@ type MockVolume struct {
Timestamps map[string]time.Time
// Bad volumes return an error for every operation.
- Bad bool
+ Bad bool
+ BadVolumeError error
// Touchable volumes' Touch() method succeeds for a locator
// that has been Put().
@@ -104,7 +105,7 @@ func (v *MockVolume) Compare(ctx context.Context, loc string, buf []byte) error
v.gotCall("Compare")
<-v.Gate
if v.Bad {
- return errors.New("Bad volume")
+ return v.BadVolumeError
} else if block, ok := v.Store[loc]; ok {
if fmt.Sprintf("%x", md5.Sum(block)) != loc {
return DiskHashError
@@ -122,7 +123,7 @@ func (v *MockVolume) Get(ctx context.Context, loc string, buf []byte) (int, erro
v.gotCall("Get")
<-v.Gate
if v.Bad {
- return 0, errors.New("Bad volume")
+ return 0, v.BadVolumeError
} else if block, ok := v.Store[loc]; ok {
copy(buf[:len(block)], block)
return len(block), nil
@@ -134,7 +135,7 @@ func (v *MockVolume) Put(ctx context.Context, loc string, block []byte) error {
v.gotCall("Put")
<-v.Gate
if v.Bad {
- return errors.New("Bad volume")
+ return v.BadVolumeError
}
if v.Readonly {
return MethodDisabledError
@@ -162,7 +163,7 @@ func (v *MockVolume) Mtime(loc string) (time.Time, error) {
var mtime time.Time
var err error
if v.Bad {
- err = errors.New("Bad volume")
+ err = v.BadVolumeError
} else if t, ok := v.Timestamps[loc]; ok {
mtime = t
} else {
-----------------------------------------------------------------------
hooks/post-receive
--
More information about the arvados-commits
mailing list