add jitter to exponential backoff wrapper

[dancompton]

I was thinking something like the following. Did this quickly so be wary

[dancompton]

Looks like transient failure in build

[matryer]

Looks like a code problem:

backoff/backoff.go:4: imported and not used: "fmt"

Maybe run go fmt and goimports or something on your code? You'll catch these little annoying bugs.

[HeavyHorst]

The nsq implementation calls

err = backoff.Do(1*time.Second, 10*time.Minute, 0, func() error {
        return t.ConnectConsumer(consumer)
})

Notice that maxCalls is zero, which means retry forever in the original implementation. Your implementation returns immediately with a nil error

for rc := 0; rc < maxTries; rc++

I think thats why the test hangs forever.

[matryer]

Definitely shouldn’t be retrying forever.

On 3 Aug 2017, at 19:46, Rene Kaufmann notifications@github.com wrote:

The nsq implementation calls

err = backoff.Do(1time.Second, 10time.Minute, 0, func() error { return t.ConnectConsumer(consumer) }) Notice that maxCalls is zero, which means retry forever in the original implementation. Your implementation returns immediately with a nil error

for rc := 0; rc < maxTries; rc++ I think thats why the test hangs forever.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/matryer/vice/pull/31#issuecomment-320056785, or mute the thread https://github.com/notifications/unsubscribe-auth/AAGNG5E2m76QPmKvBVoqC5W8opJAlA3tks5sUhWGgaJpZM4OraRQ.

[HeavyHorst]

@dan-compton can you change

err = backoff.Do(1*time.Second, 10*time.Minute, 0, func() error {
        return t.ConnectConsumer(consumer)
})

to

err = backoff.Do(1*time.Second, 10*time.Minute, 10, func() error {
        return t.ConnectConsumer(consumer)
})

in nsq.go line 113

[dancompton]

Sure, although should we us NSQ defaults here? Default max back off is 2 minutes and max retries is 5 IIRC.

Also, are we comfortable with the behavior around max and min calls? For example passing ...0 and having no function call occur? Do we need to wrap errors for exceeded retry count in a sentinel context or expose another means of determining whether or not the retry count was exceeded?