Opened 4 years ago

Last modified 5 weeks ago

#15815 new New feature

Support memcached binary protocol in PyLibMCCache

Reported by: mtigas Owned by:
Component: Core (Cache system) Version: 1.3
Severity: Normal Keywords: sprint2013
Cc: brandon.konkle@…, rob@… Triage Stage: Accepted
Has patch: yes Needs documentation: yes
Needs tests: yes Patch needs improvement: yes
Easy pickings: no UI/UX: no

Description

As per http://sendapatch.se/projects/pylibmc/#example-usage , enabling the binary protocol requires the binary=True argument to be passed into pylibmc.Client when it is initialized.

The binary protocol is available in Memcached 1.3+ and provides a performance boost in high-load circumstances: http://www.slideshare.net/tmaesaka/memcached-binary-protocol-in-a-nutshell-presentation

pylibmc ignores any unknown options passed into Client.behaviors (see below), so putting this in OPTIONS seems like the way to go.

>>> import pylibmc
>>> a=pylibmc.Client(['127.0.0.1:55838',])
>>> print a.behaviors
{'cas': 0, 'no_block': 0, 'receive_timeout': 0, 'send_timeout': 0, 'ketama_hash': 0, '_poll_timeout': 5000, '_io_bytes_watermark': 66560, 'cache_lookups': 0, '_sort_hosts': 0, '_io_key_prefetch': 0, '_auto_eject_hosts': 0, 'ketama': 0, 'ketama_weighted': 0, '_io_msg_watermark': 500, '_hash_with_prefix_key': 0, 'tcp_nodelay': 0, 'failure_limit': 0, 'buffer_requests': 0, '_socket_send_size': 0, '_retry_timeout': 0, '_noreply': 0, '_socket_recv_size': 0, '_number_of_replicas': 0, 'distribution': 'modula', 'connect_timeout': 4000, 'hash': 'default', 'verify_keys': 0}

>>> a.behaviors = {"binary":True} # should be ignored by pylibmc
>>> print a.behaviors
{'cas': 0, 'no_block': 0, 'receive_timeout': 0, 'send_timeout': 0, 'ketama_hash': 0, '_poll_timeout': 5000, '_io_bytes_watermark': 66560, 'cache_lookups': 0, '_sort_hosts': 0, '_io_key_prefetch': 0, '_auto_eject_hosts': 0, 'ketama': 0, 'ketama_weighted': 0, '_io_msg_watermark': 500, '_hash_with_prefix_key': 0, 'tcp_nodelay': 0, 'failure_limit': 0, 'buffer_requests': 0, '_socket_send_size': 0, '_retry_timeout': 0, '_noreply': 0, '_socket_recv_size': 0, '_number_of_replicas': 0, 'distribution': 'modula', 'connect_timeout': 4000, 'hash': 'default', 'verify_keys': 0}

Attachments (1)

pylibmc-binary.diff (656 bytes) - added by mtigas 4 years ago.

Download all attachments as: .zip

Change History (14)

Changed 4 years ago by mtigas

comment:1 Changed 4 years ago by mtigas

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

Not sure if patch is in the proper format; have also opened a github pull request, tracking this bug. https://github.com/django/django/pull/21

Looked into updating the docs, however the documentation appears to leave out implementation-specific details of the different memcached backends. Please let me know if I’m mistaken.

comment:2 Changed 4 years ago by jacob

  • Easy pickings unset
  • Needs documentation set
  • Needs tests set
  • Patch needs improvement set
  • Triage Stage changed from Unreviewed to Accepted

The patch isn't ideal - special-casing "binary" is a bit annoying - but this is totally worth having.

comment:3 Changed 4 years ago by mtigas

I’ll agree with that, though I wasn’t sure how to make this semantically better.

For the older cache framework, django-newcache accepted binary as a cache param (with timeout, cull_frequency, et. al.), with CACHE_BEHAVIORS as a separate settings option. (Using django-newcache as a comparison point since some of the newer 1.3+ cache features like versioning and key prefixing appear to have been based on the newcache implementation.)

But I didn’t quite feel that special casing PyLibMCCache to accept a new base parameter was correct, either …

CACHES = {
    'default' : {
        "BACKEND" : 'django.core.cache.backends.memcached.PyLibMCCache',
        "LOCATION" : '127.0.0.1:11211',
        "BINARY" : True,
        "OPTIONS" : dict(tcp_nodelay=True, ketama=True),
    }
}

… since the description of OPTIONS reads, “Any options that should be passed to cache backend. The list options understood by each backend vary with each backend. […] Cache backends backed by a third-party library will pass their options directly to the underlying cache library.”

In particular, that seems to imply that for consistency’s sake, all implementation-specific options regarding a backend should go into OPTIONS and that it’s up to the backend to do what it needs to provide the correct information to the underlying library.

Technically, a more semantically-correct option would be to do something like:

CACHES = {
    'default' : {
        "BACKEND" : 'django.core.cache.backends.memcached.PyLibMCCache',
        "LOCATION" : '127.0.0.1:11211',
        "OPTIONS" : {
            "binary": True,
            "behaviors" : dict(tcp_nodelay=True, ketama=True),
        }
    }
}

Not really sure what the best patch would be at this point.

comment:4 Changed 4 years ago by bkonkle

  • Cc brandon.konkle@… added
  • UI/UX unset

comment:5 Changed 4 years ago by robhudson

  • Cc rob@… added

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.

comment:6 Changed 3 years ago by aaugustin

#19810 is related.

comment:7 Changed 3 years ago by bpeschier

  • Keywords sprint2013 added

Enabling binary mode breaks the tests at the moment, because keys with spaces are actually allowed in binary mode.

comment:8 Changed 3 years ago by bpeschier

  • Owner changed from nobody to bpeschier
  • Status changed from new to assigned

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)

comment:9 Changed 2 years ago 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.

comment:10 Changed 21 months ago by the_drow

So this is already implemented in https://github.com/jbalogh/django-pylibmc.
Shouldn't we just merge that cache provider back?

comment:11 Changed 5 months ago by bpeschier

  • Owner bpeschier deleted
  • Status changed from assigned to new

comment:12 Changed 5 weeks ago by thedrow

How come this is not supported? What's blocking it exactly?
How can I help?

comment:13 Changed 5 weeks ago by timgraham

I don't see a reviewable patch.

Note: See TracTickets for help on using tickets.
Back to Top