[ARVADOS] updated: b534fd13b3446067ddf6f8bd3c5b19c3463bc655
git at public.curoverse.com
git at public.curoverse.com
Tue Oct 20 11:01:28 EDT 2015
Summary of changes:
sdk/go/keepclient/keepclient.go | 42 ++++++++++++++++++++++++++++++++++--
sdk/go/keepclient/keepclient_test.go | 9 ++++++--
services/keepproxy/keepproxy.go | 14 ++++++++----
services/keepproxy/keepproxy_test.go | 32 ++++++++++++++++++++-------
4 files changed, 81 insertions(+), 16 deletions(-)
via b534fd13b3446067ddf6f8bd3c5b19c3463bc655 (commit)
from 680d70e6df8e6bf30309b908eaacdf6476d37dc2 (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 b534fd13b3446067ddf6f8bd3c5b19c3463bc655
Author: radhika <radhika at curoverse.com>
Date: Tue Oct 20 10:59:41 2015 -0400
7492: better error reporting of upstream errors in keepproxy.
diff --git a/sdk/go/keepclient/keepclient.go b/sdk/go/keepclient/keepclient.go
index 67c304d..a018eb4 100644
--- a/sdk/go/keepclient/keepclient.go
+++ b/sdk/go/keepclient/keepclient.go
@@ -22,7 +22,33 @@ import (
// A Keep "block" is 64MB.
const BLOCKSIZE = 64 * 1024 * 1024
-var BlockNotFound = errors.New("Block not found")
+// Error interface with an error and boolean indicating whether the error is temporary
+type Error interface {
+ error
+ Temporary() bool
+}
+
+// multipleResponseError is of type Error
+type multipleResponseError struct {
+ error
+ isTemp bool
+}
+
+func (e *multipleResponseError) Temporary() bool {
+ return e.isTemp
+}
+
+// BlockNotFound is a multipleResponseError where isTemp is false
+var BlockNotFound = &ErrNotFound{multipleResponseError{
+ error: errors.New("Block not found"),
+ isTemp: false,
+}}
+
+// ErrNotFound is a multipleResponseError where isTemp can be true or false
+type ErrNotFound struct {
+ multipleResponseError
+}
+
var InsufficientReplicasError = errors.New("Could not write sufficient replicas")
var OversizeBlockError = errors.New("Exceeded maximum block size (" + strconv.Itoa(BLOCKSIZE) + ")")
var MissingArvadosApiHost = errors.New("Missing required environment variable ARVADOS_API_HOST")
@@ -143,6 +169,7 @@ func (kc *KeepClient) PutR(r io.Reader) (locator string, replicas int, err error
func (kc *KeepClient) getOrHead(method string, locator string) (io.ReadCloser, int64, string, error) {
var errs []string
+ var count404 int
tries_remaining := 1 + kc.Retries
serversToTry := kc.getSortedRoots(locator)
@@ -181,6 +208,8 @@ func (kc *KeepClient) getOrHead(method string, locator string) (io.ReadCloser, i
// server side failure, transient
// error, can try again.
retryList = append(retryList, host)
+ } else if resp.StatusCode == 404 {
+ count404++
}
} else {
// Success.
@@ -201,7 +230,16 @@ func (kc *KeepClient) getOrHead(method string, locator string) (io.ReadCloser, i
}
log.Printf("DEBUG: %s %s failed: %v", method, locator, errs)
- return nil, 0, "", BlockNotFound
+ var err error
+ if count404 == len(kc.getSortedRoots(locator)) {
+ err = BlockNotFound
+ } else {
+ err = &ErrNotFound{multipleResponseError{
+ error: fmt.Errorf("%s %s failed: %v", method, locator, errs),
+ isTemp: len(serversToTry) > 0,
+ }}
+ }
+ return nil, 0, "", err
}
// Get() retrieves a block, given a locator. Returns a reader, the
diff --git a/sdk/go/keepclient/keepclient_test.go b/sdk/go/keepclient/keepclient_test.go
index aaba069..3714a54 100644
--- a/sdk/go/keepclient/keepclient_test.go
+++ b/sdk/go/keepclient/keepclient_test.go
@@ -14,6 +14,7 @@ import (
"net"
"net/http"
"os"
+ "strings"
"testing"
)
@@ -554,7 +555,9 @@ func (s *StandaloneSuite) TestGetFail(c *C) {
kc.SetServiceRoots(map[string]string{"x": ks.url}, nil, nil)
r, n, url2, err := kc.Get(hash)
- c.Check(err, Equals, BlockNotFound)
+ errNotFound, _ := err.(ErrNotFound)
+ c.Check(errNotFound, NotNil)
+ c.Check(strings.Contains(err.Error(), "use of closed network connection"), Equals, true)
c.Check(n, Equals, int64(0))
c.Check(url2, Equals, "")
c.Check(r, Equals, nil)
@@ -599,7 +602,9 @@ func (s *StandaloneSuite) TestGetNetError(c *C) {
kc.SetServiceRoots(map[string]string{"x": "http://localhost:62222"}, nil, nil)
r, n, url2, err := kc.Get(hash)
- c.Check(err, Equals, BlockNotFound)
+ errNotFound, _ := err.(ErrNotFound)
+ c.Check(errNotFound, NotNil)
+ c.Check(strings.Contains(err.Error(), "connection refused"), Equals, true)
c.Check(n, Equals, int64(0))
c.Check(url2, Equals, "")
c.Check(r, Equals, nil)
diff --git a/services/keepproxy/keepproxy.go b/services/keepproxy/keepproxy.go
index 7900096..3cf1e28 100644
--- a/services/keepproxy/keepproxy.go
+++ b/services/keepproxy/keepproxy.go
@@ -362,7 +362,7 @@ func (this GetBlockHandler) ServeHTTP(resp http.ResponseWriter, req *http.Reques
log.Println("Warning:", GetRemoteAddress(req), req.Method, proxiedURI, "Content-Length not provided")
}
- switch err {
+ switch respErr := err.(type) {
case nil:
status = http.StatusOK
resp.Header().Set("Content-Length", fmt.Sprint(expectLength))
@@ -375,10 +375,16 @@ func (this GetBlockHandler) ServeHTTP(resp http.ResponseWriter, req *http.Reques
err = ContentLengthMismatch
}
}
- case keepclient.BlockNotFound:
- status = http.StatusNotFound
+ case keepclient.Error:
+ if respErr.Error() == "Block not found" {
+ status = http.StatusNotFound
+ } else if respErr.Temporary() {
+ status = http.StatusBadGateway
+ } else {
+ status = 422
+ }
default:
- status = http.StatusBadGateway
+ status = http.StatusInternalServerError
}
}
diff --git a/services/keepproxy/keepproxy_test.go b/services/keepproxy/keepproxy_test.go
index 7643e4b..17caefb 100644
--- a/services/keepproxy/keepproxy_test.go
+++ b/services/keepproxy/keepproxy_test.go
@@ -172,14 +172,18 @@ func (s *ServerRequiredSuite) TestPutAskGet(c *C) {
{
_, _, err := kc.Ask(hash)
- c.Check(err, Equals, keepclient.BlockNotFound)
+ errNotFound, _ := err.(keepclient.ErrNotFound)
+ c.Check(errNotFound, NotNil)
+ c.Check(strings.Contains(err.Error(), "Block not found"), Equals, true)
log.Print("Finished Ask (expected BlockNotFound)")
}
{
reader, _, _, err := kc.Get(hash)
c.Check(reader, Equals, nil)
- c.Check(err, Equals, keepclient.BlockNotFound)
+ errNotFound, _ := err.(keepclient.ErrNotFound)
+ c.Check(errNotFound, NotNil)
+ c.Check(strings.Contains(err.Error(), "Block not found"), Equals, true)
log.Print("Finished Get (expected BlockNotFound)")
}
@@ -251,7 +255,9 @@ func (s *ServerRequiredSuite) TestPutAskGetForbidden(c *C) {
{
_, _, err := kc.Ask(hash)
- c.Check(err, Equals, keepclient.BlockNotFound)
+ errNotFound, _ := err.(keepclient.ErrNotFound)
+ c.Check(errNotFound, NotNil)
+ c.Assert(strings.Contains(err.Error(), "HTTP 403"), Equals, true)
log.Print("Ask 1")
}
@@ -265,14 +271,18 @@ func (s *ServerRequiredSuite) TestPutAskGetForbidden(c *C) {
{
blocklen, _, err := kc.Ask(hash)
- c.Assert(err, Equals, keepclient.BlockNotFound)
+ errNotFound, _ := err.(keepclient.ErrNotFound)
+ c.Check(errNotFound, NotNil)
+ c.Assert(strings.Contains(err.Error(), "HTTP 403"), Equals, true)
c.Check(blocklen, Equals, int64(0))
log.Print("Ask 2")
}
{
_, blocklen, _, err := kc.Get(hash)
- c.Assert(err, Equals, keepclient.BlockNotFound)
+ errNotFound, _ := err.(keepclient.ErrNotFound)
+ c.Check(errNotFound, NotNil)
+ c.Assert(strings.Contains(err.Error(), "HTTP 403"), Equals, true)
c.Check(blocklen, Equals, int64(0))
log.Print("Get")
}
@@ -291,7 +301,9 @@ func (s *ServerRequiredSuite) TestGetDisabled(c *C) {
{
_, _, err := kc.Ask(hash)
- c.Check(err, Equals, keepclient.BlockNotFound)
+ errNotFound, _ := err.(keepclient.ErrNotFound)
+ c.Check(errNotFound, NotNil)
+ c.Assert(strings.Contains(err.Error(), "HTTP 400"), Equals, true)
log.Print("Ask 1")
}
@@ -305,14 +317,18 @@ func (s *ServerRequiredSuite) TestGetDisabled(c *C) {
{
blocklen, _, err := kc.Ask(hash)
- c.Assert(err, Equals, keepclient.BlockNotFound)
+ errNotFound, _ := err.(keepclient.ErrNotFound)
+ c.Check(errNotFound, NotNil)
+ c.Assert(strings.Contains(err.Error(), "HTTP 400"), Equals, true)
c.Check(blocklen, Equals, int64(0))
log.Print("Ask 2")
}
{
_, blocklen, _, err := kc.Get(hash)
- c.Assert(err, Equals, keepclient.BlockNotFound)
+ errNotFound, _ := err.(keepclient.ErrNotFound)
+ c.Check(errNotFound, NotNil)
+ c.Assert(strings.Contains(err.Error(), "HTTP 400"), Equals, true)
c.Check(blocklen, Equals, int64(0))
log.Print("Get")
}
-----------------------------------------------------------------------
hooks/post-receive
--
More information about the arvados-commits
mailing list