Change in vdsm[master]: netlink: Introduce socket pool
danken at redhat.com
danken at redhat.com
Wed Feb 19 11:43:40 UTC 2014
Dan Kenigsberg has posted comments on this change.
Change subject: netlink: Introduce socket pool
......................................................................
Patch Set 3:
(3 comments)
Nir, were do you see a thread-safety issue during init?
For your second point, I do not see a real contention issue, at least not one that is justifying creation of multiple socket pools.
http://gerrit.ovirt.org/#/c/24603/3/lib/vdsm/netlink.py
File lib/vdsm/netlink.py:
Line 109: @contextmanager
Line 110: def socket(self):
Line 111: """Takes a netlink socket from the pool and returns it afterwards."""
Line 112: with self._semaphore:
Line 113: sock = self._sockets.pop()
it would be even nicer to _open_socket() only now, and only if pop() raised KeyError - then there's no need for _get_sockets() during __init__.
Line 114: try:
Line 115: yield sock
Line 116: finally:
Line 117: self._sockets.add(sock)
Line 124: def _get_pool():
Line 125: """Get (and create if necessary) the link pool."""
Line 126: global _pool
Line 127: if _pool is None:
Line 128: if _pool_lock.acquire(False):
I find the standard idiom of (quick check, lock, safe check) clearer
if _pool is None:
with _pool_lock:
if _pool is None:
_pool = ...
return _pool
it is equivalent, and we have it spread through our code.
Line 129: try:
Line 130: _pool = NLSocketPool(_POOL_SIZE)
Line 131: finally:
Line 132: _pool_lock.release()
Line 132: _pool_lock.release()
Line 133: else:
Line 134: with _pool_lock:
Line 135: pass
Line 136: return _pool
> This lazy creation of the pool is the same code use to lazy-populate the po
In my opinion, it is actually better to have the standard singlton pattern external to the class definition.
Also, I find it nicer to avoid taking resources implicitly during import - I had an unpleasant reminder few months ago: http://gerrit.ovirt.org/22322
Line 137:
Line 138:
Line 139: def _open_socket():
Line 140: """Returns an open netlink socket."""
--
To view, visit http://gerrit.ovirt.org/24603
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I657ac3d3e0c2661ce73bdef9aa807ead888a42eb
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Antoni Segura Puimedon <asegurap at redhat.com>
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer at redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
More information about the vdsm-patches
mailing list