Rewrite util/retry as a for-loop

[tamird]

The final proposal! PTAL.

[tbg]

the error should be logged.

[tamird]

Done.

[tbg]

I like this approach, though the diff also showcases that it's much less elegant when the retryable code has a lot of return statements (which would prompt wrapping it in a function again, thus making the current solution the elegant one). But we can easily have both if we add just one small convenience method to retry which takes a func(Resettable) err where Resettable is nothing but Reset(), and functions can return a struct retry.AbortError{error} to indicate that they've run into something nasty to abort the loop (which then returns the wrapper error to the caller).
That would allow us to take whichever fits the situation best and with that addition I opt for going down the route presented in this PR.

[bdarnell]

Defers work at function scope, not block scope. With this pattern response bodies won't get closed until the last retry has completed. (and in go's http client, failing to close a response body prevents that connection from being reused). When a retry loop contains a defer, I think we need to refactor the body into a separate function.

[tamird]

in go's http client, failing to close a response body prevents that connection from being reused

Ah, I didn't know this (and was otherwise fine with closing at the very end of the function). I'll fix this up.

[petermattis]

I didn't look at the implementation in depth, but the for-loop API looks good to me.

[tamird]

But we can easily have both if we add just one small convenience method to retry which takes a func(Resettable) err where Resettable is nothing but Reset(), and functions can return a struct retry.AbortError{error} to indicate that they've run into something nasty to abort the loop (which then returns the wrapper error to the caller).

This sounds nice, but I think providing this choice is a mistake. It's just not the Go way; if a situation calls for this, this facility can trivially be written inline, and the sugar will just serve to obfuscate in this case.

[tbg]

Not sure the "Go way" mandates repeating yourself without a good reason, and it's a bit much to inline. Let's just see how far you get, no need to add it prematurely.

[tamird]

OK, this is finally complete. PTAL!

[tbg]

does MaxRetries=2 correspond to MaxAttempts=3?

[tamird]

Gah, no, it's off by one

[petermattis]

LGTM

[tamird]

Pushed some commits to address all the feedback. I'll squash and merge when this is green. @spencerkimball mind taking a look?

[spencerkimball]

It would be helpful to have a usage example in comments.