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

force TimerService to run the handler in alignment with the beginning of the second #6302

Merged
merged 3 commits into from
Oct 6, 2022

Conversation

tbenr
Copy link
Contributor

@tbenr tbenr commented Oct 6, 2022

as discussed here: #6245 (comment)

as per my checks, time drift is always corrected back to be close to the tick of the second

Documentation

  • I thought about documentation and added the doc-change-required label to this PR if updates are required.

Changelog

  • I thought about adding a changelog entry, and added one if I deemed necessary.

@zilm13
Copy link
Contributor

zilm13 commented Oct 6, 2022

If I understand everything correctly, we have 999ms* seconds delay on start here in the worst case, but we run ticks every 500ms, so we could change it to have 499ms* in the worst case. Am I right?

@zilm13
Copy link
Contributor

zilm13 commented Oct 6, 2022

Also when system clock will be changed by some utility few ms forward or backwards, will we handle it?

@tbenr
Copy link
Contributor Author

tbenr commented Oct 6, 2022

If I understand everything correctly, we have 999ms* seconds delay on start here in the worst case, but we run ticks every 500ms, so we could change it to have 499ms* in the worst case. Am I right?

yes that's correct

Also when system clock will be changed by some utility few ms forward or backwards, will we handle it?

very good question..........

@tbenr
Copy link
Contributor Author

tbenr commented Oct 6, 2022

@zilm13 completely removed the startup delay :-)

@tbenr
Copy link
Contributor Author

tbenr commented Oct 6, 2022

for the second question, I think quartz calculates next fire based on current time so it should react accordingly remaining aligned, but should be proven

Copy link
Contributor

@ajsutton ajsutton left a comment

Choose a reason for hiding this comment

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

Well that was unexpectedly simple. LGTM.

@tbenr tbenr merged commit ce5d3e7 into Consensys:master Oct 6, 2022
@tbenr tbenr deleted the ensure_onTick_alligned_with_second branch October 7, 2022 12:06
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