Arvados

Bug #7492

Updated by Tom Clegg over 8 years ago

Keepproxy (which uses keepclient) is failing requests because one of the upstream servers returned "read tcp 10.26.0.7:25107: use of closed network connection".    This is being returned to the client as a 404, but actually should be a 502 (Bad Gateway) which would trigger retry logic on the client. 

 h3. Requirements 

 If an error occurs in (*keepclient.KeepClient)Get() then keepproxy's http response status must be: 
 * 404 if every keepstore service returned 404 
 * 422 if every keepstore service returned a non-retryable error (but some didn't return 404) 
 * 502 if any keepstore service had still returned a retryable error when @tries_remaining@ ran out 

 Also, the list of errors received from the upstream keepstore services should be propagated to keepproxy's client in the response body. (Currently this information is logged by the SDK, so it shows up in keepproxy's log file but it is not available to the originator of the request.) 

 h3. Implementation 

 We need to update @(*keepclient.KeepClient)Get()@ to make it _possible_ for keepproxy to distinguish between the above outcomes. 

 keepclient: 
 * create an Error interface (similar to https://golang.org/pkg/net/#Error) 
 ** <pre><code>type Error struct { 
	 error 
	 Temporary() bool // Is the error temporary? 
 }</code></pre> 
 * create a multipleResponseError type that implements Error 
 ** <pre><code>type multipleResponseError struct { 
	 error 
	 isTemp bool 
 } 
 func (e *multipleResponseError) Temporary() bool { 
	 return e.isTemp 
 } 
 </code></pre> 
 * create a NotFoundError type that implements Error 
 ** <pre><code>type NotFoundError struct { 
	 multipleResponseError 
 } 
 </code></pre> 
 * Change BlockNotFound to a NotFoundError 
 ** <pre><code>var BlockNotFound = &NotFoundError{multipleResponseError{ 
	 error:    errors.New("Block not found"), 
	 isTemp: false, 
 }} 
 </code></pre> 
 * in Get(), count how many responses are http.StatusNotFound 
 * when returning from Get() after all attempts have failed, make a NotFoundError or a multipleResponseError: 
 ** <pre><code> 
 if count404 == len(serversToTry) { 
	 err = BlockNotFound 
 } else { 
	 err = &multipleResponseError{ 
		 error: 	 fmt.Errorf("%s %s failed: %v", method, locator, errs), 
		 isTemp: len(serversToTry) > 0, 
	 } 
 } 
 </code></pre> 

 (*Defer:* add a way for clients to see a slice of all errors encountered, or at least the last error from each server. For now, that information is only available as a string, via the usual Error() method.) 

 keepproxy: 
 * Do a type select on the error returned from keepclient.Get() to determine the appropriate HTTP response 
 ** <pre><code> 
 switch err := err.(type) { 
 case keepclient.NotFoundError: 
	 // respond 404 
 case keepclient.Error: 
	 if err.Temporary() { 
		 // respond 502 
	 } else { 
		 // respond 422 
	 } 
 default: 
	 // respond 500 
 } 
 </code></pre> 
 * In all cases, return err.Error() in the response body. 

Back