Opened 7 years ago
Closed 4 years ago
#28342 closed Cleanup/optimization (needsinfo)
Make PyLibMCCache backend catch exceptions when memcached server is down
Reported by: | Julian Andrews | Owned by: | Joel Andrews |
---|---|---|---|
Component: | Core (Cache system) | Version: | 3.0 |
Severity: | Normal | Keywords: | memcached cache pylibmc |
Cc: | Triage Stage: | Accepted | |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
If your memcached server is down, pylibmc will raise a pylibmc.ConnectionError
or pylibmc.ServerDown
exception for all operations. This is different from the behavior of the python-memcached
package (see http://sendapatch.se/projects/pylibmc/misc.html). However, the PyLibMCCache
cache backend does nothing to catch these errors. As a result, the behavior is different from the MemcachedCache
backend which fails silently.
This behavior is particularly problematic for fragment caches which obviously shouldn't raise an exception. In any case, it seems pretty clear that both backends should have the same behavior.
Steps to reproduce:
- Setup a Django app
CACHES
set to usePyLibMCCache
:
CACHES = { 'default': { 'BACKEND': 'django.core.cache.backends.memcached.PyLibMCCache', 'LOCATION': '127.0.0.1:11211', } }
- With no active memcached server open a shell and run:
from django.core.cache import cache cache.get('foo')
pylibmc
will raise an exception. Using MemcachedCache
, cache.get
will instead return None
.
Change History (8)
comment:1 by , 7 years ago
follow-up: 3 comment:2 by , 7 years ago
Summary: | PyLibMCCache throwing exceptions when memcached server is down → Make PyLibMCCache backend catch exceptions when memcached server is down |
---|---|
Triage Stage: | Unreviewed → Accepted |
Type: | Bug → Cleanup/optimization |
It might be worth looking at django_pylibmc and emulating the behavior there.
comment:3 by , 7 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
comment:4 by , 7 years ago
I've written up a patch here: https://github.com/django/django/pull/8681. I looked at the django-pylibmc
code, and it was very similar to what I was already doing, but there are a few differences.
First, django-pylibmc
seems to be returning False
to indicate failure for a bunch of methods (and None
to indicate success). Given that that differs from the MemcachedCache
behavior, and that the False
vs. None
distinction is sort of confusing, I decided to stick with the default behavior. I actually like the idea of a return value that indicates if cache fetch was successful, but in that case, the relevant methods should probably always return True
or False
, and that behavior should be consistent across backends. I decided not to do that as part of this PR.
Second, django-pylibmc
includes logging of errors. I actually *really* like that, but again, it's not consistent with the other backend behavior, so I didn't include that. I could add logging though I'm not really that familiar with the django logging conventions, so some pointers on that would be helpful!
Third, django-pylibmc
was never catching ConnectionError
s. As far as I can tell, when the memcached server goes down, the first error gets a ConnectionError
and then later ones throw ServerDown
so I'm pretty sure we need to catch both.
comment:5 by , 7 years ago
Has patch: | set |
---|---|
Patch needs improvement: | set |
comment:6 by , 4 years ago
Owner: | changed from | to
---|---|
Patch needs improvement: | unset |
Version: | 1.11 → 3.0 |
Since this ticket and its corresponding pull request (https://github.com/django/django/pull/8681) have gone stale, I've reassigned this to myself and submitted a new, up-to-date pull request for 3.0: https://github.com/django/django/pull/12893.
comment:8 by , 4 years ago
Resolution: | → needsinfo |
---|---|
Status: | assigned → closed |
I'm not sure about catching all kind of exceptions or even pylibmc.ConnectionError
or pylibmc.ServerDown
. Consistency between different backends for the same serves is not a strong argument for me. pylibmc
policy is strongly expressed in the docs. Folks using different libraries can expect different behaviors, and you can always change your stack.
I think we should go back to the DevelopersMailingList and reach a strong consensus to move it forward.
My current workaround is subclass of
PyLibMCCache
which catchespylibmc.ConnectionError
orpylibmc.ServerDown
for each operation, and then does whateverMemcachedCache
is doing in each case. I'd be happy to turn that into a PR for a change toPyLibMCCache
if that approach is good. I'm not sure if there are other sorts of exceptions thatpylibmc
raises that should also be handled, but I think it would be a good start.