Merge pull request #129 from jonboulle/exp

fix(pkg): simplify exponential backoff to avoid overflows

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)
 	}
 

pkg/http_client_test.go

@@ -3,11 +3,30 @@ package pkg
 import (
 	"fmt"
 	"io"
+	"math"
 	"net/http"
 	"net/http/httptest"
 	"testing"
+	"time"
 )
 
+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
@@ -16,10 +35,6 @@ var expBackoffTests = []struct {
 		{1, "number of attempts: 1"},
 		{2, "number of attempts: 2"},
 	}
-
-// Test exponential backoff and that it continues retrying if a 5xx response is
-// received
-func TestGetURLExpBackOff(t *testing.T) {
 	client := NewHttpClient()
 
 	for i, tt := range expBackoffTests {