[Django] #20892: Django memcached binding artificially limits item size
Django
-------------------------------+--------------------
Reporter: alex@… | Owner: nobody
Type: Bug | Status: new
Component: Uncategorized | Version: 1.5
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+--------------------
This appears to be an issue related to the fact that older releases of
memcached insisted on a 1 MB value size limit.
Current releases of memcached have a user-configurable limit, however
Django still uses a hard-wired 1 MB limit.
This (undocumented) 'feature' is currently causing errors, see the
following two SO questions (one by me):
http://stackoverflow.com/questions/18143159/apparent-bug-storing-large-
keys-in-django-memcached
http://stackoverflow.com/questions/11874377/django-caching-a-large-list
A 'solution' is proposed by the second SO question:
also edit this class in memcached.py located in /usr/lib/python2.7/dist-
packages/django/core/cache/backends to look like this:
{{{#!python
class MemcachedCache(BaseMemcachedCache):
"An implementation of a cache binding using python-memcached"
def __init__(self, server, params):
import memcache
memcache.SERVER_MAX_VALUE_LENGTH = 1024*1024*10 #added limit to accept
10mb
super(MemcachedCache, self).__init__(server, params,
library=memcache,
value_not_found_exception=ValueError)
}}}
Perhaps this could be added to the config of the logger?
For example, an ITEM_SIZE_MAX key could be created for the dictionary, so
the following code could be used:
{{{#!python
CACHES = {
'default': {
'BACKEND': 'django.core.cache.backends.memcached.MemcachedCache',
'ITEM_SIZE_MAX': 10,
'LOCATION': 'localhost:11211',
}
}
}}}
Obviously, the user would also have to add the necessary -I parameter to
their /etc/memcached.conf (system dependent). I suppose the best would be
if we could automatically detect the item size from their current
memcached setup. For example:
{{{#!python
from subprocess import Popen, Pipe
memcached_call = Popen("echo stats settings | nc localhost 11211",
stdout=PIPE, stderr=PIPE, shell=True)
out, err = memcached_call.communicate()
if err:
log("Error when getting memcached stats to find item size max:
"+err+"\n. Assuming max size is 1 MB")
maxsize_int = 1024*1024
else:
values = out.split("\n")
maxsize_field = filter(lambda field: field.startswith("STAT
item_size_max"), values)[0]
maxsize_int = int(maxsize_field.strip()[maxsize_field.rindex(" "):])
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/20892>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: alex@… | Owner: nobody
Type: Bug | Status: new
Component: Uncategorized | Version: 1.5
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by anonymous):
* needs_better_patch: => 0
* needs_tests: => 0
* needs_docs: => 0
Comment:
{{{#!python
import memcache
memcache.SERVER_MAX_VALUE_LENGTH = 1024*1024*10 #added limit to accept
10mb
}}}
Apparently this solution does **NOT** actually work... the value from the
module is not propagated to the client properly.
I've reported the issue to python-memcache: https://github.com/linsomniac
/python-memcached/issues/13
--
Ticket URL: <https://code.djangoproject.com/ticket/20892#comment:1>
Django
Reporter: alex@… | Owner: nobody
Type: Bug | Status: new
Component: Uncategorized | Version: 1.5
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by aaugustin):
As far as I can tell, Django itself doesn't contain any code to limit the
size of cached items. (If it does, sorry, I missed it.)
Could you clarify your assertion that "Django still uses a hard-wired 1 MB
limit."?
If the limitation is entirely handled by the memcache client library and
server, I'm inclined to close this ticket as invalid.
--
Ticket URL: <https://code.djangoproject.com/ticket/20892#comment:2>
Django
Reporter: alex@… | Owner: nobody
Type: Bug | Status: new
Component: Uncategorized | Version: 1.5
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by shai):
* cc: shai@… (added)
Comment:
If I understand correctly, the ticket complains that there is no sane way
to configure the max-item-size for use in Django; this leads to Django
apps "always" using the default 1M limit. For the suggested change to
work, the user also needs to take care of their server (at least if using
a larger value), and if the [comment:1 anonymous user] is right, also fix
a bug in the client lib, but the Django part is missing either way.
I think that is valid.
--
Ticket URL: <https://code.djangoproject.com/ticket/20892#comment:3>
Django
Reporter: alex@… | Owner: nobody
Type: Bug | Status: new
Component: Uncategorized | Version: 1.5
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by timo):
It's possible to accomplish this using a custom cache backend, but it may
be nice to make this a bit easier:
{{{#!python
import pickle
from django.core.cache.backends.memcached import MemcachedCache
MIN_COMPRESS_LENGTH = 1048576 # 1MB
class CustomMemcachedCache(MemcachedCache):
@property
def _cache(self):
"""
Override to add pass some custom parameters to the
python_memcached
backend.
"""
if getattr(self, '_client', None) is None:
options = {}
if self._options and 'SERVER_MAX_VALUE_LENGTH' in
self._options:
options['server_max_value_length'] =
self._options['SERVER_MAX_VALUE_LENGTH']
self._client = self._lib.Client(self._servers,
pickleProtocol = pickle.HIGHEST_PROTOCOL, **options
)
return self._client
def set(self, key, value, timeout=0, version=None):
"""
Override to pass MIN_COMPRESS_LENGTH so large values are
compressed.
"""
key = self.make_key(key, version=version)
self._cache.set(key, value, self._get_memcache_timeout(timeout),
MIN_COMPRESS_LENGTH)
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/20892#comment:4>
Django
Reporter: alex@… | Owner: nobody
Type: Bug | Status: new
Component: Uncategorized | Version: 1.5
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by anonymous):
""If I understand correctly, the ticket complains that there is no sane
way to configure the max-item-size for use in Django""
Precisely.
Django does not deliberately limit the size, but it's dependent modules
do, without having their configuration options passed through.
--
Ticket URL: <https://code.djangoproject.com/ticket/20892#comment:5>
Django
Reporter: alex@… | Owner: nobody
Type: Bug | Status: new
Component: Uncategorized | Version: 1.5
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by anonymous):
Sorry guys, I just realised I really ought to make an account and stop
posting anonymously... I posted this issue, and the anonymous comments are
by me.
--
Ticket URL: <https://code.djangoproject.com/ticket/20892#comment:6>
Django
Reporter: alex@… | Owner: nobody
Type: Bug | Status: new
Component: Uncategorized | Version: 1.5
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by AlexF124):
Ok, python-memcache has gone ahead and fixed the bug, so the
{{{!#python
import memcache
memcache.SERVER_MAX_VALUE_LENGTH = 1024*1024*10 #added limit to accept
10mb
}}}
would work from python-memcache's next release and onwards.
--
Ticket URL: <https://code.djangoproject.com/ticket/20892#comment:7>
Django
Reporter: alex@… | Owner: nobody
Type: Bug | Status: new
Component: Uncategorized | Version: 1.5
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by aaugustin):
I'm still unconvinced there's much value in channeling this configuration
through Django's CACHE setting, but I'm not opposing it if another core
dev wants it.
--
Ticket URL: <https://code.djangoproject.com/ticket/20892#comment:8>
Django
-------------------------------+------------------------------------
Reporter: alex@… | Owner: nobody
Type: New feature | Status: new
Component: Uncategorized | Version: 1.5
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by shai):
* type: Bug => New feature
* stage: Unreviewed => Accepted
Comment:
I'm not sure if that was Aymeric's point, but in any case, there should be
a "don't do that unless you know what you're doing" warning on the use of
this feature; memcached recommends strongly against it. I think as part of
this, the entry should be named "FORCE_ITEM_MAX_SIZE" or something like
it.
BTW, I think the most common cause for wanting it -- the only case I've
seen up close -- is pushing big objects into a cached session. Perhaps
this is something we should specifically recommend against.
--
Ticket URL: <https://code.djangoproject.com/ticket/20892#comment:9>
Django
Reporter: alex@… | Owner: nobody
Type: New feature | Status: new
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timo):
* component: Uncategorized => Core (Cache system)
--
Ticket URL: <https://code.djangoproject.com/ticket/20892#comment:10>
Django
-------------------------------------+------------------------------------
Type: New feature | Status: new
Component: Core (Cache system) | Version: 1.5
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by olavmrk):
* cc: olavmrk@… (added)
Comment:
Hi,
I ran into this issue now, and have a few comments:
* This problem only affects the
`django.core.cache.backends.memcached.MemcachedCache` backend. A simple
workaround may be to use the
`django.core.cache.backends.memcached.PyLibMCCache` backend.
* To fix this, one doesn't actually need to write to
`memcache.SERVER_MAX_VALUE_LENGTH` variable. It can also be passed as an
option to the constructor: `[...].Client(self._servers,
pickleProtocol=pickle.HIGHEST_PROTOCOL,
server_max_value_length=max_value_size)`
As for reasons to want this: In my case, I am fetching a blob of data from
a remote web service that is 1.2 MiB. Caching that data blob saves an
expensive API call.
--
Ticket URL: <https://code.djangoproject.com/ticket/20892#comment:11>
Django
-------------------------------------+------------------------------------
Reporter: alex@… | Owner: nobody
Type: New feature | Status: new
Component: Core (Cache system) | Version: 1.5
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by timgraham):
Lets make this ticket a bit more generic after a request in #26301 to set
another option (`socket_timeout`) in `Client.__init__()`. I'm thinking a
key in `'OPTIONS'` like `CLIENT_OPTIONS` which would be a dictionary
passed to `Client.__init__()` as `kwargs` might work.
--
Ticket URL: <https://code.djangoproject.com/ticket/20892#comment:12>
Django
-------------------------------------+------------------------------------
Type: New feature | Status: new
Component: Core (Cache system) | Version: 1.5
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Fiedzia):
PR: https://github.com/django/django/pull/6233
--
Ticket URL: <https://code.djangoproject.com/ticket/20892#comment:13>
Django
-------------------------------------+------------------------------------
Type: New feature | Status: new
Component: Core (Cache system) | Version: 1.5
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* has_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/20892#comment:14>
Django
-------------------------------------+------------------------------------
Type: New feature | Status: new
Component: Core (Cache system) | Version: 1.5
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* needs_better_patch: 0 => 1
Comment:
Please uncheck "patch needs improvement" after you update the patch,
thanks!
--
Ticket URL: <https://code.djangoproject.com/ticket/20892#comment:15>
Django
-------------------------------------+------------------------------------
Type: New feature | Status: new
Component: Core (Cache system) | Version: 1.5
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Fiedzia):
* needs_better_patch: 1 => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/20892#comment:16>
Django
-------------------------------------+------------------------------------
Type: New feature | Status: new
Component: Core (Cache system) | Version: 1.5
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* needs_better_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/20892#comment:17>
Django
-------------------------------------+------------------------------------
Type: New feature | Status: new
Component: Core (Cache system) | Version: 1.5
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Comment (by Tim Graham <timograham@…>):
In [changeset:"606a303856afee684563f9349c2a55578854f1ba" 606a303]:
{{{
#!CommitTicketReference repository=""
revision="606a303856afee684563f9349c2a55578854f1ba"
Fixed #27124 -- Excluded cull-related cache configs from memcached tests.
Since the `cull` and `zero_cull` test cache configs set `MAX_ENTRIES`
and `CULL_FREQUENCY` in `OPTIONS`, which are only intended for use with
the locmem, filesystem, and database backends. This prevents test
failures once refs #20892 is fixed.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/20892#comment:18>
Django
-------------------------------------+------------------------------------
Type: New feature | Status: assigned
Component: Core (Cache system) | Version: 1.5
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by edmorley):
* status: new => assigned
* needs_better_patch: 1 => 0
* owner: nobody => edmorley
* cc: emorley@… (added)
Comment:
I've opened a new PR, which takes a slightly different approach:
* The contents of `OPTIONS` is now passed directly to the client
constructor
* For pylibmc, behaviors are now passed inside the `behaviors` key within
`OPTIONS`, rather than as top-level properties of `OPTIONS`. The old
behavior is still supported for now, but generates deprecation warnings.
Discussion of alternative approaches (on the old PR):
https://github.com/django/django/pull/6233#issuecomment-192459369
The new PR:
https://github.com/django/django/pull/7160
--
Ticket URL: <https://code.djangoproject.com/ticket/20892#comment:19>
Django
-------------------------------------+------------------------------------
Reporter: alex@… | Owner: edmorley
Type: New feature | Status: assigned
Component: Core (Cache system) | Version: 1.5
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tim Graham <timograham@…>):
In [changeset:"fb8eea5680f20d3df50eb6a7d972645a388cb196" fb8eea56]:
{{{
#!CommitTicketReference repository=""
revision="fb8eea5680f20d3df50eb6a7d972645a388cb196"
[1.10.x] Fixed #27124 -- Excluded cull-related cache configs from
memcached tests.
Since the `cull` and `zero_cull` test cache configs set `MAX_ENTRIES`
and `CULL_FREQUENCY` in `OPTIONS`, which are only intended for use with
the locmem, filesystem, and database backends. This prevents test
failures once refs #20892 is fixed.
Backport of 606a303856afee684563f9349c2a55578854f1ba from master
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/20892#comment:20>
Django
-------------------------------------+------------------------------------
Reporter: alex@… | Owner: edmorley
Component: Core (Cache system) | Version: 1.5
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham <timograham@…>):
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"65ec8fa8ca56a5378345375e1079025c96d0b833" 65ec8fa]:
{{{
#!CommitTicketReference repository=""
revision="65ec8fa8ca56a5378345375e1079025c96d0b833"
Fixed #20892 -- Allowed configuring memcached client using OPTIONS.
Previously, the MemcachedCache backend ignored `OPTIONS` and
PyLibMCCache used them to set pylibmc behaviors. Both backends now
pass `OPTIONS` as keyword arguments to the client constructors.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/20892#comment:21>
Django
Reporter: alex@… | Owner: Ed Morley
Type: New feature | Status: closed
Component: Core (Cache system) | Version: 1.5
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tim Graham <timograham@…>):
In [changeset:"e47b56d791eb6c81c6d83529b7a068959ea1c1ec" e47b56d7]:
{{{
#!CommitTicketReference repository=""
revision="e47b56d791eb6c81c6d83529b7a068959ea1c1ec"
Refs #20892 -- Removed support for passing pylibmc behavior settings as
top-level attributes of CACHES['OPTIONS'].
Per deprecation timeline.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/20892#comment:22>