Add a Factory, FactoryFunc interfaces

[slnt]

Often I have components in my code take a BackOff. In some cases, there are concurrent pieces of code that need to be using similarly configured backoffs, but since backoffs are not safe for concurrent use, I'd need to pass the goroutine a new backoff.

It is difficult to do this when working with a BackOff directly, as the code doesn't know about then underlying implementation, and shouldn't. In this situation, a Factory would be of great use, as it would allow the code to create new backoffs on demand, and pass them into its goroutines, without having to know what the underlying implementation details of the specific backoff instance it has.

I have been using a very similar interface to this, and it has worked quite well for me. I think it would be beneficial to have it be part of the package itself, as then no separate backoffutil style package would be needed.

[coveralls]

Coverage increased (+1.2%) to 86.307% when pulling 1704c7e on slnt:slnt/backoff-factory into 31cc31b on cenkalti:v4.

[cenkalti]

Hi @slnt . This does not seem like a common use case to me. Why are you using backoffs concurrently? Can you tell more about your use case please?

[slnt]

Here's an example situation where we have found having this factory useful. foo and bar are placeholders for outgoing requests to other services.

type Foo struct {
	bf backoff.Factory
}

func (f *Foo) HandleRequest(...) error {
	var g errgroup.Group
	
	g.Go(func() error {
		b = bf.NewBackOff()
		backoff.Retry(func() error {
			return foo()
		}, b)
	})

	g.Go(func() error {
		b = bf.NewBackOff()
		backoff.Retry(func() error {
			return bar()
		}, b)
	})

	return g.Wait()
}

If Foo were to have just a BackOff here, instead of a Factory, there isn't a clean way to get a copy of the backoff. We could specify the backoff in HandleRequest, but then testing becomes difficult, as we'd be coupled to a specific backoff (eg ExponentialBackOff, but we'd want to use ZeroBackOff in our tests). The Factory makes this process a lot easier, and has proven very useful to us.

As I wrote the above it occurred to me that maybe we should have just defined all of our factories as func() backoff.BackOff, though I do still think being able to pass in ExponentialBackOff to something accepting a Factory might be a bit more "ergonomic".

[cenkalti]

I am trying to understand. How it is different from writing a function that returns a backoff instance?

type Foo struct {
	factory func() backoff.Backoff
}

func (f *Foo) HandleRequest(...) error {
	var g errgroup.Group
	
	g.Go(func() error {
		b := f.factory()
		backoff.Retry(func() error {
			return foo()
		}, b)
	})

	g.Go(func() error {
		b := f.factory()
		backoff.Retry(func() error {
			return bar()
		}, b)
	})

	return g.Wait()
}

[slnt]

I am trying to understand. How it is different from writing a function that returns a backoff instance?

It isn't, really, and I mentioned that at the bottom of my previous comment. The only difference is a slight "ergonomic" increase, I think.

foo := Foo{
	backoffFactory: backoff.NewExponentialBackOff(),
}

foo := Foo{
	backoffFactory: func() backoff.BackOff { return backoff.NewExponentialBackOff() },
}

The first is slightly nicer to write, but perhaps not worth adding more to the API.

[cenkalti]

I think the API is complex enough. I don't want to make it more complex by adding new methods/interfaces. I would like to reject this PR.