Skip to content

Commit

Permalink
Handle overflow cases (sethvargo#13)
Browse files Browse the repository at this point in the history
  • Loading branch information
sethvargo committed Feb 10, 2022
1 parent 7a0bb9f commit 009eed1
Show file tree
Hide file tree
Showing 5 changed files with 66 additions and 12 deletions.
4 changes: 2 additions & 2 deletions backoff.go
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ func WithCappedDuration(cap time.Duration, next Backoff) Backoff {
return 0, true
}

if val > cap {
if val <= 0 || val > cap {
val = cap
}
return val, false
Expand All @@ -127,7 +127,7 @@ func WithMaxDuration(timeout time.Duration, next Backoff) Backoff {
return 0, true
}

if val > diff {
if val <= 0 || val > diff {
val = diff
}
return val, false
Expand Down
21 changes: 15 additions & 6 deletions backoff_exponential.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package retry

import (
"context"
"math"
"sync/atomic"
"time"
)
Expand All @@ -11,17 +12,19 @@ type exponentialBackoff struct {
attempt uint64
}

// Exponential is a wrapper around Retry that uses an exponential backoff. It's
// very efficient, but does not check for overflow, so ensure you bound the
// retry. It panics if the given base is less than zero.
// Exponential is a wrapper around Retry that uses an exponential backoff. See
// NewExponential.
func Exponential(ctx context.Context, base time.Duration, f RetryFunc) error {
return Do(ctx, NewExponential(base), f)
}

// NewExponential creates a new exponential backoff using the starting value of
// base and doubling on each failure (1, 2, 4, 8, 16, 32, 64...), up to max.
// It's very efficient, but does not check for overflow, so ensure you bound the
// retry. It panics if the given base is less than 0.
//
// Once it overflows, the function constantly returns the maximum time.Duration
// for a 64-bit integer.
//
// It panics if the given base is less than zero.
func NewExponential(base time.Duration) Backoff {
if base <= 0 {
panic("base must be greater than 0")
Expand All @@ -34,5 +37,11 @@ func NewExponential(base time.Duration) Backoff {

// Next implements Backoff. It is safe for concurrent use.
func (b *exponentialBackoff) Next() (time.Duration, bool) {
return b.base << (atomic.AddUint64(&b.attempt, 1) - 1), false
next := b.base << (atomic.AddUint64(&b.attempt, 1) - 1)
if next <= 0 {
atomic.AddUint64(&b.attempt, ^uint64(0))
next = math.MaxInt64
}

return next, false
}
18 changes: 18 additions & 0 deletions backoff_exponential_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package retry_test

import (
"fmt"
"math"
"reflect"
"sort"
"testing"
Expand Down Expand Up @@ -48,6 +49,23 @@ func TestExponentialBackoff(t *testing.T) {
8192 * time.Nanosecond,
},
},
{
name: "overflow",
base: 100_000 * time.Hour,
tries: 10,
exp: []time.Duration{
100_000 * time.Hour,
200_000 * time.Hour,
400_000 * time.Hour,
800_000 * time.Hour,
1_600_000 * time.Hour,
math.MaxInt64,
math.MaxInt64,
math.MaxInt64,
math.MaxInt64,
math.MaxInt64,
},
},
}

for _, tc := range cases {
Expand Down
17 changes: 13 additions & 4 deletions backoff_fibonacci.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package retry

import (
"context"
"math"
"sync/atomic"
"time"
"unsafe"
Expand All @@ -13,16 +14,20 @@ type fibonacciBackoff struct {
state unsafe.Pointer
}

// Fibonacci is a wrapper around Retry that uses a Fibonacci backoff. It panics
// if the given base is less than zero.
// Fibonacci is a wrapper around Retry that uses a Fibonacci backoff. See
// NewFibonacci.
func Fibonacci(ctx context.Context, base time.Duration, f RetryFunc) error {
return Do(ctx, NewFibonacci(base), f)
}

// NewFibonacci creates a new Fibonacci backoff using the starting value of
// base. The wait time is the sum of the previous two wait times on each failed
// attempt (1, 1, 2, 3, 5, 8, 13...). It panics if the given base is less than
// zero.
// attempt (1, 1, 2, 3, 5, 8, 13...).
//
// Once it overflows, the function constantly returns the maximum time.Duration
// for a 64-bit integer.
//
// It panics if the given base is less than zero.
func NewFibonacci(base time.Duration) Backoff {
if base <= 0 {
panic("base must be greater than 0")
Expand All @@ -40,6 +45,10 @@ func (b *fibonacciBackoff) Next() (time.Duration, bool) {
currState := (*state)(curr)
next := currState[0] + currState[1]

if next <= 0 {
return math.MaxInt64, false
}

if atomic.CompareAndSwapPointer(&b.state, curr, unsafe.Pointer(&state{currState[1], next})) {
return next, false
}
Expand Down
18 changes: 18 additions & 0 deletions backoff_fibonacci_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package retry_test

import (
"fmt"
"math"
"reflect"
"sort"
"testing"
Expand Down Expand Up @@ -60,6 +61,23 @@ func TestFibonacciBackoff(t *testing.T) {
610 * time.Nanosecond,
},
},
{
name: "overflow",
base: 100_000 * time.Hour,
tries: 10,
exp: []time.Duration{
100_000 * time.Hour,
200_000 * time.Hour,
300_000 * time.Hour,
500_000 * time.Hour,
800_000 * time.Hour,
1_300_000 * time.Hour,
2_100_000 * time.Hour,
math.MaxInt64,
math.MaxInt64,
math.MaxInt64,
},
},
}

for _, tc := range cases {
Expand Down

0 comments on commit 009eed1

Please sign in to comment.