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 67: self._working_on = tag Line 68: self._work_started_at = self._timefunc() Line 69: yield Line 70: self._work_started_at = None Line 71: self._working_on = None > I wouldn't necessarily consider different class of tasks in one pool. There no point in keeping 10s of libvirt connections stuck. We should close stuck connections and open new connections instead. Why do you think that connection pool will push the problem to libvirt? we can limit the number of connections and be nice to libvirt. Line 72: Line 73: Line 74: class Worker(TimeTrackingThread): Line 75: """ 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() > by connection pool you mean libvirt connections pool? Miachl, we don't want thread per vm, which is not scale-able as you just reported in the devel list. One stuck connection degrade the performance of the whole pool. So closing such connection si the nicest thing we can do, unless thats breaks libvirt badly :-) Line 85: Line 86: def run(self): Line 87: logging.info('worker %s starting', self.name) Line 88: 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() > I don't think there is a solution to cancel a stuck thread. IMO the only wa Michal, do you want to stop stats collections temporary for a vm that fails? If so, then vm object should keep its stats health, and schedule new stats calls according to its health - we don't need periodic tasks in this case. Each stat call can schedule the next one, preventing flooding in case libvirt does not respond well for queries on this vm. Line 85: Line 86: def run(self): Line 87: logging.info('worker %s starting', self.name) Line 88: -- 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
