[ARVADOS] created: c4f5281d2453faf3184c367f54b29bb77ecf533b

git at public.curoverse.com git at public.curoverse.com
Fri Oct 9 15:50:42 EDT 2015


        at  c4f5281d2453faf3184c367f54b29bb77ecf533b (commit)


commit c4f5281d2453faf3184c367f54b29bb77ecf533b
Author: Peter Amstutz <peter.amstutz at curoverse.com>
Date:   Fri Oct 9 15:52:36 2015 -0400

    7491: Remove KeepServerError, all errors are BlockNotFound errors for now.
    Refactor Get() and Ask() to use common function.

diff --git a/sdk/go/keepclient/keepclient.go b/sdk/go/keepclient/keepclient.go
index 2cf852b..26764e5 100644
--- a/sdk/go/keepclient/keepclient.go
+++ b/sdk/go/keepclient/keepclient.go
@@ -29,7 +29,6 @@ var OversizeBlockError = errors.New("Exceeded maximum block size (" + strconv.It
 var MissingArvadosApiHost = errors.New("Missing required environment variable ARVADOS_API_HOST")
 var MissingArvadosApiToken = errors.New("Missing required environment variable ARVADOS_API_TOKEN")
 var InvalidLocatorError = errors.New("Invalid locator")
-var KeepServerError = errors.New("One or more keep servers returned an error")
 
 // ErrNoSuchKeepServer is returned when GetIndex is invoked with a UUID with no matching keep server
 var ErrNoSuchKeepServer = errors.New("No keep server matching the given UUID is found")
@@ -140,14 +139,7 @@ func (kc *KeepClient) PutR(r io.Reader) (locator string, replicas int, err error
 	}
 }
 
-// Get() retrieves a block, given a locator. Returns a reader, the
-// expected data length, the URL the block is being fetched from, and
-// an error.
-//
-// If the block checksum does not match, the final Read() on the
-// reader returned by this method will return a BadChecksum error
-// instead of EOF.
-func (kc *KeepClient) Get(locator string) (io.ReadCloser, int64, string, error) {
+func (kc *KeepClient) getOrHead(method string, locator string) (io.ReadCloser, int64, string, error) {
 	var errs []string
 
 	tries_remaining := 1 + kc.Retries
@@ -161,7 +153,7 @@ func (kc *KeepClient) Get(locator string) (io.ReadCloser, int64, string, error)
 		for _, host := range serversToTry {
 			url := host + "/" + locator
 
-			req, err := http.NewRequest("GET", url, nil)
+			req, err := http.NewRequest(method, url, nil)
 			if err != nil {
 				errs = append(errs, fmt.Sprintf("%s: %v", url, err))
 				continue
@@ -174,36 +166,51 @@ func (kc *KeepClient) Get(locator string) (io.ReadCloser, int64, string, error)
 				errs = append(errs, fmt.Sprintf("%s: %v", url, err))
 				retryList = append(retryList, host)
 			} else if resp.StatusCode != http.StatusOK {
-				respbody, _ := ioutil.ReadAll(&io.LimitedReader{resp.Body, 4096})
-				resp.Body.Close()
+				var respbody []byte
+				if resp.Body != nil {
+					respbody, _ = ioutil.ReadAll(&io.LimitedReader{resp.Body, 4096})
+					resp.Body.Close()
+				}
 				errs = append(errs, fmt.Sprintf("%s: %d %s",
 					url, resp.StatusCode, bytes.TrimSpace(respbody)))
 
-				if resp.StatusCode >= 500 {
-					// Server side failure, may be
-					// transient, can try again.
+				if resp.StatusCode == 408 ||
+					resp.StatusCode == 429 ||
+					resp.StatusCode >= 500 {
+					// Timeout, too many requests, or other
+					// server side failure, transient
+					// error, can try again.
 					retryList = append(retryList, host)
 				}
 			} else {
 				// Success.
-				return HashCheckingReader{
-					Reader: resp.Body,
-					Hash:   md5.New(),
-					Check:  locator[0:32],
-				}, resp.ContentLength, url, nil
+				if method == "GET" {
+					return HashCheckingReader{
+						Reader: resp.Body,
+						Hash:   md5.New(),
+						Check:  locator[0:32],
+					}, resp.ContentLength, url, nil
+				} else {
+					return nil, resp.ContentLength, url, nil
+				}
 			}
 		}
 		serversToTry = retryList
 	}
 	log.Printf("DEBUG: GET %s failed: %v", locator, errs)
 
-	if len(retryList) > 0 {
-		// There was at least one failure to get a final answer
-		return nil, 0, "", KeepServerError
-	} else {
-		// Ever server returned a 4xx error
-		return nil, 0, "", BlockNotFound
-	}
+	return nil, 0, "", BlockNotFound
+}
+
+// Get() retrieves a block, given a locator. Returns a reader, the
+// expected data length, the URL the block is being fetched from, and
+// an error.
+//
+// If the block checksum does not match, the final Read() on the
+// reader returned by this method will return a BadChecksum error
+// instead of EOF.
+func (kc *KeepClient) Get(locator string) (io.ReadCloser, int64, string, error) {
+	return kc.getOrHead("GET", locator)
 }
 
 // Ask() verifies that a block with the given hash is available and
@@ -214,18 +221,8 @@ func (kc *KeepClient) Get(locator string) (io.ReadCloser, int64, string, error)
 // Returns the data size (content length) reported by the Keep service
 // and the URI reporting the data size.
 func (kc *KeepClient) Ask(locator string) (int64, string, error) {
-	for _, host := range kc.getSortedRoots(locator) {
-		url := host + "/" + locator
-		req, err := http.NewRequest("HEAD", url, nil)
-		if err != nil {
-			continue
-		}
-		req.Header.Add("Authorization", fmt.Sprintf("OAuth2 %s", kc.Arvados.ApiToken))
-		if resp, err := kc.Client.Do(req); err == nil && resp.StatusCode == http.StatusOK {
-			return resp.ContentLength, url, nil
-		}
-	}
-	return 0, "", BlockNotFound
+	_, size, url, err := kc.getOrHead("HEAD", locator)
+	return size, url, err
 }
 
 // GetIndex retrieves a list of blocks stored on the given server whose hashes
diff --git a/sdk/go/keepclient/keepclient_test.go b/sdk/go/keepclient/keepclient_test.go
index 95269bf..b1395ef 100644
--- a/sdk/go/keepclient/keepclient_test.go
+++ b/sdk/go/keepclient/keepclient_test.go
@@ -554,7 +554,7 @@ func (s *StandaloneSuite) TestGetFail(c *C) {
 	kc.SetServiceRoots(map[string]string{"x": ks.url}, map[string]string{ks.url: ""}, nil)
 
 	r, n, url2, err := kc.Get(hash)
-	c.Check(err, Equals, KeepServerError)
+	c.Check(err, Equals, BlockNotFound)
 	c.Check(n, Equals, int64(0))
 	c.Check(url2, Equals, "")
 	c.Check(r, Equals, nil)
@@ -599,7 +599,7 @@ func (s *StandaloneSuite) TestGetNetError(c *C) {
 	kc.SetServiceRoots(map[string]string{"x": "http://localhost:62222"}, map[string]string{"http://localhost:62222": ""}, nil)
 
 	r, n, url2, err := kc.Get(hash)
-	c.Check(err, Equals, KeepServerError)
+	c.Check(err, Equals, BlockNotFound)
 	c.Check(n, Equals, int64(0))
 	c.Check(url2, Equals, "")
 	c.Check(r, Equals, nil)

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


hooks/post-receive
-- 




More information about the arvados-commits mailing list