[utils] Fixed issue in clearing QueryCache
When trying to add an item in a full QueryCache, the cache tries to make room.
It tries to list and remove non-permanent items with a transient counter (number
of times it has been requested). However, there is a pathological case where
items could be non-permanent but still not have a transient counter because they
were added, but never requested. In some cases, the full cache could be flushed,
including the permanent items.
This changeset attempts to fix this issue by only dropping the non-permanent
items that did not hav a transient counter.
--- a/cubicweb/test/unittest_utils.py Mon Nov 13 16:08:58 2017 +0100
+++ b/cubicweb/test/unittest_utils.py Tue Apr 24 11:22:17 2018 +0200
@@ -109,6 +109,26 @@
'itemcount': 10,
'permanentcount': 5})
+ def test_clear_on_overflow(self):
+ """Tests that only non-permanent items in the cache are wiped-out on ceiling overflow
+ """
+ c = QueryCache(ceiling=10)
+ # set 10 values
+ for x in range(10):
+ c[x] = x
+ # arrange for the first 5 to be permanent
+ for x in range(5):
+ for r in range(QueryCache._maxlevel + 2):
+ v = c[x]
+ self.assertEqual(v, x)
+ # Add the 11-th
+ c[10] = 10
+ self.assertEqual(c._usage_report(),
+ {'transientcount': 0,
+ 'itemcount': 6,
+ 'permanentcount': 5})
+
+
class UStringIOTC(TestCase):
def test_boolean_value(self):
self.assertTrue(UStringIO())
--- a/cubicweb/utils.py Mon Nov 13 16:08:58 2017 +0100
+++ b/cubicweb/utils.py Tue Apr 24 11:22:17 2018 +0200
@@ -684,10 +684,20 @@
break
level = v
else:
- # we removed cruft but everything is permanent
+ # we removed cruft
if len(self._data) >= self._max:
- logger.warning('Cache %s is full.' % id(self))
- self._clear()
+ if len(self._permanent) >= self._max:
+ # we really are full with permanents => clear
+ logger.warning('Cache %s is full.' % id(self))
+ self._clear()
+ else:
+ # pathological case where _transient was probably empty ...
+ # drop all non-permanents
+ to_drop = set(self._data.keys()).difference(self._permanent)
+ for k in to_drop:
+ # should not be in _transient
+ assert k not in self._transient
+ self._data.pop(k, None)
def _usage_report(self):
with self._lock: