Francesco Romani has posted comments on this change.

Change subject: schedule: Introduce scheduling libary
......................................................................


Patch Set 4:

(2 comments)

http://gerrit.ovirt.org/#/c/29607/4/lib/vdsm/schedule.py
File lib/vdsm/schedule.py:

Line 77:         t = threading.Thread(target=self._run, name=name)
Line 78:         t.daemon = True
Line 79:         t.start()
Line 80: 
Line 81:     def schedule(self, delay, callable):
> Was callle before, changed to callable as Saggi suggested. This hides the b
Fair enough.
Line 82:         """
Line 83:         Schedule callable to be called after delay seconds on the 
scheduler
Line 84:         thread.
Line 85: 


Line 82:         """
Line 83:         Schedule callable to be called after delay seconds on the 
scheduler
Line 84:         thread.
Line 85: 
Line 86:         Callable must not block or take excessive time to complete. It 
it does
> You cannot call this if your call blocks. When used for sampling, you will 
Fine, but then what's the advantage of having a private executor thread inside 
the Scheduler?

Wouldn't that better if the Scheduler receives a reference to an executor, like 
for example a thread pool or anything which implements a given simple interface 
and just submits the callable to it?

This way is even simpler and more regular: scheduler just schedules and does no 
more; threadpool just executes and does no more.
Line 87:         not finish quickly, it may delay other scheduled calls on the 
scheduler
Line 88:         thread.
Line 89: 
Line 90:         Returns a ScheduledCall that may be canceled if callable was 
not called


-- 
To view, visit http://gerrit.ovirt.org/29607
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie3764806d93bd37c3b5924080eb5ae4d29e4f4e0
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer <[email protected]>
Gerrit-Reviewer: Adam Litke <[email protected]>
Gerrit-Reviewer: Allon Mureinik <[email protected]>
Gerrit-Reviewer: Antoni Segura Puimedon <[email protected]>
Gerrit-Reviewer: Dan Kenigsberg <[email protected]>
Gerrit-Reviewer: Federico Simoncelli <[email protected]>
Gerrit-Reviewer: Francesco Romani <[email protected]>
Gerrit-Reviewer: Martin Sivák <[email protected]>
Gerrit-Reviewer: Michal Skrivanek <[email protected]>
Gerrit-Reviewer: Nir Soffer <[email protected]>
Gerrit-Reviewer: Saggi Mizrahi <[email protected]>
Gerrit-Reviewer: Yoav Kleinberger <[email protected]>
Gerrit-Reviewer: [email protected]
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
vdsm-patches mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Reply via email to