Nir Soffer has posted comments on this change. Change subject: threadpool: add a thread pool with watchdog ......................................................................
Patch Set 8: (3 comments) http://gerrit.ovirt.org/#/c/29191/8/lib/threadpool/worker.py File lib/threadpool/worker.py: Line 79: """ Line 80: def __init__(self, work_queue): Line 81: super(Worker, self).__init__() Line 82: self.daemon = True Line 83: self._work_queue = work_queue > To implement the periodic tasks. Scheduling is not the responsibility of a threadpool. You are mixing unrelated stuff. If you like to scheduled stuff, add a scheduler that wakes up when the next job is ready and run or submit it to a thread pool dedicated to running such tasks. Where do we need such scheduling in vdsm? Line 84: self._stop = threading.Event() Line 85: Line 86: def run(self): Line 87: logging.info('worker %s starting', self.name) Line 80: def __init__(self, work_queue): Line 81: super(Worker, self).__init__() Line 82: self.daemon = True Line 83: self._work_queue = work_queue Line 84: self._stop = threading.Event() > Your solution to stop threads is more general and nicer, so I'll switch to Connection pool is probably what we need, and each connection need a thread since you cannot submit multiple requests on the same connection without getting a response for the previous - right? This is similar to database connection pool or http connection pool. Line 85: Line 86: def run(self): Line 87: logging.info('worker %s starting', self.name) Line 88: Line 115: yield item Line 116: except schedqueue.NotYet: Line 117: yield None Line 118: except schedqueue.Empty: Line 119: yield None > Nice supervisor code, I'll try to come up with something like that (and aga ok, we need priodic tasks - so lets design a scheduler for these - not a thread pool. Since these tasks are periodic, we have a very clear timeout for them - if a call that must run every 15 seconds did not run yet because it is waiting in the queue, or because libvirt connection is stuck, we should cancel this call. Otherwise we will have huge queue of tasks that grows and grows without limit, giving irrelevant results (task ran minutes after its scheduled run time). This will also make sure that we will never consume too much cpu - if we cannot get task to run because of slow response time and too many task to run, we will run only some of the tasks. I built similar schedulers, using a heapq (https://docs.python.org/2.6/library/heapq.html) and poll, polling until the next scheduled call should run. Using such scheduler, we can schedule a cancel operation on a tak when we start the task, and we don't need to supervise the running task. Again, this is not related to a threadpool. A threapool can be used to run scheduled tasks concurrently. Line 120: Line 121: def _do_work(self, item): Line 122: """ Line 123: process a single work unit. -- To view, visit http://gerrit.ovirt.org/29191 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ic06da1ba57868dc2c7db67a1868ad10087a1cff2 Gerrit-PatchSet: 8 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani <[email protected]> Gerrit-Reviewer: Antoni Segura Puimedon <[email protected]> Gerrit-Reviewer: Dan Kenigsberg <[email protected]> Gerrit-Reviewer: Dima Kuznetsov <[email protected]> Gerrit-Reviewer: Federico Simoncelli <[email protected]> Gerrit-Reviewer: Francesco Romani <[email protected]> Gerrit-Reviewer: Michal Skrivanek <[email protected]> Gerrit-Reviewer: Nir Soffer <[email protected]> Gerrit-Reviewer: Saggi Mizrahi <[email protected]> Gerrit-Reviewer: Vinzenz Feenstra <[email protected]> Gerrit-Reviewer: Yaniv Bronhaim <[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
