Commit 4ef05b9e authored by Julien Muchembled's avatar Julien Muchembled

client: fix item eviction from cache, which could break loading from storage

`ClientCache._oid_dict` shall not have empty values. For a given oid, when the
last item is removed from the cache, the oid must be removed as well to free
memory. In some cases, this was not done.

A consequence of this bug is the following exception:

    ERROR ZODB.Connection Couldn't load state for 0x02d1e1e4
    Traceback (most recent call last):
      File "ZODB/Connection.py", line 860, in setstate
        self._setstate(obj)
      File "ZODB/Connection.py", line 901, in _setstate
        p, serial = self._storage.load(obj._p_oid, '')
      File "neo/client/Storage.py", line 82, in load
        return self.app.load(oid)[:2]
      File "neo/client/app.py", line 358, in load
        self._cache.store(oid, data, tid, next_tid)
      File "neo/client/cache.py", line 228, in store
        prev = item_list[-1]
    IndexError: list index out of range
parent 2b3993f1
Pipeline #3683 skipped
...@@ -102,6 +102,12 @@ class ClientCache(object): ...@@ -102,6 +102,12 @@ class ClientCache(object):
if item is head: if item is head:
break break
def _remove_from_oid_dict(self, item):
item_list = self._oid_dict[item.oid]
item_list.remove(item)
if not item_list:
del self._oid_dict[item.oid]
def _add(self, item): def _add(self, item):
level = item.level level = item.level
try: try:
...@@ -126,10 +132,7 @@ class ClientCache(object): ...@@ -126,10 +132,7 @@ class ClientCache(object):
self._history_size += 1 self._history_size += 1
if self._max_history_size < self._history_size: if self._max_history_size < self._history_size:
self._remove(head) self._remove(head)
item_list = self._oid_dict[head.oid] self._remove_from_oid_dict(head)
item_list.remove(head)
if not item_list:
del self._oid_dict[head.oid]
def _remove(self, item): def _remove(self, item):
level = item.level level = item.level
...@@ -165,7 +168,7 @@ class ClientCache(object): ...@@ -165,7 +168,7 @@ class ClientCache(object):
if head.level or head.counter: if head.level or head.counter:
self._add(head) self._add(head)
else: else:
self._oid_dict[head.oid].remove(head) self._remove_from_oid_dict(head)
break break
def _load(self, oid, before_tid=None): def _load(self, oid, before_tid=None):
...@@ -247,7 +250,7 @@ class ClientCache(object): ...@@ -247,7 +250,7 @@ class ClientCache(object):
head.level = 0 head.level = 0
self._add(head) self._add(head)
else: else:
self._oid_dict[head.oid].remove(head) self._remove_from_oid_dict(head)
if self._size <= max_size: if self._size <= max_size:
return return
head = next head = next
......
  • @jm this patch comes without a test, though imho it should not be hard to replay a scenario where items are evicted. Please do not forget to add corresponding test.

    /cc @vpelletier

Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment