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

nsoffer at redhat.com nsoffer at redhat.com
Thu Jan 29 17:05:05 UTC 2015


Nir Soffer has posted comments on this change.

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


Patch Set 32:

(1 comment)

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

Line 195:         finally:
Line 196:             self._busy = False
Line 197:             if self._discarded:
Line 198:                 raise _WorkerDiscarded()
Line 199:             if discard is not None:
> I don't really like this check. I think it is better and slightly cleaner t
Isn't this little over-engineered? Why do you care about checking discard?

We can clean this up by moving discard to the object:

    self._discard_after(timeout)
    try:
        callable()
    except ...
        log...
    finally:
        self._cancel_discard()

    def _start_discard_timeout(self, timeout):
        self._discard = ...
 
    def _cancel_discard(self):
        if self._discard is not None:
            self._discard.cancel()
            self._discard = None

This can also be easier to debug because we can access the timeout object of the blocked thread (assuming we get into the process using manhole or something similar).
Line 200:                 discard.cancel()
Line 201: 
Line 202:     def _discard_after(self, timeout):
Line 203:         if timeout is not None:


-- 
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: 32
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani <fromani at redhat.com>
Gerrit-Reviewer: Antoni Segura Puimedon <toni at midokura.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