Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Exponential backoff with max delay #226

Closed
foxware00 opened this issue Jun 3, 2019 · 3 comments
Closed

Exponential backoff with max delay #226

foxware00 opened this issue Jun 3, 2019 · 3 comments

Comments

@foxware00
Copy link
Contributor

The RetryConstraint enum exponential case seems it will infinitely grow exponentially.
My suggestion is to add an additional field for maxDelay such that.
.exponential(initial: TimeInterval, maxDelay: TimeInterval) to prevent it growing uncontrollably. Does this sound like something appropriate for this lib? Happy to construct a PR of you need some help.

let delay = info.currentRepetition == 1 ? initial : initial * pow(2, Double(info.currentRepetition - 1))
return min(maxDelay, delay)
@lucas34
Copy link
Owner

lucas34 commented Jun 3, 2019

Sounds good to me ! Can you send a PR ?

@foxware00
Copy link
Contributor Author

Of course. One issue I foresee is exponential enum won't allow default parameters and adding maxDelay would be a breaking change to the API.

I can think of a few options

  1. Add a new `expontentialWithMax(initial: TimeInterval, maxDelay: TimeInterval)
    verbose but non-breaking
  2. Change RetryConstraint to a struct to allow multiple factory methods and a private constructor to allow for the same method names, a non-breaking change but we wouldn't be able to use the switch statement inside retryJob
  3. Have factory methods inside the enum named as they are today and rename the enum cases to something else. This gives us two ways to create each case which might be confusing.
  4. Add maxDelay and cause a breaking change (worst option)

Any preference, thoughts or different solutions to the above?

@lucas34
Copy link
Owner

lucas34 commented Jun 4, 2019

In my opinion it's better to go with the most straight forward solution. The solution 1 looks good to me

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants