Nir Soffer has posted comments on this change.
Change subject: schedule: Introduce scheduling library ......................................................................
Patch Set 14:
(3 comments)
http://gerrit.ovirt.org/#/c/29607/14/lib/vdsm/schedule.py File lib/vdsm/schedule.py:
Line 102: """ Line 103: Schedule callable to be called after delay seconds on the scheduler Line 104: thread. Line 105: Line 106: Callable must not block or take excessive time to complete. It it does
typo: It it does
Done Line 107: not finish quickly, it may delay other scheduled calls on the scheduler Line 108: thread. Line 109: Line 110: Returns a ScheduledCall that may be canceled if callable was not called
Line 164: Line 165: def _cancel_calls(self): Line 166: # Help the garbage collector by breaking reference cycles Line 167: with self._cond: Line 168: for deadline, call in self._calls:
why not just empty self._calls, like this: self._calls = []
Typically an object schedule a call and keep for one of its methods, and keep a reference to the scheduler. So we have this cycle:
obj -> scheduler -> calls -> scheduled_call -> method -> obj
cancel() remove method from the loop, breaking the cycle. Line 169: call.cancel() Line 170: Line 171: Line 172: class ScheduledCall(object):
Line 200: Line 201: Line 202: # Sentinel for marking calls as invalid. Callable so we can invalidate a call Line 203: # in a thread safe manner without locks. Line 204: def _INVALID():
Why uppercase?
This is a constant - you are not supposed to use this function. It is a function only to allow _execute to run without any racy checks.
For example, we could (wrongly) implement this with None:
def _execute(self): if self._callable is not None: self._callable()
But it is racy. Using a function solve this race, but is function is not part of the api of this module.
I think that nothing can be more clear than:
self._callable = _INVALID