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

[GRM-287] - Tasks scheduled by the scheduler seem to either behave erratically or don't run at all. #5

Merged
merged 1 commit into from
Aug 9, 2022

Conversation

rocos-jeremy-scott
Copy link

@rocos-jeremy-scott rocos-jeremy-scott commented Aug 9, 2022

GRM-287

The reason this is necessary is because once the first clock synchronisation callback is fired, the timer will become locked to a single point in time. The break statement will cause the inner for loop to complete, as shown below, but will not cause any update to the cached now value (without the addition of the new now line).
https://github.com/team-rocos/cron/blob/966ed6863c9e1311d6e9e9244a21b06e257850ba/cron.go#L354-L359

This means that each time we recalculate the time because of a time synchronisation, the now value will always be the same and since the job time is not changing, the two times will always return the same positive difference, never getting any closer to each other. As can be seen on the code block below- in particular line 312. This obviously doesn't mean the timer will never fire, it just means that the remaining time to start a task must be less than the duration until the next time synchronisation.
https://github.com/team-rocos/cron/blob/966ed6863c9e1311d6e9e9244a21b06e257850ba/cron.go#L302-L315

Copy link

@edwinhayes edwinhayes left a comment

Choose a reason for hiding this comment

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

LGTM.

@edwinhayes edwinhayes merged commit f287ec2 into v3 Aug 9, 2022
@edwinhayes edwinhayes removed the request for review from mohsenpashna August 9, 2022 21:48
@edwinhayes edwinhayes deleted the grm287_scheduler_erratic_task_behaviour branch August 9, 2022 22:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants