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

Performance improvements for checking schedule intervals #276

Merged
merged 1 commit into from
Jul 25, 2018

Conversation

drcapulet
Copy link
Contributor

When working with large schedules of jobs (~100) that run infrequently (weekly or less) our Sidekiq server can take 8 to 10 minutes to start due to the check to ensure jobs don't run more frequently than the scheduler.

For Cron jobs, Fugit supports up to 1 second cron resolution, the entire check is skipped if the scheduler frequency is below that. Then based on a simple heuristic if the minimum delta between the 2nd, 3rd, and 4th runtimes is greater than a week the brute_frequency method is used. Otherwise the old enumeration logic over next_time_from is used.

I added simple specs to ensure adding both frequent and infrequent cron additions take less than a second (on master the infrequent spec takes 12 seconds).

This reduces the schedule parsing time for us to just under 20s.

@jmettraux jmettraux self-assigned this Jul 25, 2018
@jmettraux
Copy link
Owner

Hello,

this looks excellent. Give me time to fully wake up and integrate that.

Thanks!

@RyanBalfanz
Copy link

I approve of this change.

@jmettraux
Copy link
Owner

May I get a Square sticker?

@jmettraux jmettraux merged commit eeebb18 into jmettraux:master Jul 25, 2018
jmettraux added a commit that referenced this pull request Jul 25, 2018
@jmettraux
Copy link
Owner

@drcapulet What if I introduce a scheduler option to tell it not to check the frequency at all (trusting you the user to feed it with schedules that match)?

jmettraux added a commit that referenced this pull request Jul 25, 2018
jmettraux added a commit to floraison/fugit that referenced this pull request Jul 26, 2018
jmettraux added a commit to floraison/fugit that referenced this pull request Jul 29, 2018
jmettraux added a commit that referenced this pull request Jul 29, 2018
@jmettraux
Copy link
Owner

jmettraux commented Jul 29, 2018

@drcapulet

Is using Fugit::Cron#rough_frequency instead of your logic OK for you? It should cut down the #check_frequency time even more.

Thanks in advance.

@drcapulet
Copy link
Contributor Author

@jmettraux Ah yeah - looks great, thanks!

@drcapulet drcapulet deleted the alexc-delta branch July 31, 2018 17:07
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.

3 participants