Change in vdsm[master]: executor: introduce the executor library

fromani at redhat.com fromani at redhat.com
Fri Aug 8 14:49:09 UTC 2014


Francesco Romani has posted comments on this change.

Change subject: executor: introduce the executor library
......................................................................


Patch Set 21:

(3 comments)

http://gerrit.ovirt.org/#/c/29191/21/lib/vdsm/executor.py
File lib/vdsm/executor.py:

Line 69:         with self._lock:
Line 70:             self._running = False
Line 71:             self._tasks.clear()
Line 72:             for i in range(self._workers_count):
Line 73:                 self._tasks.put((_STOP, 0))
> Lets use None here, which mean no timeout. It does not matter because the c
Done
Line 74:             workers = tuple(self._workers) if wait else ()
Line 75:         for worker in workers:
Line 76:             try:
Line 77:                 worker.join()


Line 210:     def __init__(self, max_tasks):
Line 211:         self._max_tasks = max_tasks
Line 212:         self._tasks = collections.deque()
Line 213:         # deque is thread safe - we can append and pop from both ends without
Line 214:         # additioanl locking. We need this condition only for waiting.
> Lets add a url to the docs about this:
Done, and corrected a small typo along the way.
Line 215:         self._cond = threading.Condition(threading.Lock())
Line 216:         self._waiters = 0
Line 217: 
Line 218:     def put(self, task):


Line 229:                 return self._tasks.popleft()
Line 230:             except IndexError:
Line 231:                 with self._cond:
Line 232:                     self._waiters += 1
Line 233:                     self._cond.wait()
> To avoid the race explained above:
It seems to me you are right, so I'm doing the change you suggested
Line 234:                     self._waiters -= 1
Line 235: 
Line 236:     def clear(self):


-- 
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: 21
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani <fromani at redhat.com>
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Dima Kuznetsov <dkuznets at redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce at redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani at redhat.com>
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek at redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer at redhat.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi at redhat.com>
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr at redhat.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei at redhat.com>
Gerrit-Reviewer: automation at ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list