From 269a658d4b7b9f1053d8ed3a9c237fa739274dc9 Mon Sep 17 00:00:00 2001 From: Jonathan Boulle Date: Thu, 29 May 2014 11:03:15 -0700 Subject: [PATCH] fix(pkg): simplify exponential backoff to avoid overflows --- pkg/http_client.go | 17 ++++++++++------- pkg/http_client_test.go | 29 ++++++++++++++++++++++------- 2 files changed, 32 insertions(+), 14 deletions(-) diff --git a/pkg/http_client.go b/pkg/http_client.go index f32a205..59e055f 100644 --- a/pkg/http_client.go +++ b/pkg/http_client.go @@ -6,7 +6,6 @@ import ( "fmt" "io/ioutil" "log" - "math" "net" "net/http" neturl "net/url" @@ -43,6 +42,14 @@ func NewHttpClient() *HttpClient { } } +func expBackoff(interval, max time.Duration) time.Duration { + interval = interval * 2 + if interval > max { + interval = max + } + return interval +} + // Fetches a given URL with support for exponential backoff and maximum retries func (h *HttpClient) Get(rawurl string) ([]byte, error) { if rawurl == "" { @@ -84,6 +91,7 @@ func (h *HttpClient) Get(rawurl string) ([]byte, error) { Transport: transport, } + duration := 50 * time.Millisecond for retry := 1; retry <= h.MaxRetries; retry++ { log.Printf("Fetching data from %s. Attempt #%d", dataURL, retry) @@ -106,13 +114,8 @@ func (h *HttpClient) Get(rawurl string) ([]byte, error) { log.Printf("Unable to fetch data: %s", err.Error()) } - duration := time.Millisecond * time.Duration((math.Pow(float64(2), float64(retry)) * 100)) - if duration > h.MaxBackoff { - duration = h.MaxBackoff - } - + duration = expBackoff(duration, h.MaxBackoff) log.Printf("Sleeping for %v...", duration) - time.Sleep(duration) } diff --git a/pkg/http_client_test.go b/pkg/http_client_test.go index b0f0e46..4c246a0 100644 --- a/pkg/http_client_test.go +++ b/pkg/http_client_test.go @@ -3,23 +3,38 @@ package pkg import ( "fmt" "io" + "math" "net/http" "net/http/httptest" "testing" + "time" ) -var expBackoffTests = []struct { - count int - body string -}{ - {0, "number of attempts: 0"}, - {1, "number of attempts: 1"}, - {2, "number of attempts: 2"}, +func TestExpBackoff(t *testing.T) { + duration := time.Millisecond + max := time.Hour + for i := 0; i < math.MaxUint16; i++ { + duration = expBackoff(duration, max) + if duration < 0 { + t.Fatalf("duration too small: %v %v", duration, i) + } + if duration > max { + t.Fatalf("duration too large: %v %v", duration, i) + } + } } // Test exponential backoff and that it continues retrying if a 5xx response is // received func TestGetURLExpBackOff(t *testing.T) { + var expBackoffTests = []struct { + count int + body string + }{ + {0, "number of attempts: 0"}, + {1, "number of attempts: 1"}, + {2, "number of attempts: 2"}, + } client := NewHttpClient() for i, tt := range expBackoffTests {