Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make the first exp of retry with backoff be 0 #29

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 15 additions & 11 deletions retry/retry.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,38 +21,44 @@ type Config struct {
Times int
}

const (
defaultWaitBase = 20 * time.Millisecond
defaultTimesToRetry = 3
)

func (c *Config) defaults() {
if c.WaitBase <= 0 {
c.WaitBase = 20 * time.Millisecond
c.WaitBase = defaultWaitBase
}

// TODO: Allow -1 for forever retries?
if c.Times <= 0 {
c.Times = 3
c.Times = defaultTimesToRetry
}
}

// New returns a new retry ready executor, the execution will be retried the number
// of times specificed on the config (+1, the original execution that is not a retry).
// of times specified on the config (+1, the original execution that is not a retry).
func New(cfg Config) goresilience.Runner {
return NewMiddleware(cfg)(nil)
}

// NewMiddleware returns a new retry middleware, the execution will be retried the number
// of times specificed on the config (+1, the original execution that is not a retry).
// of times specified on the config (+1, the original execution that is not a retry).
func NewMiddleware(cfg Config) goresilience.Middleware {
cfg.defaults()

return func(next goresilience.Runner) goresilience.Runner {
next = goresilience.SanitizeRunner(next)
random := rand.New(rand.NewSource(time.Now().UnixNano()))

// Use the algorithms for jitter and backoff.
// https://aws.amazon.com/es/blogs/architecture/exponential-backoff-and-jitter/
return goresilience.RunnerFunc(func(ctx context.Context, f goresilience.Func) error {
var err error
metricsRecorder, _ := metrics.RecorderFromContext(ctx)

// Start the attemps. (it's 1 + the number of retries.)
// Start the attempts (it's 1 + the number of retries.)
for i := 0; i <= cfg.Times; i++ {
// Only measure the retries.
if i != 0 {
Expand All @@ -69,16 +75,14 @@ func NewMiddleware(cfg Config) goresilience.Middleware {

// Apply Backoff.
// The backoff is calculated exponentially based on a base time
// and the attemp of the retry.
// and the attempt of the retry.
if !cfg.DisableBackoff {
exp := math.Exp2(float64(i + 1))
exp := math.Exp2(float64(i))
waitDuration = time.Duration(float64(cfg.WaitBase) * exp)
// Round to millisecs.
waitDuration = waitDuration.Round(time.Millisecond)

// Apply "full jitter".
random := rand.New(rand.NewSource(time.Now().UnixNano()))
waitDuration = time.Duration(float64(waitDuration) * random.Float64())

waitDuration = waitDuration.Round(time.Millisecond)
}

time.Sleep(waitDuration)
Expand Down
129 changes: 63 additions & 66 deletions retry/retry_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package retry_test
import (
"context"
"errors"
"fmt"
"testing"
"time"

Expand All @@ -13,16 +12,16 @@ import (
"github.com/slok/goresilience/retry"
)

var err = errors.New("wanted error")
var err = errors.New("error wanted")

type counterFailer struct {
notFailOnAttemp int
timesExecuted int
type eventuallySucceed struct {
successfulExecutionAttempt int
timesExecuted int
}

func (c *counterFailer) Run(ctx context.Context) error {
func (c *eventuallySucceed) Run(_ context.Context) error {
c.timesExecuted++
if c.timesExecuted == c.notFailOnAttemp {
if c.timesExecuted == c.successfulExecutionAttempt {
return nil
}

Expand All @@ -45,7 +44,7 @@ func TestRetryResult(t *testing.T) {
Times: 3,
},
getF: func() goresilience.Func {
c := &counterFailer{notFailOnAttemp: 4}
c := &eventuallySucceed{successfulExecutionAttempt: 4}
return c.Run
},
expErr: nil,
Expand All @@ -58,7 +57,7 @@ func TestRetryResult(t *testing.T) {
Times: 3,
},
getF: func() goresilience.Func {
c := &counterFailer{notFailOnAttemp: 5}
c := &eventuallySucceed{successfulExecutionAttempt: 5}
return c.Run
},
expErr: err,
Expand All @@ -67,17 +66,15 @@ func TestRetryResult(t *testing.T) {

for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
assert := assert.New(t)

cmd := retry.New(test.cfg)
err := cmd.Run(context.TODO(), test.getF())
exec := retry.New(test.cfg)
err := exec.Run(context.TODO(), test.getF())

assert.Equal(test.expErr, err)
assert.Equal(t, test.expErr, err)
})
}
}

var notime = time.Time{}
var noTime = time.Time{}

// patternTimer will store the execution time passed
// (in milliseconds) between the executions.
Expand All @@ -86,26 +83,28 @@ type patternTimer struct {
waitPattern []time.Duration
}

func (p *patternTimer) Run(ctx context.Context) error {
func (p *patternTimer) Run(_ context.Context) error {
now := time.Now()

if p.prevExecution != notime {
if p.prevExecution == noTime {
p.prevExecution = now
} else {
durationSince := now.Sub(p.prevExecution)
p.prevExecution = now
p.waitPattern = append(p.waitPattern, durationSince.Round(time.Millisecond))
}

return errors.New("wanted error")
return errors.New("error wanted")
}

func TestConstantRetry(t *testing.T) {
tests := []struct {
name string
cfg retry.Config
expWaitPattern []time.Duration // use ints so we can used rounded.
expWaitPattern []time.Duration
}{
{
name: "A retry executions without backoff should be at constant rate. (2ms, 9 retries)",
name: "A retry executions without backoff should be at constant rate. (10ms, 4 retries)",
cfg: retry.Config{
WaitBase: 10 * time.Millisecond,
DisableBackoff: true,
Expand All @@ -119,7 +118,7 @@ func TestConstantRetry(t *testing.T) {
},
},
{
name: "A retry executions without backoff should be at constant rate (5ms, 4 retries)",
name: "A retry executions without backoff should be at constant rate (30ms, 2 retries)",
cfg: retry.Config{
WaitBase: 30 * time.Millisecond,
DisableBackoff: true,
Expand All @@ -134,62 +133,60 @@ func TestConstantRetry(t *testing.T) {

for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
assert := assert.New(t)

exec := retry.New(test.cfg)
pt := &patternTimer{}
exec.Run(context.TODO(), pt.Run)
_ = exec.Run(context.TODO(), pt.Run)

assert.InEpsilonSlice(test.expWaitPattern, pt.waitPattern, 0.08)
assert.Equal(t, test.expWaitPattern, pt.waitPattern)
})
}
}

func TestBackoffJitterRetry(t *testing.T) {
tests := []struct {
name string
cfg retry.Config
times int
}{
{
name: "Multiple retry executions with backoff should have all different wait times.",
cfg: retry.Config{
t.Run("Multiple retry executions with backoff should have all different wait times.", func(t *testing.T) {
// We do several iterations of the same process to reduce the probability that
// the property we are testing for was obtained by chance.
const numberOfIterations = 3
for i := 0; i < numberOfIterations; i++ {
pt := &patternTimer{}
exec := retry.New(retry.Config{
WaitBase: 50 * time.Millisecond,
DisableBackoff: false,
Times: 2,
},
times: 3,
},
}

for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
assert := assert.New(t)

occurrences := map[string]struct{}{}

// Let's do N iterations of the same process.
for i := 0; i < test.times; i++ {
runner := &patternTimer{}
exec := retry.New(test.cfg)
exec.Run(context.TODO(), runner.Run)

// Check that the wait pattern results (diferent from 0)
// are different, this guarantees that at least we are waiting
// different durations..
for _, dur := range runner.waitPattern {
if dur == 0 {
continue
}

// Round to microseconds.
roundedDur := dur.Round(time.Microsecond)
key := fmt.Sprintf("%s", roundedDur)
_, ok := occurrences[key]
assert.False(ok, "using a exponential jitter a iteration wait time should be different from another, this iteration wait time already appeared (%s)", key)
occurrences[key] = struct{}{}
})
_ = exec.Run(context.TODO(), pt.Run)
occurrences := make(map[time.Duration]struct{})

// Check that the wait pattern results (different from 0)
// are different, this guarantees that at least we are waiting
// different durations.
for _, dur := range pt.waitPattern {
if dur == 0 {
continue
}

_, ok := occurrences[dur]
assert.False(t, ok, "Using exponential jitter, attempts' waiting times should be different from one another. This attempt's waiting time has already appeared before (%s)", dur)
occurrences[dur] = struct{}{}
}
})
}
}
})

t.Run("Multiple retry executions with backoff should have the first waiting time to be no more than the base waiting time configured.", func(t *testing.T) {
// We do several iterations of the same process to reduce the probability that
// the property we are testing for was obtained by chance.
const numberOfIterations = 3
for i := 0; i < numberOfIterations; i++ {
pt := &patternTimer{}
cfg := retry.Config{
WaitBase: 50 * time.Millisecond,
DisableBackoff: false,
Times: 2,
}
exec := retry.New(cfg)
_ = exec.Run(context.TODO(), pt.Run)

assert.True(t, pt.waitPattern[0] <= cfg.WaitBase)
}
})
}