MemcachedCacheTests failing on pylibmc

[Bas Peschier]

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',
    }
}

[Jacob]

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.

[Ed Morley]

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.

[Tim Graham]

Maybe the warning could become an exception on memcached?

[Ed Morley]

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:

1. not set keys names containing spaces (ie skip the test)
2. enable pylibmc's client side key validation passing verify_keys within behaviors (just for this specific test)
3. 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 ​by design)
  • pylibmc 1.5.1: pylibmc.Error: error 37 from memcached_set: SUCCESS
  • pylibmc master: pylibmc.TooBig

Possible options:

1. just add a try-except around the .set() for both python-memcached and pylibmc
2. differentiate between the two, and only expect exceptions for the pylibmc case (either by just catching them, or by enforcing using assertRaises)

[Ed Morley]

Reopening, since a test suite should be green on master, even if the underlying library is at fault.

[Ed Morley]

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

[Tim Graham <timograham@…>]

In cfd1f93d:

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).

[Tim Graham <timograham@…>]

In 5756edd4:

[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

[Tim Graham <timograham@…>]

In 674e3fe1:

Fixed #19914 -- Fixed test failures with pylibmc.

[Ed Morley <emorley@…>]

In 255456be:

[1.10.x] Fixed #19914 -- Fixed test failures with pylibmc.

Backport of 674e3fe13e5156344bfafbea59018b8837eb3044 from master