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

Retry Settings #14

Closed
wants to merge 35 commits into from
Closed

Retry Settings #14

wants to merge 35 commits into from

Conversation

tooolbox
Copy link
Contributor

This PR adds settings to control retry behavior in Dalga. Depends on #13

Fixed Schedule Intervals

When FixedIntervals is enabled in the config, the next_sched ("next scheduled run") column tracks the job's cadence irrespective of retries, to prevent schedule drift.

Concrete example:

For example, if you have a P1W job scheduled for Saturday, and it fails and retries 3 times over the next couple of days, it should still fire on the next Saturday.

Frequency, Limit, Backoff

TODO

@tooolbox
Copy link
Contributor Author

Couple points:

  1. I notice that my commits are accumulating, so the chain gets longer with each PR! I'm not entirely sure what to do about that; I'm guessing it's because you're doing squash & merge. If it's problematic on your end, I can newly branch off your master and cherry-pick my own commits or something.
  2. Although the Dalga readme states that retries use exponential backoff, I don't actually see that in the code! Since I need to implement some kind of backoff strategy, I'm wondering if you have a preference. Should I pull in https://github.com/cenkalti/backoff ?

@cenkalti
Copy link
Owner

1. I notice that my commits are accumulating, so the chain gets longer with each PR!  I'm not entirely sure what to do about that; I'm guessing it's because you're doing squash & merge.  If it's problematic on your end, I can newly branch off your `master` and cherry-pick my own commits or something.

I am squash&merging for cleaner history. That would be great if you can rebase or cherry-pick your commits to latest master for next PRs.

2. Although the Dalga readme states that retries use exponential backoff, I don't actually see that in the code!  Since I need to implement some kind of backoff strategy, I'm wondering if you have a preference.  Should I pull in https://github.com/cenkalti/backoff ?

Yes, it was there at one point but I have removed it on a refactor. I forgot to remove from README. We can put it back again with github.com/cenkalti/backoff. Please let me know if you need help with that.

@tooolbox tooolbox mentioned this pull request Aug 23, 2020
@cenkalti cenkalti mentioned this pull request Aug 24, 2020
@tooolbox tooolbox mentioned this pull request Aug 24, 2020
@tooolbox
Copy link
Contributor Author

We can put it back again with github.com/cenkalti/backoff. Please let me know if you need help with that.

It would be great to have your help! I opened #16 that is just a single commit added to your latest master, and introduces next_sched, which is something of an independent change. If you want to add exponential backoff on top of that (or even without it, I suppose that would work) that would be great.

I thought about my use case, and I realize that having a retry count is not important. I plan to track retries at the receiver and return a 200 if I want to terminate the retry behavior. So since already have retry_interval as a config setting, just adding exponential backoff should get me most of the way there. The only other aspect is knowing at the receiver which iteration it is, which sort of ties in with idempotency, and I opened up #17 for that.

So yeah, I'm going to close this in favor of #16 and if you have time to hit exponential backoffs that would be excellent.

@tooolbox tooolbox closed this Aug 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants