File 0002-httputil-don-t-retry-when-network-is-down.... of Package snapd

Overview Repositories Revisions Requests Users Attributes Meta

File 0002-httputil-don-t-retry-when-network-is-down.patch of Package snapd (Revision 7978d9cb1f9e6fcd8cd7e5212cb945bb)

Currently displaying revision 7978d9cb1f9e6fcd8cd7e5212cb945bb , Show latest

From 3b09b93f3349dd2b78699fd615faea699b7b5054 Mon Sep 17 00:00:00 2001
From: Zygmunt Krynicki <me@zygoon.pl>
Date: Thu, 30 May 2019 12:40:27 +0200
Subject: [PATCH 2/4] httputil: don't retry when network is down

Running our test suite on an offline system, or more conveniently, in a
network namespace with only loopback device, I ran across this error:

FAIL: retry_test.go:406: retrySuite.TestRetryDoesNotFailForPermanentDNSErrors

retry_test.go:424:
	// we try exactly once, a non-existing server is a permanent error
	c.Assert(n, Equals, 1)
    ... obtained int = 5
    ... expected int = 1

The retry test would attempt to perform a network operation five times
even though the system has no chance of resolving any names because it
is offline.

Inspection of the returned errors indicates the following error chain:

outer:  &url.Error{Op:"Get", URL:"http://nonexistingserver909123.com/", Err:(*net.OpError)(0xc000200140)}
    nested: &net.OpError{Op:"dial", Net:"tcp", Source:net.Addr(nil), Addr:net.Addr(nil), Err:(*net.DNSError)(0xc0001b8080)}
    nexted: &net.DNSError{Err:"dial udp 172.16.124.2:53: connect: network is unreachable", Name:"nonexistingserver909123.com", Server:"172.16.124.2:53", IsTimeout:false, IsTemporary:true}

Crucially the innermost error is marked as temporary.

The logic we have first checks if an error is temporary, and if so,
immediately schedules retry, and only then checks if the system is
offline or if DNS is unavailable.

I admit that the retry logic is complex but on a quick look swapping
those checks around is the right thigh to do. When we're offline there's
no point in retrying.

The test now passes and there are no regressions in any systems in
spread.

Signed-off-by: Zygmunt Krynicki <me@zygoon.pl>
---
 httputil/retry.go | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/httputil/retry.go b/httputil/retry.go
index 80d2cd4045..b756e256ba 100644
--- a/httputil/retry.go
+++ b/httputil/retry.go
@@ -190,13 +190,14 @@ func RetryRequest(endpoint string, doRequest func() (*http.Response, error), rea
 
 		resp, err = doRequest()
 		if err != nil {
+			if isNetworkDown(err) || isDnsUnavailable(err) {
+				err = &PerstistentNetworkError{Err: err}
+				break
+			}
 			if ShouldRetryError(attempt, err) {
 				continue
 			}
 
-			if isNetworkDown(err) || isDnsUnavailable(err) {
-				err = &PerstistentNetworkError{Err: err}
-			}
 			break
 		}
 
-- 
2.21.0

Places

File 0002-httputil-don-t-retry-when-network-is-down.patch of Package snapd (Revision 7978d9cb1f9e6fcd8cd7e5212cb945bb)

Places