Stabilization of the period of ioloop.PeriodicCallback. #320

Merged
merged 1 commit into from Aug 3, 2011

2 participants

@jfisteus

The execution of callbacks by ioloop.PeriodicCallback was not trully periodic because the timeout for the following event was computed by adding callback_time to the actual time the previous callback finished. This piece of code demonstrates that:

import time
import tornado.ioloop

def callback():
    print time.time()
    for i in range(1, 1000000):
        a = 2 * i

sched = tornado.ioloop.PeriodicCallback(callback, 3000)
sched.start()
tornado.ioloop.IOLoop.instance().start()

The period should be 3 seconds, but it is slightly more, due to the time the callback takes to execute plus the delay between the instant the callback is planned to be executed and the actual time it is executed.

As a user of PeriodicCallback, after having read its documentation, I expected callbacks to be trully periodic, being callback_time their period. The commit I propose schedules the next timeout with respect to when the previous timeout was scheduled. This makes it trully periodic.

Note, however, that my solution has a problem when the time it takes the callback to execute is systematically greater than callback_time: the queue of scheduled events would grow indefinitely. It is clearly a problem in the client code, but if we wanted to protect PeriodicCallback against that, I beleive this other patch would at least keep the size of the queue under control by dropping missed periods:

     def _schedule_next(self):
         if self._running:
-            self._next_timeout += self.callback_time / 1000.0
+            current_time = time.time()
+            while self._next_timeout < current_time:
+                self._next_timeout += self.callback_time / 1000.0
             self.io_loop.add_timeout(self._next_timeout, self._run)

I haven't included it in the commit because I'm not really sure this case should be addressed.

@jfisteus jfisteus Stabilization of the period of ioloop.PeriodicCallback.
Previously, the timeout for the following event was computed by adding
callback_time to the actual time the callback finished, which caused
calls to the callback not being trully periodic.

This commit makes callback_time be a stable period by scheduling the
next timeout callback_time after the previous timeout was scheduled.
c4c5960
@bdarnell
tornadoweb member

I do think it's worth addressing missed callbacks. It's true that if you consistently can't keep up then you're in trouble no matter what, but when transient events (network outage, system clock changed, etc) cause scheduled callbacks to be missed it's better to skip them and get back on the scheduled period than to try and "catch up" by running everything we missed. I'll merge this change and add the overrun check.

@bdarnell bdarnell merged commit c4c5960 into tornadoweb:master Aug 3, 2011
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment