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

Long running durable timers #1390

Merged
merged 11 commits into from
Aug 12, 2020
Merged

Long running durable timers #1390

merged 11 commits into from
Aug 12, 2020

Conversation

bachuv
Copy link
Collaborator

@bachuv bachuv commented Jun 23, 2020

These changes allow durable timers to run longer than 7 days and resolves #14 .

Durable timer messages use queue messages which don't last longer than 7 days. To get around this, if a timer is scheduled for longer than 7 days, then the timer will go off every 3 days until the difference between the current time and the scheduled time is less than 3 days.

@bachuv bachuv requested review from cgillum and ConnorMcMahon June 23, 2020 00:28
@bachuv
Copy link
Collaborator Author

bachuv commented Jun 24, 2020

The tests, AzureStorage_TimerLimitExceeded_ThrowsException() and ThrowExceptionOnLongTimer(), fail now because of the changes that make durable timers last longer than 7 days. Would it be best to remove them and see if they can be replaced with a test that has a shorter interval time and max time?

Copy link
Member

@cgillum cgillum left a comment

Choose a reason for hiding this comment

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

This is looking great. Can we add a test or two to validate the behavior?

@MaximRouiller
Copy link

This is exactly what I needed. I have things to schedule that may be every 7 days, 14 days, 4 weeks, or 6 weeks.

I'd love for this to be merged yesterday 😛

@jedjohan
Copy link

Oh yes, was just implementing a complicated (to me) while loop to crank more than 7 days out of durable timers. This makes me really happy. Will there be a new maximum delay implemented ? Keep up the good work, thanks !

@bachuv
Copy link
Collaborator Author

bachuv commented Jul 16, 2020

Glad to hear that @jedjohan! No, there isn't a new maximum delay. If a timer is scheduled for longer than the maximum delay, then a new timer is scheduled every three days until the difference is less than three days. This ensures that users won't have to worry about staying within a maximum time.

Copy link
Contributor

@ConnorMcMahon ConnorMcMahon left a comment

Choose a reason for hiding this comment

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

A few pieces to cleanup, but the PR is shaping up nicely!

Copy link
Contributor

@ConnorMcMahon ConnorMcMahon left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link
Member

@cgillum cgillum left a comment

Choose a reason for hiding this comment

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

Just a few non-blocking suggestions, but I think this is ready to be merged when you feel ready!

test/FunctionsV2/TimerTests.cs Show resolved Hide resolved
test/FunctionsV2/TimerTests.cs Outdated Show resolved Hide resolved
@MaximRouiller
Copy link

Can't wait for this to be merged. 🤩

Copy link
Contributor

@ConnorMcMahon ConnorMcMahon left a comment

Choose a reason for hiding this comment

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

LGTM, just look into that last comment I made.

@bachuv bachuv merged commit a4f874e into dev Aug 12, 2020
@bachuv bachuv deleted the vabachu/durabletimers branch August 12, 2020 00:37
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.

Allow timers to last longer than 7 days
5 participants