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

Avoid busy waiting in ioloop when system time is skipped forward #1290

Merged
merged 1 commit into from Feb 11, 2015

Conversation

Projects
None yet
2 participants
@nosyjoe
Copy link
Contributor

commented Dec 29, 2014

  • when the system time changes, jumps into the future make the _schedule_next method do a busy wait.
    • on slow machines (RPi), jumps of a few months into the future can block the loop for a few minutes
      => added a check for big differences in current system time and the current value of the next scheduled timeout

On a first boot of an older RPi image, tornado sometimes starts before date&time gets updated through NTP, hence blocking the ioloop for several minutes.

@bdarnell

This comment has been minimized.

Copy link
Member

commented Jan 11, 2015

Instead of an arbitrary threshold, I wonder if it would be better to do something like this:

self._next_timeout += self.callback_time_sec
if self._next_timeout < current_time:
    self._next_timeout += math.ceil((current_time - self._next_timeout) / self.callback_time_sec) * self.callback_time_sec)

If I've got the math right, this is a constant-time version of the current semantics (including the fact that next_timeout always advances by a multiple of callback_time, although I don't know how important it is to preserve this behavior).

Also, systems that experience large NTP adjustments may be better served by configuring the IOLoop to use time.monotonic() instead of time.time().

@bdarnell bdarnell added the ioloop label Jan 12, 2015

@nosyjoe

This comment has been minimized.

Copy link
Contributor Author

commented Jan 13, 2015

ok, I will try your solution and report back!

@nosyjoe nosyjoe force-pushed the nosyjoe:branch4.0-mrbeam branch 4 times, most recently from 880152c to 70ef0b1 Feb 10, 2015

modified method _schedule_next of PeriodicCallback to handle sudden c…
…hanges of the system time differently:

 * calculating next timeout value directly while advancing by a multiple of callback_time
 * when the system time changes, jumps into the future make the _schedule_next method do a busy wait.
 * on slow machines (RPi), jumps of a few months into the future can block the loop for a few minutes
=> added a check for big differences in current system time and the current value of the next scheduled timeout

On a first boot of an older RPi image, tornado sometime starts before the date&time were updated through NTP, hence blocking the ioloop for several minutes.

@nosyjoe nosyjoe force-pushed the nosyjoe:branch4.0-mrbeam branch from 70ef0b1 to a5e7ee7 Feb 10, 2015

@nosyjoe

This comment has been minimized.

Copy link
Contributor Author

commented Feb 10, 2015

I used the code you suggested with small changes. As in the original code, the _next_timeout is increased when it is smaller or equal to the current time.

I made the code calculate the next_timeout directly by tweaking your formula a bit. The new value of next_timeout is larger than current_time and smaller or equal to current_time + callback_time_sec.

bdarnell added a commit that referenced this pull request Feb 11, 2015

Merge pull request #1290 from nosyjoe/branch4.0-mrbeam
Avoid busy waiting in ioloop when system time is skipped forward

@bdarnell bdarnell merged commit 9b35208 into tornadoweb:master Feb 11, 2015

1 check passed

continuous-integration/travis-ci The Travis CI build passed
Details

foosel added a commit to foosel/OctoPrint that referenced this pull request Mar 16, 2015

Monkey patching tornado to backport tornadoweb/tornado#1290
Should hopefully help with freezing/blocking issues upon first connect with the net on the Pi, more tests needed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.