Change in vdsm[master]: virt: add ExpiringCache

fromani at redhat.com fromani at redhat.com
Mon Feb 9 08:46:20 UTC 2015


Francesco Romani has posted comments on this change.

Change subject: virt: add ExpiringCache
......................................................................


Patch Set 8:

(8 comments)

http://gerrit.ovirt.org/#/c/36716/8/vdsm/virt/utils.py
File vdsm/virt/utils.py:

Line 49:     is attached to each key. Thread safe.
Line 50: 
Line 51:     Parameters:
Line 52:     ttl: validity of the keys in `clock' time units since
Line 53:          the time of their inserion in the ExpiringCache.
> This can be simplified and be more clear:
agreed and done
Line 54:     clock: time.time() or monotonic_time()-like callable.
Line 55: 
Line 56:     NOTE: The time interval of key validity is [T, T+ttl)
Line 57:     where `T' is the clock() time of insertion.


Line 53:          the time of their inserion in the ExpiringCache.
Line 54:     clock: time.time() or monotonic_time()-like callable.
Line 55: 
Line 56:     NOTE: The time interval of key validity is [T, T+ttl)
Line 57:     where `T' is the clock() time of insertion.
> This is not clear. I think the description of ttl is good enough.
Removed.
Line 58: 
Line 59:     Expired keys are transparently removed on the first attempt
Line 60:     to access them, using get or __getitem__. In that case,
Line 61:     ItemExpired is raised; in case of a missing key, KeyError


Line 55: 
Line 56:     NOTE: The time interval of key validity is [T, T+ttl)
Line 57:     where `T' is the clock() time of insertion.
Line 58: 
Line 59:     Expired keys are transparently removed on the first attempt
> transparently can be removed.
Done
Line 60:     to access them, using get or __getitem__. In that case,
Line 61:     ItemExpired is raised; in case of a missing key, KeyError
Line 62:     is raised as for regular dict()s.
Line 63:     """


Line 56:     NOTE: The time interval of key validity is [T, T+ttl)
Line 57:     where `T' is the clock() time of insertion.
Line 58: 
Line 59:     Expired keys are transparently removed on the first attempt
Line 60:     to access them, using get or __getitem__. In that case,
> "using get or __getitem__" does not add anything. There is no other way to 
Not at present moment, so removed.
Line 61:     ItemExpired is raised; in case of a missing key, KeyError
Line 62:     is raised as for regular dict()s.
Line 63:     """
Line 64:     def __init__(self, ttl, clock=monotonic_time):


Line 57:     where `T' is the clock() time of insertion.
Line 58: 
Line 59:     Expired keys are transparently removed on the first attempt
Line 60:     to access them, using get or __getitem__. In that case,
Line 61:     ItemExpired is raised; in case of a missing key, KeyError
> The discussion about ItemExpired is relevant only to __getitem__, get wil n
Done
Line 62:     is raised as for regular dict()s.
Line 63:     """
Line 64:     def __init__(self, ttl, clock=monotonic_time):
Line 65:         self._ttl = ttl


Line 58: 
Line 59:     Expired keys are transparently removed on the first attempt
Line 60:     to access them, using get or __getitem__. In that case,
Line 61:     ItemExpired is raised; in case of a missing key, KeyError
Line 62:     is raised as for regular dict()s.
> People reading this may miss the fact that ItemExpired is a KeyError. The t
Agreed and done.
Line 63:     """
Line 64:     def __init__(self, ttl, clock=monotonic_time):
Line 65:         self._ttl = ttl
Line 66:         self._clock = clock


Line 90:             del self._items[key]
Line 91: 
Line 92:     # private
Line 93: 
Line 94:     def _get_unexpired(self, key):
> This name is strange. Maybe _get_live?
get_live is probably a little nicer. Changed.
Line 95:         """
Line 96:         Automatically cleanups expired keys.
Line 97:         Raises KeyError if a key was not found.
Line 98:         Raises ItemExpired if an expired key is cleaned up.


Line 94:     def _get_unexpired(self, key):
Line 95:         """
Line 96:         Automatically cleanups expired keys.
Line 97:         Raises KeyError if a key was not found.
Line 98:         Raises ItemExpired if an expired key is cleaned up.
> This documentation belongs to __getitem__
Done
Line 99:         """
Line 100:         now = self._clock()
Line 101:         with self._lock:
Line 102:             expiration, value = self._items[key]


-- 
To view, visit http://gerrit.ovirt.org/36716
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I51e38cea6a23b7abe2375780e6ece0ff90831b6d
Gerrit-PatchSet: 8
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani <fromani at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani at redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer 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