From c58aede9a818762b2a139de50e1d513ef556c54a Mon Sep 17 00:00:00 2001 From: Joseph Cumines Date: Wed, 24 Jan 2024 22:21:44 +1000 Subject: [PATCH] Remove unnecessary Retry.Last method, and improve the documentation --- retry.go | 35 +++++++++++++++-------------------- retry_test.go | 5 ++++- 2 files changed, 19 insertions(+), 21 deletions(-) diff --git a/retry.go b/retry.go index a8580d2..d1437fc 100644 --- a/retry.go +++ b/retry.go @@ -4,12 +4,10 @@ import ( "time" ) -// Retry extends Backoff to provide a simpler, higher-level API, for +// Retry extends [Backoff] to provide a simpler, higher-level API, for // implementing exponential backoff/retry logic. // -// It is not safe to use concurrently. Additionally, mutating the Backoff -// or Next fields directly may have unintended consequences, e.g. affecting the -// Last value. +// It is not safe to use concurrently. type Retry struct { // Backoff is the internal [Backoff]. Backoff *Backoff @@ -17,8 +15,7 @@ type Retry struct { // Next is the next allowed time. As that suggests, it is set on // [Retry.Allow], each time an attempt is allowed. // Another attempt will be allowed if Next is a time <= now, or the zero - // value. Mutating this field directly may cause unexpected behavior, - // use [Retry.Reset], instead. + // value. Next time.Time } @@ -27,7 +24,8 @@ var timeNow = time.Now // monkey patchable for testing // Allow acts as a limiter, returning the next allowed time, or `time.Time{}`, // if the next attempt should commence, inclusive of the first attempt. // If `time.Time{}` is returned, [Backoff.Duration] will be called, which will -// increment the [Backoff.Attempt] count, and then used to update [Retry.Next]. +// increment the [Backoff.Attempt] count. The value of [Retry.Next] will be +// updated to reflect the next allowed time (the returned duration, from now). func (x *Retry) Allow() time.Time { now := timeNow() if x.Next != (time.Time{}) && now.Before(x.Next) { @@ -37,20 +35,17 @@ func (x *Retry) Allow() time.Time { return time.Time{} } -// Reset resets the attempt count, and clears the next allowed time, removing -// any applied limit. -// This method will typically be called after each success. +// Reset resets the attempt count and clears the next allowed time, thereby +// removing any applied backoff delay. This method is typically invoked after +// a successful operation. By doing so, it ensures that if a subsequent +// failure occurs, the backoff timer restarts from the minimum duration. This +// approach is useful for scenarios where intermittent issues are resolved, +// allowing the system to promptly react to new errors without being delayed +// by the increased backoff time accumulated from previous failures. +// +// Clearing the Next field directly is an alternative, that will allow the next +// attempt immediately, without resetting the attempt count. func (x *Retry) Reset() { x.Backoff.Reset() x.Next = time.Time{} } - -// Last is the last time Allow was called. -// -// WARNING: Mutating the Backoff directly may cause this value to be incorrect. -func (x *Retry) Last() time.Time { - if x.Next == (time.Time{}) { - return time.Time{} - } - return x.Next.Add(-x.Backoff.ForAttempt(x.Backoff.Attempt() - 1)) -} diff --git a/retry_test.go b/retry_test.go index 8303a5b..e9c546e 100644 --- a/retry_test.go +++ b/retry_test.go @@ -122,7 +122,10 @@ func ExampleRetry_Allow_backoffBehavior() { attemptAllowAt := func(at time.Duration) { currentAttempt := retry.Backoff.Attempt() - 1 currentBackoff := retry.Backoff.ForAttempt(currentAttempt) - lastAttempt := retry.Last() + var lastAttempt time.Time + if retry.Next != (time.Time{}) { + lastAttempt = retry.Next.Add(-retry.Backoff.ForAttempt(retry.Backoff.Attempt() - 1)) + } now := time.Unix(0, int64(at)) timeNow = func() time.Time { return now } limitedUntil := retry.Allow()