juju/retry take 2 - looping

Tim Penhey tim.penhey at canonical.com
Tue Oct 25 20:42:36 UTC 2016


Some comments first, then addressing issues:

I thought it might help if I walked you through my thought process of 
making this change.

* the purpose of the retry package is to provide a way to retry actions 
that may fail until they succeed, or some predetermined limit is hit
* the way that failure is idiomatically handled is with an error
* I wanted the interface to be very simple to use, and hard to get wrong
* it should work inline in a simple for loop
* the library should make the simple case very simple

var result Foo
var err error
for loop := retry.Loop(spec); loop.Next(err); {
      result, err = SomeFunc(blah)
}
if err != nil {
      // what ever
}
// continue using result

What about fatal errors?

var result Foo
var err error
for loop := retry.Loop(spec); loop.Next(err); {
      result, err = SomeFunc(blah)
      if isFatalErr(err) {
          break
      }
}
if err != nil {
    // what ever
}
// continue using result


On 21/10/16 06:33, roger peppe wrote:
> On 20 October 2016 at 16:30, Katherine Cox-Buday
> <katherine.cox-buday at canonical.com> wrote:
>> John Meinel <john at arbash-meinel.com> writes:
>>
>>> As commented on the pull request, passing 'err' into Next() initially feels weird, but
>>> it allows a big improvement to whether the loop exited because we ran out of
>>> retries, or it exited because the request succeeded. (loop.Error()
>>> tells us either way.)
>>
>> I don't understand why the retry logic has the concern of why the loop exits (i.e. Next(error)).

The rationale behind that is to make the the library handle the 80% case 
easily. The main reason of passing the error into Next is so you don't 
have to do the check and explicit break within the loop.

>> The motivation of moving towards loops was so that the concern of what's being retried is brought back to be inline with the surrounding code. Having the retry mechanism inspecting errors doesn't fall in line with that goal.
>>
>> In my mind, any retry solutions only needs to cover two things:
>> 1. Provide a primitive that will delay a for iteration in a controlled way.
>> 2. Be preemptable.
>>
>> Everything else is the logic of the thing you're retrying, including why it eventually succeeded/failed.

Here is were we disagree slightly. I also think that very high on the 
list are:
    * easy to get right
    * hard to get wrong

> That's well put, and fits well with my thoughts too, thanks.
>
> See also Pawel Stolowski's comment on
> https://github.com/juju/retry/pull/5. He's from the Snappy
> team who have also been trying to move towards using a standard retry mechanism.
> There are many possible reasons for wanting to retry - error values
> are just one possibility.

Sure, but this could be trivially handled by assigning something to err 
(consider if we made a public error in the retry package for this):

var result Foo
var err error
for loop := retry.Loop(spec); loop.Next(err); {
    result, err = SomeFunc(blah)
    if err == nil && resultNotGoodEnough(result) {
      err = retry.ErrTryAgain
    }
}
if err != nil {
    // what ever
}
// continue using result

Tim



More information about the Juju-dev mailing list