#19914 closed Bug (fixed)
MemcachedCacheTests failing on pylibmc
Reported by: | Bas Peschier | Owned by: | Ed Morley |
---|---|---|---|
Component: | Core (Cache system) | Version: | dev |
Severity: | Normal | Keywords: | |
Cc: | emorley@… | Triage Stage: | Accepted |
Has patch: | no | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
Tests fail due to a bug in either pylibmc or libmemcached (detailts at pylibmc-issue: https://github.com/lericson/pylibmc/issues/114)
(sprint)mbdev001:tests basp$ python runtests.py --settings=test_sqlite_cache cache.MemcachedCacheTests Creating test database for alias 'default'... Creating test database for alias 'other'... ...........................E....... ====================================================================== ERROR: test_invalid_keys (regressiontests.cache.tests.MemcachedCacheTests) ---------------------------------------------------------------------- Traceback (most recent call last): File "/Users/basp/dev/repos/django/tests/regressiontests/cache/tests.py", line 963, in tearDown self.cache.clear() File "/Users/basp/dev/envs/sprint/lib/python2.7/site-packages/django/core/cache/backends/memcached.py", line 140, in clear self._cache.flush_all() SomeErrors: error 19 from flush_all: SUCCESS ---------------------------------------------------------------------- Ran 35 tests in 4.085s FAILED (errors=1) Destroying test database for alias 'default'... Destroying test database for alias 'other'...
settings:
CACHES = { 'default': { 'BACKEND': 'django.core.cache.backends.memcached.PyLibMCCache', 'LOCATION': '127.0.0.1:11211', } }
Change History (10)
comment:1 by , 12 years ago
Resolution: | → invalid |
---|---|
Status: | new → closed |
comment:2 by , 8 years ago
Cc: | added |
---|
This still reproduces with pylibmc 1.5.1 and libmemcached 1.0.8 (the latest for Ubuntu 14.04).
The pylibmc issue was closed as "expected" - perhaps we should just skip these tests on pylibmc?
test_incr_version (cache.tests.MemcachedCacheTests) ... ok test_invalid_keys (cache.tests.MemcachedCacheTests) ... ERROR test_long_timeout (cache.tests.MemcachedCacheTests) ... ERROR ERROR test_memcached_deletes_key_on_failed_set (cache.tests.MemcachedCacheTests) ... python: libmemcached/storage.cc:341: memcached_return_t memcached_send_ascii(memcached_st*, memcached_server_write_instance_st, const char*, size_t, const char*, size_t, time_t, uint32_t, uint64_t, bool, bool, memcached_storage_action_t): Assertion `memcached_failed(rc)' failed. Aborted
If not, then I think the ticket's resolution is best 'wontfix' rather than 'invalid' - though I mainly just wanted to update this ticket now to have something to point at when my PR fails some of the tests, to help prove that the failures were pre-existing.
comment:4 by , 8 years ago
So I've dug into this a bit more, there are actually two tests that are broken under pylibmc, and the above only covers the first.
Issues with test_invalid_keys:
This test tries to set two different types of invalid keys:
- a key that contains space characters
- another that exceeds the max allowed length
Both python-memcached and pylibmc handle (2) fine. However (1) is problematic with pylibmc, since it causes subsequent requests to the server (ie teardown and later tests in the suite) to fail for the next few seconds.
The cause of this is roughly:
- python-memcached validates key names before sending, whereas by pylibmc doesn't by default (or more precisely, libmemcached doesn't by default)
- therefore for pylibmc the invalid key name gets sent to the server, rather than being rejected client-side
- this appears to expose some memcached server/libmemcache bug that isn't pylibmc's fault (though I'm hoping for clarification, see: https://github.com/lericson/pylibmc/issues/114#issuecomment-242929115)
Possible workarounds are:
- not set keys names containing spaces (ie skip the test)
- enable pylibmc's client side key validation passing
verify_keys
withinbehaviors
(just for this specific test) - use the binary protocol (since it's not affected)
In addition, since the bug only affects half of test_invalid_keys()
, we could split the test in two, to reduce the amount we skip/test in a non-standard configuration.
Note: The issue presents itself in two forms depending on libmemcached/memcached server versions. On Ubuntu 14.04 an assertion occurs in libmemcached, which causes the whole test run to abort (example in comment 2 above). Whereas on Ubuntu 16.04 a handful of exceptions are seen and the rest of the tests complete fine. I'm pretty sure the assertion *is* something pylibmc should be handling better, for which I've filed:
https://github.com/lericson/pylibmc/issues/218
(though even with that fixed, the underlying ~server bug still needs to be worked around)
Issues with test_memcached_deletes_key_on_failed_set:
With the problematic test above skipped, the one remaining failure under pylibmc is:
ERROR: test_memcached_deletes_key_on_failed_set (cache.tests.MemcachedCacheTests) ---------------------------------------------------------------------- Traceback (most recent call last): File "/home/vagrant/src/_todo/django/tests/cache/tests.py", line 1233, in test_memcached_deletes_key_on_failed_set cache.set('small_value', large_value) File "/home/vagrant/src/_todo/django/django/core/cache/backends/memcached.py", line 83, in set if not self._cache.set(key, value, self.get_backend_timeout(timeout)): Error: error 37 from memcached_set: SUCCESS
This is due to difference in behaviour between clients when the *value* is too long.
Compare:
- When the *key* is too long:
- python-memcached:
memcache.MemcachedKeyLengthError: Key length is > 250
- pylibmc:
ValueError: key length 251 too long, max is 250
- python-memcached:
- When the *value* is too long:
- python-memcached: Returns successfully but didn't set the key-value (this is by design)
- pylibmc 1.5.1:
pylibmc.Error: error 37 from memcached_set: SUCCESS
- pylibmc master:
pylibmc.TooBig
Possible options:
- just add a try-except around the
.set()
for both python-memcached and pylibmc - differentiate between the two, and only expect exceptions for the pylibmc case (either by just catching them, or by enforcing using
assertRaises
)
comment:5 by , 8 years ago
Resolution: | invalid |
---|---|
Status: | closed → new |
Triage Stage: | Unreviewed → Accepted |
Reopening, since a test suite should be green on master, even if the underlying library is at fault.
comment:6 by , 8 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
This is fixed by the PR in #27132.
For the first failing test in comment 4, I went with option (a) after splitting the test into two.
For the second test, I also went with its option (a).
For more details, see the comments added to the tests in the PR:
https://github.com/django/django/pull/7168
It seems to me that this is a bug with pylibmc, right? I'm not sure Django could do anything here. I'm going to close this, please feel free to reopen if the bug's actually in Django.