[Django] #19914: MemcachedCacheTests failing on pylibmc
Django
-------------------------------------+--------------------
Reporter: bpeschier | Owner: nobody
Type: Bug | Status: new
Component: Core (Cache system) | Version: master
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+--------------------
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',
}
}
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/19914>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: bpeschier | Owner: nobody
Type: Bug | Status: closed
Component: Core (Cache system) | Version: master
Keywords: | Triage Stage:
Has patch: 0 | Unreviewed
Needs tests: 0 | Needs documentation: 0
Easy pickings: 0 | Patch needs improvement: 0
| UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by jacob):
* status: new => closed
* needs_better_patch: => 0
* resolution: => invalid
* needs_tests: => 0
* needs_docs: => 0
Comment:
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/19914#comment:1>
Django
Reporter: bpeschier | Owner: nobody
Type: Bug | Status: closed
Component: Core (Cache system) | Version: master
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by edmorley):
* cc: emorley@… (added)
Comment:
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/19914#comment:2>
Django
Reporter: bpeschier | Owner: nobody
Type: Bug | Status: closed
Component: Core (Cache system) | Version: master
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by timgraham):
Maybe the warning could become an exception on memcached?
--
Ticket URL: <https://code.djangoproject.com/ticket/19914#comment:3>
Django
Reporter: bpeschier | Owner: nobody
Type: Bug | Status: closed
Component: Core (Cache system) | Version: master
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by edmorley):
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:
1. a key that contains space characters
2. 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:
a. not set keys names containing spaces (ie skip the test)
b. enable pylibmc's client side key validation passing `verify_keys`
within `behaviors` (just for this specific test)
c. 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`
* When the *value* is too long:
- python-memcached: Returns successfully but didn't set the key-value
(this is [https://github.com/linsomniac/python-memcached/issues/32 by
design])
- pylibmc 1.5.1: `pylibmc.Error: error 37 from memcached_set: SUCCESS`
- pylibmc master: `pylibmc.TooBig`
Possible options:
a. just add a try-except around the `.set()` for both python-memcached
and pylibmc
b. differentiate between the two, and only expect exceptions for the
pylibmc case (either by just catching them, or by enforcing using
`assertRaises`)
--
Ticket URL: <https://code.djangoproject.com/ticket/19914#comment:4>
Django
Reporter: bpeschier | Owner: nobody
Type: Bug | Status: new
Component: Core (Cache system) | Version: master
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by edmorley):
* status: closed => new
* resolution: invalid =>
* stage: Unreviewed => Accepted
Comment:
Reopening, since a test suite should be green on master, even if the
underlying library is at fault.
--
Ticket URL: <https://code.djangoproject.com/ticket/19914#comment:5>
Django
Reporter: bpeschier | Owner: edmorley
Type: Bug | Status: assigned
Component: Core (Cache system) | Version: master
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by edmorley):
* owner: nobody => edmorley
* status: new => assigned
Comment:
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
--
Ticket URL: <https://code.djangoproject.com/ticket/19914#comment:6>
Django
Reporter: bpeschier | Owner: edmorley
Type: Bug | Status: assigned
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tim Graham <timograham@…>):
In [changeset:"cfd1f93d55d3b9317bdf26b426fe21d935ab3399" cfd1f93d]:
{{{
#!CommitTicketReference repository=""
revision="cfd1f93d55d3b9317bdf26b426fe21d935ab3399"
Refs #19914 -- Split the test_invalid_keys cache test into two.
The first half of the test fails when using pylibmc (so will need
to be skipped).
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/19914#comment:7>
Django
Reporter: bpeschier | Owner: edmorley
Type: Bug | Status: assigned
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tim Graham <timograham@…>):
In [changeset:"5756edd46dfad87a0ab9856489887ed5bc2914b5" 5756edd4]:
{{{
#!CommitTicketReference repository=""
revision="5756edd46dfad87a0ab9856489887ed5bc2914b5"
[1.10.x] Refs #19914 -- Split the test_invalid_keys cache test into two.
The first half of the test fails when using pylibmc (so will need
to be skipped).
Backport of cfd1f93d55d3b9317bdf26b426fe21d935ab3399 from master
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/19914#comment:8>
Django
Reporter: bpeschier | Owner: edmorley
Component: Core (Cache system) | Version: master
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham <timograham@…>):
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"674e3fe13e5156344bfafbea59018b8837eb3044" 674e3fe1]:
{{{
#!CommitTicketReference repository=""
revision="674e3fe13e5156344bfafbea59018b8837eb3044"
Fixed #19914 -- Fixed test failures with pylibmc.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/19914#comment:9>
Django
Reporter: bpeschier | Owner: edmorley
Type: Bug | Status: closed
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Ed Morley <emorley@…>):
In [changeset:"255456becdf1435ce685faea5f3b58d34993e8a8" 255456be]:
{{{
#!CommitTicketReference repository=""
revision="255456becdf1435ce685faea5f3b58d34993e8a8"
[1.10.x] Fixed #19914 -- Fixed test failures with pylibmc.
Backport of 674e3fe13e5156344bfafbea59018b8837eb3044 from master
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/19914#comment:10>