Re: [Django] #15815: Support memcached binary protocol in PyLibMCCache
Django
-------------------------------------+-------------------------------------
Reporter: mtigas | Owner: nobody
Type: New | Status: new
feature | Component: Core (Cache system)
Milestone: | Severity: Normal
Version: 1.3 | Keywords:
Resolution: | Has patch: 1
Triage Stage: Accepted | Needs tests: 1
Needs documentation: 1 | Easy pickings: 0
Patch needs improvement: 1 |
UI/UX: 0 |
-------------------------------------+-------------------------------------
Changes (by bkonkle):
* cc: brandon.konkle@… (added)
* ui_ux: => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/15815#comment:4>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
-------------------------------------+------------------------------------
Type: New feature | Status: new
Component: Core (Cache system) | Version: 1.3
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1
Needs tests: 1 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+------------------------------------
Changes (by robhudson):
* cc: rob@… (added)
Comment:
I recently updated django-pylibmc, a 3rd party cache backend, with a few
features we wanted at Mozilla. Namely, to make a timeout of zero mean an
infinite timeout, add compression support (which pylibmc handles), and
support the binary protocol. The package can be found here:
https://github.com/jbalogh/django-pylibmc.
I'd be happy to help push this forward or bring some code over into
Django. Mapping `OPTIONS` to pylibmc "behaviors" and adding the extra
`BINARY=True` parameter made sense to me. But whichever way is decided,
this should be an easy thing to add to this backend.
--
Ticket URL: <https://code.djangoproject.com/ticket/15815#comment:5>
Django
-------------------------------------+------------------------------------
Reporter: mtigas | Owner: nobody
Type: New feature | Status: new
Component: Core (Cache system) | Version: 1.3
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1
Needs tests: 1 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+------------------------------------
Comment (by aaugustin):
#19810 is related.
--
Ticket URL: <https://code.djangoproject.com/ticket/15815#comment:6>
Django
-------------------------------------+------------------------------------
Reporter: mtigas | Owner: nobody
Type: New feature | Status: new
Component: Core (Cache system) | Version: 1.3
Severity: Normal | Resolution:
Has patch: 1 | Needs documentation: 1
Needs tests: 1 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+------------------------------------
* keywords: => sprint2013
Comment:
Enabling binary mode breaks the tests at the moment, because keys with
spaces are actually allowed in binary mode.
--
Ticket URL: <https://code.djangoproject.com/ticket/15815#comment:7>
Django
Reporter: mtigas | Owner: bpeschier
Type: New feature | Status: assigned
Component: Core (Cache system) | Version: 1.3
Severity: Normal | Resolution:
Keywords: sprint2013 | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1
Needs tests: 1 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
Changes (by bpeschier):
* owner: nobody => bpeschier
* status: new => assigned
Comment:
Working on it here: https://github.com/bpeschier/django/tree/ticket_15815
Still needs some docs to demystify the options for pylibmc a bit.
Currently all options are set as behaviors except binary (which still
feels a bit ugly)
--
Ticket URL: <https://code.djangoproject.com/ticket/15815#comment:8>
Django
-------------------------------------+-------------------------------------
Reporter: mtigas | Owner: bpeschier
Type: New feature | Status: assigned
Component: Core (Cache system) | Version: 1.3
Severity: Normal | Resolution:
Keywords: sprint2013 | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1
Needs tests: 1 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by otherjacob):
I really don't think special casing is a big deal here -- it's not far off
of what BaseCache does already, although I'd rather `pop` binary off of
_options in the _cache method before setting behaviors to it.
Happy to finish this off if bpeschier can't get to it, but it's almost
done at this point I think.
--
Ticket URL: <https://code.djangoproject.com/ticket/15815#comment:9>
Django
-------------------------------------+-------------------------------------
Reporter: mtigas | Owner: bpeschier
Type: New feature | Status: assigned
Component: Core (Cache system) | Version: 1.3
Severity: Normal | Resolution:
Keywords: sprint2013 | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1
Needs tests: 1 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by the_drow):
So this is already implemented in https://github.com/jbalogh/django-
pylibmc.
Shouldn't we just merge that cache provider back?
--
Ticket URL: <https://code.djangoproject.com/ticket/15815#comment:10>
Django
-------------------------------------+------------------------------------
Type: New feature | Status: new
Severity: Normal | Resolution:
Keywords: sprint2013 | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1
Needs tests: 1 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+------------------------------------
* owner: bpeschier =>
* status: assigned => new
--
Ticket URL: <https://code.djangoproject.com/ticket/15815#comment:11>
Django
-------------------------------------+------------------------------------
Reporter: mtigas | Owner:
Type: New feature | Status: new
Component: Core (Cache system) | Version: 1.3
Severity: Normal | Resolution:
Keywords: sprint2013 | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1
Needs tests: 1 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+------------------------------------
Comment (by thedrow):
How come this is not supported? What's blocking it exactly?
How can I help?
--
Ticket URL: <https://code.djangoproject.com/ticket/15815#comment:12>
Django
-------------------------------------+------------------------------------
Reporter: mtigas | Owner:
Type: New feature | Status: new
Component: Core (Cache system) | Version: 1.3
Severity: Normal | Resolution:
Keywords: sprint2013 | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1
Needs tests: 1 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+------------------------------------
Comment (by timgraham):
I don't see a reviewable patch.
--
Ticket URL: <https://code.djangoproject.com/ticket/15815#comment:13>
Django
-------------------------------------+------------------------------------
Reporter: mtigas | Owner:
Type: New feature | Status: new
Component: Core (Cache system) | Version: 1.3
Severity: Normal | Resolution:
Keywords: sprint2013 | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1
Needs tests: 1 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+------------------------------------
Comment (by mlowicki):
any news about it?
--
Ticket URL: <https://code.djangoproject.com/ticket/15815#comment:14>
Django
-------------------------------------+------------------------------------
Reporter: mtigas | Owner:
Type: New feature | Status: new
Component: Core (Cache system) | Version: 1.3
Severity: Normal | Resolution:
Keywords: sprint2013 | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1
Needs tests: 1 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+------------------------------------
* cc: mlowicki@… (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/15815#comment:15>
Django
-------------------------------------+------------------------------------
Reporter: mtigas | Owner:
Type: New feature | Status: new
Component: Core (Cache system) | Version: 1.3
Severity: Normal | Resolution:
Keywords: sprint2013 | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1
Needs tests: 1 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+------------------------------------
Comment (by edmorley):
In case this helps anyone working on this in the future, if you get test
failures running the Django pylibmc cache tests with binary mode enabled
in `test_zero_timeout()` - I believe it's due to a bug in older versions
of libmemcached-dev.
This appears to only affect `.set()` and not `.add()`, and only appears if
using binary mode. The bug means that a Django zero timeout (which is
converted to `-1` when passed to pylibmc) is ignored by libmemcached and
the key is in fact set after all.
Affected libmemcached versions include that running on Travis on their
Ubuntu precise images (which presuming they are using the official package
is http://packages.ubuntu.com/precise/libmemcached-dev). libmemcached-dev
1.0.8 on Ubuntu trusty works fine. I couldn't find a related bug or commit
on https://launchpad.net/libmemcached but Launchpad is pretty painful to
use so I may have just missed it.
--
Ticket URL: <https://code.djangoproject.com/ticket/15815#comment:16>
Django
-------------------------------------+------------------------------------
Reporter: mtigas | Owner:
Type: New feature | Status: new
Component: Core (Cache system) | Version: 1.3
Severity: Normal | Resolution:
Keywords: sprint2013 | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1
Needs tests: 1 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+------------------------------------
Comment (by edmorley):
Correction: it affects both set() and add() from what I can now tell. The
behaviour also seems inconsistent on newer versions too.
I've opened an issue against pylibmc for this:
https://github.com/lericson/pylibmc/issues/202
--
Ticket URL: <https://code.djangoproject.com/ticket/15815#comment:17>
Django
Reporter: mtigas | Owner:
Type: New feature | Status: closed
Component: Core (Cache system) | Version: 1.3
Keywords: sprint2013 | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1
Needs tests: 1 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
Changes (by edmorley):
* status: new => closed
* cc: emorley@… (added)
* resolution: => duplicate
Comment:
This has been fixed in ticket #20892, by adding generic support for
passing parameters through to the memcached client constructor (and for
all backends, not just `PyLibMCCache`).
pylibmc's constructor is:
{{{#!python
def __init__(self, servers, behaviors=None, binary=False,
username=None, password=None):
}}}
So on Django master (will be 1.11), binary mode can be enabled using eg:
{{{#!python
CACHES = {
'default': {
'BACKEND': 'django.core.cache.backends.memcached.PyLibMCCache',
'LOCATION': '127.0.0.1:11211',
'OPTIONS': {
'binary': True,
'username': 'user',
'password': 'pass',
'behaviors': {
'ketama': True,
}
}
}
}
}}}
For more examples, see:
https://docs.djangoproject.com/en/dev/topics/cache/#cache-arguments
I'm going to try and backport these changes (plus ticket #27152) to
django-pylibmc, so the same Django settings file will work for both, to
make transitioning from "older Django+django-pylibmc backend" to "Django
1.11+ with stock backend" easier.
--
Ticket URL: <https://code.djangoproject.com/ticket/15815#comment:18>