Merge pull request #6 from toga4/feature/limit-attempts

Change the backoff interfaces to allow limiting number of attempts

README.md

@@ -55,4 +55,4 @@ Some implementation for using 3rd-party backoff libraries are provided. See [ada
 
 ## Caveats
 
-- No limit on the number of attempts or the length of retries. Instead, use `http.Client.Timeout` or `context.WithTimeout` to limit the length of retries.
+- No limit on the length of retries. Instead, use `http.Client.Timeout` or `context.WithTimeout` to limit the length of retries.

adapter/github.com/googleapis/gax-go.v2/gaxbackoff/backoff.go

@@ -1,6 +1,8 @@
 package gaxbackoff
 
 import (
+	"context"
+	"sync"
 	"time"
 
 	"github.com/googleapis/gax-go/v2"
@@ -15,10 +17,43 @@ type BackoffPolicy struct {
 
 var _ retryabletransport.BackoffPolicy = (*BackoffPolicy)(nil)
 
-func (p *BackoffPolicy) New() retryabletransport.Backoff {
-	return &gax.Backoff{
-		Initial:    p.Initial,
-		Max:        p.Max,
-		Multiplier: p.Multiplier,
+type backoff struct {
+	ctx     context.Context
+	first   bool
+	backoff *gax.Backoff
+	mu      sync.Mutex
+}
+
+func (p *BackoffPolicy) New(ctx context.Context) retryabletransport.Backoff {
+	return &backoff{
+		ctx:   ctx,
+		first: true,
+		backoff: &gax.Backoff{
+			Initial:    p.Initial,
+			Max:        p.Max,
+			Multiplier: p.Multiplier,
+		},
+	}
+}
+
+func (b *backoff) Continue() bool {
+	if b.isFirst() {
+		return true
 	}
+
+	c := time.After(b.backoff.Pause())
+	select {
+	case <-b.ctx.Done():
+		return false
+	case <-c:
+		return true
+	}
+}
+
+func (b *backoff) isFirst() bool {
+	b.mu.Lock()
+	defer b.mu.Unlock()
+	f := b.first
+	b.first = false
+	return f
 }

adapter/github.com/googleapis/gax-go.v2/gaxbackoff/backoff_test.go

@@ -1,39 +1,36 @@
 package gaxbackoff
 
 import (
-	"math"
+	"context"
+	"log"
 	"testing"
 	"time"
 )
 
 func TestBackoff(t *testing.T) {
-	initialBackoff := 250 * time.Millisecond
-	maxBackoff := 5 * time.Second
+	ctx := context.Background()
+
+	initialBackoff := 20 * time.Millisecond
+	maxBackoff := 50 * time.Millisecond
 	backoffMultiplier := 2.0
 
-	backoffConfig := &BackoffPolicy{
+	backoffPolicy := &BackoffPolicy{
 		Initial:    initialBackoff,
 		Max:        maxBackoff,
 		Multiplier: backoffMultiplier,
 	}
 
-	b := backoffConfig.New()
-
-	var val time.Duration
-
-	for i := 0; i < 5; i++ {
-		max := initialBackoff * time.Duration(math.Pow(2, float64(i)))
+	ctx, cancel := context.WithTimeout(ctx, 110*time.Millisecond)
+	defer cancel()
 
-		val = b.Pause()
-		if val > max {
-			t.Errorf("expected %v to be less than %v", val, max)
-		}
+	b := backoffPolicy.New(ctx)
+	count := 0
+	for b.Continue() {
+		log.Println(count)
+		count++
 	}
 
-	for i := 0; i < 100_000; i++ {
-		val = b.Pause()
-		if val > maxBackoff {
-			t.Errorf("expected %v to be less than %v", val, maxBackoff)
-		}
+	if count < 4 {
+		t.Errorf("expected count is greater than or equal to 4 but %v", count)
 	}
 }

adapter/github.com/googleapis/gax-go.v2/gaxbackoff/go.mod

@@ -6,3 +6,5 @@ require (
 	github.com/googleapis/gax-go/v2 v2.4.0
 	github.com/toga4/go-retryabletransport v0.2.0
 )
+
+replace github.com/toga4/go-retryabletransport => ../../../../..

adapter/github.com/lestrrat-go/backoff.v2/lestrratbackoff/backoff.go

@@ -1,15 +1,13 @@
 package lestrratbackoff
 
 import (
-	"time"
-
 	"github.com/lestrrat-go/backoff/v2"
 )
 
 type adapter struct {
-	backoff backoff.IntervalGenerator
+	controller backoff.Controller
 }
 
-func (a *adapter) Pause() time.Duration {
-	return a.backoff.Next()
+func (a *adapter) Continue() bool {
+	return backoff.Continue(a.controller)
 }

adapter/github.com/lestrrat-go/backoff.v2/lestrratbackoff/constant.go

@@ -1,18 +1,20 @@
 package lestrratbackoff
 
 import (
+	"context"
+
 	"github.com/lestrrat-go/backoff/v2"
 	"github.com/toga4/go-retryabletransport"
 )
 
 type constantPolicy struct {
-	options []backoff.ConstantOption
+	policy backoff.Policy
 }
 
-func NewConstantPolicy(options ...backoff.ConstantOption) retryabletransport.BackoffPolicy {
-	return &constantPolicy{options}
+func NewConstantPolicy(options ...backoff.Option) retryabletransport.BackoffPolicy {
+	return &constantPolicy{backoff.Constant(options...)}
 }
 
-func (p *constantPolicy) New() retryabletransport.Backoff {
-	return &adapter{backoff.NewConstantInterval(p.options...)}
+func (p *constantPolicy) New(ctx context.Context) retryabletransport.Backoff {
+	return &adapter{p.policy.Start(ctx)}
 }

adapter/github.com/lestrrat-go/backoff.v2/lestrratbackoff/constant_test.go

@@ -1,30 +1,38 @@
 package lestrratbackoff
 
 import (
+	"context"
 	"testing"
 	"time"
 
 	"github.com/lestrrat-go/backoff/v2"
 )
 
 func Test_ConstantPolicy(t *testing.T) {
+	ctx := context.Background()
+
 	backoffPolicy := NewConstantPolicy(
-		backoff.WithInterval(1000*time.Millisecond),
-		backoff.WithJitterFactor(0.1),
+		backoff.WithInterval(20*time.Millisecond),
+		backoff.WithMaxRetries(3),
 	)
 
-	min := 900 * time.Millisecond
-	max := 1100 * time.Millisecond
+	b := backoffPolicy.New(ctx)
+
+	start := time.Now()
 
-	b := backoffPolicy.New()
+	count := 0
+	for b.Continue() {
+		count++
+	}
 
-	for i := 0; i < 100_000; i++ {
-		val := b.Pause()
-		if val < min {
-			t.Errorf("expected %v to be greater than %v", val, min)
-		}
-		if val > max {
-			t.Errorf("expected %v to be less than %v", val, max)
-		}
+	durationMillis := time.Now().Sub(start)
+
+	if count != 4 {
+		t.Errorf("expected count is equal to 4 but %v", count)
+	}
+	min := (60 - 5) * time.Millisecond
+	max := (60 + 5) * time.Millisecond
+	if durationMillis < min || durationMillis > max {
+		t.Errorf("expected duration is between %v and %v but %v", min, max, durationMillis)
 	}
 }

adapter/github.com/lestrrat-go/backoff.v2/lestrratbackoff/exponential.go

@@ -1,18 +1,20 @@
 package lestrratbackoff
 
 import (
+	"context"
+
 	"github.com/lestrrat-go/backoff/v2"
 	"github.com/toga4/go-retryabletransport"
 )
 
 type exponentialPolicy struct {
-	options []backoff.ExponentialOption
+	policy backoff.Policy
 }
 
 func NewExponentialPolicy(options ...backoff.ExponentialOption) retryabletransport.BackoffPolicy {
-	return &exponentialPolicy{options}
+	return &exponentialPolicy{backoff.Exponential(options...)}
 }
 
-func (p *exponentialPolicy) New() retryabletransport.Backoff {
-	return &adapter{backoff.NewExponentialInterval(p.options...)}
+func (p *exponentialPolicy) New(ctx context.Context) retryabletransport.Backoff {
+	return &adapter{p.policy.Start(ctx)}
 }

adapter/github.com/lestrrat-go/backoff.v2/lestrratbackoff/exponential_test.go

@@ -1,60 +1,40 @@
 package lestrratbackoff
 
 import (
+	"context"
 	"testing"
 	"time"
 
 	"github.com/lestrrat-go/backoff/v2"
 )
 
 func Test_ExponentialPolicy(t *testing.T) {
+	ctx := context.Background()
+
 	backoffPolicy := NewExponentialPolicy(
-		backoff.WithMinInterval(100*time.Millisecond),
-		backoff.WithMaxInterval(1*time.Second),
-		backoff.WithJitterFactor(0.1),
+		backoff.WithMinInterval(20*time.Millisecond),
+		backoff.WithMaxInterval(50*time.Millisecond),
 		backoff.WithMultiplier(2.0),
+		backoff.WithMaxRetries(3),
 	)
 
-	b := backoffPolicy.New()
-
-	val := b.Pause()
+	b := backoffPolicy.New(ctx)
 
-	{
-		min := 90 * time.Millisecond
-		max := 110 * time.Millisecond
+	start := time.Now()
 
-		if val < min {
-			t.Errorf("expected %v to be greater than %v", val, min)
-		}
-		if val > max {
-			t.Errorf("expected %v to be less than %v", val, max)
-		}
+	count := 0
+	for b.Continue() {
+		count++
 	}
 
-	for i := 0; i < 3; i++ {
-		interval := val * 2.0
-		min := time.Duration(float64(interval) * 0.9)
-		max := time.Duration(float64(interval) * 1.1)
-
-		val = b.Pause()
-		if val < min {
-			t.Errorf("expected %v to be greater than %v", val, min)
-		}
-		if val > max {
-			t.Errorf("expected %v to be less than %v", val, max)
-		}
-	}
+	durationMillis := time.Now().Sub(start)
 
-	min := 900 * time.Millisecond
-	max := 1100 * time.Millisecond
-
-	for i := 0; i < 100_000; i++ {
-		val := b.Pause()
-		if val < min {
-			t.Errorf("expected %v to be greater than %v", val, min)
-		}
-		if val > max {
-			t.Errorf("expected %v to be less than %v", val, max)
-		}
+	if count != 4 {
+		t.Errorf("expected count is equal to 4 but %v", count)
+	}
+	min := (110 - 5) * time.Millisecond
+	max := (110 + 5) * time.Millisecond
+	if durationMillis < min || durationMillis > max {
+		t.Errorf("expected duration is between %v and %v but %v", min, max, durationMillis)
 	}
 }

adapter/github.com/lestrrat-go/backoff.v2/lestrratbackoff/go.mod

@@ -6,3 +6,5 @@ require (
 	github.com/lestrrat-go/backoff/v2 v2.0.8
 	github.com/toga4/go-retryabletransport v0.2.0
 )
+
+replace github.com/toga4/go-retryabletransport => ../../../../..

backoff.go

@@ -1,17 +1,19 @@
 package retryabletransport
 
 import (
-	"time"
+	"context"
 )
 
 // Backoff is an interface that generates next backoff interval.
 type Backoff interface {
-	// Pause returns next backoff interval.
-	Pause() time.Duration
+	// Continue returns when to run the next backoff. The 1st call should return
+	// true immediately. The next and subsequent calls should return whether or
+	// not the next should be run after a backoff timeout.
+	Continue() bool
 }
 
 // BackoffPolicy is an interface that generates new Backoff instance.
 type BackoffPolicy interface {
 	// New returns new Backoff instance.
-	New() Backoff
+	New(ctx context.Context) Backoff
 }

examples/go.mod

@@ -8,3 +8,9 @@ require (
 	github.com/toga4/go-retryabletransport/adapter/github.com/googleapis/gax-go.v2/gaxbackoff v0.2.1
 	github.com/toga4/go-retryabletransport/adapter/github.com/lestrrat-go/backoff.v2/lestrratbackoff v0.2.1
 )
+
+replace (
+	github.com/toga4/go-retryabletransport => ../
+	github.com/toga4/go-retryabletransport/adapter/github.com/googleapis/gax-go.v2/gaxbackoff => ../adapter/github.com/googleapis/gax-go.v2/gaxbackoff
+	github.com/toga4/go-retryabletransport/adapter/github.com/lestrrat-go/backoff.v2/lestrratbackoff => ../adapter/github.com/lestrrat-go/backoff.v2/lestrratbackoff
+)