From 9a4828821dac461058a7b995f6575eeae0914b82 Mon Sep 17 00:00:00 2001 From: Wojciech Tyczynski Date: Tue, 20 Dec 2016 14:41:18 +0100 Subject: [PATCH] Retry connection reset by peer --- pkg/client/restclient/request.go | 15 ++++++++++++- pkg/client/restclient/request_test.go | 31 +++++++++++++++++++++++++++ pkg/util/net/util.go | 10 +++++++++ 3 files changed, 55 insertions(+), 1 deletion(-) diff --git a/pkg/client/restclient/request.go b/pkg/client/restclient/request.go index c2498d42e3d..8c45c16818c 100644 --- a/pkg/client/restclient/request.go +++ b/pkg/client/restclient/request.go @@ -811,7 +811,20 @@ func (r *Request) request(fn func(*http.Request, *http.Response)) error { r.backoffMgr.UpdateBackoff(r.URL(), err, resp.StatusCode) } if err != nil { - return err + // "Connection reset by peer" is usually a transient error. + // Thus in case of "GET" operations, we simply retry it. + // We are not automatically retrying "write" operations, as + // they are not idempotent. + if !net.IsConnectionReset(err) || r.verb != "GET" { + return err + } + // For the purpose of retry, we set the artificial "retry-after" response. + // TODO: Should we clean the original response if it exists? + resp = &http.Response{ + StatusCode: http.StatusInternalServerError, + Header: http.Header{"Retry-After": []string{"1"}}, + Body: ioutil.NopCloser(bytes.NewReader([]byte{})), + } } done := func() bool { diff --git a/pkg/client/restclient/request_test.go b/pkg/client/restclient/request_test.go index 4493537a88c..d03ef9375f1 100755 --- a/pkg/client/restclient/request_test.go +++ b/pkg/client/restclient/request_test.go @@ -22,12 +22,14 @@ import ( "fmt" "io" "io/ioutil" + "net" "net/http" "net/http/httptest" "net/url" "os" "reflect" "strings" + "syscall" "testing" "time" @@ -1120,6 +1122,35 @@ func TestCheckRetryClosesBody(t *testing.T) { } } +func TestConnectionResetByPeerIsRetried(t *testing.T) { + count := 0 + backoff := &testBackoffManager{} + req := &Request{ + verb: "GET", + client: clientFunc(func(req *http.Request) (*http.Response, error) { + count++ + if count >= 3 { + return &http.Response{ + StatusCode: 200, + Body: ioutil.NopCloser(bytes.NewReader([]byte{})), + }, nil + } + return nil, &net.OpError{Err: syscall.ECONNRESET} + }), + backoffMgr: backoff, + } + // We expect two retries of "connection reset by peer" and the success. + _, err := req.Do().Raw() + if err != nil { + t.Errorf("Unexpected error: %v", err) + } + // We have a sleep before each retry (including the initial one) and for + // every "retry-after" call - thus 5 together. + if len(backoff.sleeps) != 5 { + t.Errorf("Expected 5 retries, got: %d", len(backoff.sleeps)) + } +} + func TestCheckRetryHandles429And5xx(t *testing.T) { count := 0 ch := make(chan struct{}) diff --git a/pkg/util/net/util.go b/pkg/util/net/util.go index 1348f4deeb9..461144f0bab 100644 --- a/pkg/util/net/util.go +++ b/pkg/util/net/util.go @@ -19,6 +19,7 @@ package net import ( "net" "reflect" + "syscall" ) // IPNetEqual checks if the two input IPNets are representing the same subnet. @@ -34,3 +35,12 @@ func IPNetEqual(ipnet1, ipnet2 *net.IPNet) bool { } return false } + +// Returns if the given err is "connection reset by peer" error. +func IsConnectionReset(err error) bool { + opErr, ok := err.(*net.OpError) + if ok && opErr.Err.Error() == syscall.ECONNRESET.Error() { + return true + } + return false +}