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

Support custom delays for automatic retries #931

Merged
merged 8 commits into from
Jul 11, 2017
Merged

Support custom delays for automatic retries #931

merged 8 commits into from
Jul 11, 2017

Conversation

aidmsu
Copy link
Contributor

@aidmsu aidmsu commented Jul 7, 2017

This PR adds ability to specify custom delays for retries via AutomaticRetryAttribute.

Specify custom delays by an integer array.

You can set a delay for a job:

// All delays are the same and equal 10 seconds.
[AutomaticRetry(DelaysInSeconds = new [] { 10 })]
public void SomeJob() { ... }

// First delay is 1 second, second delay is 5 seconds, other delays are 10 seconds.
[AutomaticRetry(DelaysInSeconds = new [] { 1, 5, 10 })]
public void SomeJob() { ... }

Also it's possible set a delay for all jobs:

GlobalJobFilters.Filters.Add(new AutomaticRetryAttribute { DelaysInSeconds = new { 1,5,100 }});

Specify custom delay by a function.

If you want to use complex algorithm to define a delay by a attempt you should set AutomaticRetryAttribute.DelayInSecondsByAttemptFunc property:

// Delays will be 1, 4, 9 seconds and so on.
var func = attempt => attempt * attempt;
GlobalJobFilters.Filters.Add(new AutomaticRetryAttribute { DelayInSecondsByAttemptFunc = func});

Behaviour

If you specifty both DelaysInSeconds and DelayInSecondsByAttemptFunc then only DelaysInSeconds will be used.

If given DelayInSecondsByAttemptFunc throws excecption then the default function will be used.

@codecov-io
Copy link

codecov-io commented Jul 7, 2017

Codecov Report

Merging #931 into dev will increase coverage by 0.34%.
The diff coverage is 88.88%.

Impacted Files Coverage Δ
src/Hangfire.Core/AutomaticRetryAttribute.cs 86.66% <88.88%> (+6.38%) ⬆️
src/Hangfire.Core/DashboardOptions.cs 90.9% <0%> (+90.9%) ⬆️

@aidmsu aidmsu changed the title Support custom delays for automatic retries [WIP] Support custom delays for automatic retries Jul 10, 2017
@aidmsu aidmsu changed the title [WIP] Support custom delays for automatic retries Support custom delays for automatic retries Jul 10, 2017
@aidmsu
Copy link
Contributor Author

aidmsu commented Jul 10, 2017

I reconsidered behaviour in case a user code threw exception inside AutomaticRetryAttribute. I came to a conclusion it's better to rethrow the exception and as a consequence to apply Failed state to a job. Main reason is a user changes default behavior of AutomaticRetryAttribute for some reason and it may be critical for her not to use default behaviour.

@odinserj
Copy link
Member

What if 0 value is given? AFAIK currently it will be created in the ScheduledState with the DateTime.UtcNow argument. I think we can create it using the EnqueuedState instead to avoid unnecessary state changes. What do you think?

@aidmsu
Copy link
Contributor Author

aidmsu commented Jul 10, 2017

@odinserj You are right, if 0 value is given we should apply EnqueuedState instead of ScheduledState. I'll fix it.

@odinserj
Copy link
Member

Great @aidmsu! Pay special attention to the retries counter, there may be dragons 😉

@odinserj odinserj self-requested a review July 10, 2017 17:32
Copy link
Member

@odinserj odinserj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't get used to reviews, waiting for EnqueuedState transition on zero delay.

@aidmsu
Copy link
Contributor Author

aidmsu commented Jul 11, 2017

@odinserj I fixed it. AutomaticRetryAttribute applies EnqueuedState instead of ScheduledState when the delay is zero.

@odinserj odinserj merged commit e524433 into HangfireIO:dev Jul 11, 2017
@hQst
Copy link

hQst commented Nov 24, 2017

This feature is nice but you can't define a DelayInSecondsByAttemptFunc in the GlobalFilters and put the Attribute on the Job. Because then the DefaultDelayInSecondsByAttemptFunc will be used. And due to the nature of an attribute you can't defined the Func in the attribute.

@camiller2
Copy link

camiller2 commented Jun 1, 2018

@odinserj / any other admin: Is there an update on whether (and if so when) this will be merged into master and released? Looking at https://github.com/HangfireIO/Hangfire/compare/dev it appears that this has not been released, but I could be wrong.

@mauriciojsola
Copy link

Hey @odinserj, any ETA for this feature to be released? Being able to adjust the retry delay times is really useful on some scenarios. Thanks!

@camiller2
Copy link

camiller2 commented Jun 11, 2018

I've created a version of AutomaticRetryAttribute based off of this version that allows DefaultDelayInSecondsByAttemptFunc (and all the other properties of the attribute) to be configured at runtime on an attribute. This is possible via an autofac-hangfire filter provider integration I wrote and a ConfigKey. Assuming this integration has been enabled, the API looks like this:

[AutomaticRetryV2(ConfigKey = MyClass.ConfigKey, OnAttemptsExceeded = AttemptsExceededAction.Delete)]
        Task MyMethod(long someId);

// In your autofac registration:
builder.Register(cc => new Dictionary<string, AutomaticRetryV2AttributeConfig>
        {
            {
                MyClass.ConfigKey,
                new AutomaticRetryV2AttributeConfig
                {
                    DelaysInSeconds = _settings.MyMethodDelayBetweenAttempts.HasValue
                        ? new[] {_settings.MyMethodDelayBetweenAttempts.Value}
                        : null,
                    Attempts = _settings.MyMethodRetryAttempts
                }
            }
        })
        .As<IDictionary<string, AutomaticRetryV2AttributeConfig>>()
        .SingleInstance();

Let me know if anybody's interested, and I'd be happy to create a PR for the attribute update. I'm planning on creating a PR for the required autofac-hangfire filter provider integration in https://github.com/HangfireIO/Hangfire.Autofac before too long.

rlaneve pushed a commit to enthusem/Hangfire that referenced this pull request Jul 11, 2018
* Support custom delays for retries

* Specify delayInSeconds as int array

* User code errors lead to exception in AutomaticRetryAttribute

* Fix DelaysInSeconds property documentation

* Remove one empty line

* Apply Enqueue state for retry job if delay is zero

* Add more tests for AutomaticRetryAttribute

* Update AutomaticRetryAttribute tests
@zuckerthoben
Copy link

Any updates on this?

@tho-mas
Copy link

tho-mas commented Sep 5, 2018

It would be very nice if this PR can be included in the next release.

@aprooks
Copy link

aprooks commented Sep 19, 2018

It's published in 1.7-beta.

@SimonOrdo
Copy link

Am I reading this correctly, that there is no way to specify a delay algorithm on a per-job basis? I.e. it's either global, for all jobs, or static?

I need to implement an exponential back-off with jitter delay algorithm, but it should only be applied to specific jobs.

@HristoDimitrovBede
Copy link

@SimonOrdo I am looking for the same. I think it isn't implemented yet.

@jonathancounihan
Copy link

@SimonOrdo , @HristoDimitrovBede did you ever manage to implement an exponential back-off with jitter delay algorithm for specific jobs?
Thanks.

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

Successfully merging this pull request may close these issues.