mailgun / expiringdict

Dictionary with auto-expiring values for caching purposes.
Apache License 2.0
344 stars 76 forks source link

Key expiration does not work with Python 3.5 #16

Open gwillem opened 8 years ago

gwillem commented 8 years ago

I guess the inherited OrderedDict implementation has changed.

I do not use ttl or iteration, so I have fixed my use case with an extra try around the popitem(last=False) in __setitem__.

======================================================================
ERROR: tests.expiringdict_test.test_basics
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/willem/.virtualenvs/expiringdict/lib/python3.5/site-packages/nose/case.py", line 198, in runTest
    self.test(*self.arg)
  File "/home/willem/git/expiringdict/tests/expiringdict_test.py", line 37, in test_basics
    d['e'] = 'z'
  File "/home/willem/git/expiringdict/expiringdict/__init__.py", line 72, in __setitem__
    self.popitem(last=False)
KeyError: 'a'

======================================================================
ERROR: tests.expiringdict_test.test_iter
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/willem/.virtualenvs/expiringdict/lib/python3.5/site-packages/nose/case.py", line 198, in runTest
    self.test(*self.arg)
  File "/home/willem/git/expiringdict/tests/expiringdict_test.py", line 78, in test_iter
    eq_([k for k in d.values()], [])
  File "/home/willem/git/expiringdict/expiringdict/__init__.py", line 124, in values
    for key in self:
RuntimeError: OrderedDict mutated during iteration

======================================================================
ERROR: tests.expiringdict_test.test_ttl
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/willem/.virtualenvs/expiringdict/lib/python3.5/site-packages/nose/case.py", line 198, in runTest
    self.test(*self.arg)
  File "/home/willem/git/expiringdict/tests/expiringdict_test.py", line 101, in test_ttl
    Mock(return_value=('x', 10**9))):
  File "/home/willem/.virtualenvs/expiringdict/lib/python3.5/site-packages/mock/mock.py", line 1460, in __enter__
    setattr(self.target, self.attribute, new_attr)
TypeError: can't set attributes of built-in/extension type 'collections.OrderedDict'

----------------------------------------------------------------------
kaniini commented 8 years ago

We see this on our internal fork of expiringdict as well. It is a regression introduced by the "optimized" version of OrderedDict introduced in Python 3.5.

The regression is reported here: http://bugs.python.org/issue27275

In the meantime you can force Python to give you a copy of the pure python version by doing this to import OrderedDict:

import sys

# Python 3.5 has a broken 'optimised' version of OrderedDict which can corrupt itself at high load.
if sys.version_info.major == 3 and sys.version_info.minor == 5:
    from test import support

    py_coll = support.import_fresh_module('collections', blocked=['_collections'])
    OrderedDict = py_coll.OrderedDict
else:
    from collections import OrderedDict

To be perfectly honest, I don't know why this has been included. I see no notable performance change between the C and Python versions. On our codebase, 10k+ iterations/sec of expiringdict accesses we see no change in performance between the C and Python version of OrderedDict.

Something something premature optimization is the root of all evil.

allardhoeve commented 7 years ago

any news on this PR?

kaniini commented 7 years ago

Python 3.6 resolves the regression, but honestly we don't even need OrderedDict there for this particular use.

christophlingg commented 7 years ago

I wanted to use this project, but it's abandoned and unmaintained. I ended up using https://pythonhosted.org/cachetools/#cachetools.TTLCache instead.

and-semakin commented 4 years ago

Greetings from 2020!