Change in vdsm[master]: virt: add ExpiringCache

nsoffer at redhat.com nsoffer at redhat.com
Fri Feb 6 23:54:43 UTC 2015


Nir Soffer 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:

  ttl: items will expire ttl seconds after they were inserted.
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.
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.
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 access items, right?
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 never raise. Moving this discussion to __getitem__ docstring will make it more clear.
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 text should make it clear that if item is missing or expired, KeyError is raised. If the user want to detect that item was expired, it should catch ItemExpired.
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?
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__
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