[Django] #28342: PyLibMCCache throwing exceptions when memcached server is down
Django
-------------------------------------+-------------------------------------
Reporter: Julian | Owner: nobody
Andrews |
Type: Bug | Status: new
Component: Core | Version: 1.11
(Cache system) | Keywords: memcached cache
Severity: Normal | pylibmc
Triage Stage: | Has patch: 0
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
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:
1. Setup a Django app `CACHES` set to use `PyLibMCCache`:
{{{
CACHES = {
'default': {
'BACKEND': 'django.core.cache.backends.memcached.PyLibMCCache',
'LOCATION': '127.0.0.1:11211',
}
}
}}}
2. 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`.
--
Ticket URL: <https://code.djangoproject.com/ticket/28342>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
-------------------------------------+-------------------------------------
Type: Bug | Status: new
Component: Core (Cache system) | Version: 1.11
Severity: Normal | Resolution:
Keywords: memcached cache | Triage Stage:
pylibmc | Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Julian Andrews):
My current workaround is subclass of `PyLibMCCache` which catches
`pylibmc.ConnectionError` or `pylibmc.ServerDown` for each operation, and
then does whatever `MemcachedCache` is doing in each case. I'd be happy to
turn that into a PR for a change to `PyLibMCCache` if that approach is
good. I'm not sure if there are other sorts of exceptions that `pylibmc`
raises that should also be handled, but I think it would be a good start.
--
Ticket URL: <https://code.djangoproject.com/ticket/28342#comment:1>
Django
-------------------------------------+-------------------------------------
Reporter: Julian Andrews | Owner: nobody
Type: | Status: new
Cleanup/optimization |
Component: Core (Cache system) | Version: 1.11
Severity: Normal | Resolution:
pylibmc |
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* type: Bug => Cleanup/optimization
* stage: Unreviewed => Accepted
Comment:
It might be worth looking at [https://github.com/django-pylibmc/django-
pylibmc/blob/master/django_pylibmc/memcached.py django_pylibmc] and
emulating the behavior there.
--
Ticket URL: <https://code.djangoproject.com/ticket/28342#comment:2>
Django
-------------------------------------+-------------------------------------
Reporter: Julian Andrews | Owner: Julian
Type: | Andrews
Cleanup/optimization | Status: assigned
Component: Core (Cache system) | Version: 1.11
Severity: Normal | Resolution:
Keywords: memcached cache | Triage Stage: Accepted
pylibmc |
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* owner: nobody => Julian Andrews
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/28342#comment:3>
Django
-------------------------------------+-------------------------------------
Reporter: Julian Andrews | Owner: Julian
Type: | Andrews
Cleanup/optimization | Status: assigned
Component: Core (Cache system) | Version: 1.11
Severity: Normal | Resolution:
Keywords: memcached cache | Triage Stage: Accepted
pylibmc |
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Julian Andrews):
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/28342#comment:4>
Django
-------------------------------------+-------------------------------------
Reporter: Julian Andrews | Owner: Julian
Type: | Andrews
Cleanup/optimization | Status: assigned
Component: Core (Cache system) | Version: 1.11
Severity: Normal | Resolution:
Keywords: memcached cache | Triage Stage: Accepted
pylibmc |
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* needs_better_patch: 0 => 1
* has_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/28342#comment:5>
Django
-------------------------------------+-------------------------------------
Reporter: Julian Andrews | Owner: Joel
Type: | Andrews
Cleanup/optimization | Status: assigned
Severity: Normal | Resolution:
Keywords: memcached cache | Triage Stage: Accepted
pylibmc |
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* owner: Julian Andrews => Joel Andrews
* needs_better_patch: 1 => 0
* version: 1.11 => 3.0
Comment:
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/28342#comment:6>
Django
-------------------------------------+-------------------------------------
Reporter: Julian Andrews | Owner: Joel
Type: | Andrews
Cleanup/optimization | Status: assigned
Component: Core (Cache system) | Version: 3.0
Severity: Normal | Resolution:
Keywords: memcached cache | Triage Stage: Accepted
pylibmc |
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Joel Andrews):
No relation to the ticket reporter. :D
--
Ticket URL: <https://code.djangoproject.com/ticket/28342#comment:7>
Django
-------------------------------------+-------------------------------------
Reporter: Julian Andrews | Owner: Joel
Type: | Andrews
Component: Core (Cache system) | Version: 3.0
Keywords: memcached cache | Triage Stage: Accepted
pylibmc |
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: assigned => closed
* resolution: => needsinfo
Comment:
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
[http://sendapatch.se/projects/pylibmc/misc.html#exceptions 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.
--
Ticket URL: <https://code.djangoproject.com/ticket/28342#comment:8>