Opened 7 years ago
Closed 4 years ago
#29403 closed Cleanup/optimization (needsinfo)
Make PyLibMCCache backend handle TooBig exception from pylibmc
Reported by: | SKisContent | Owned by: | Hasan Ramezani |
---|---|---|---|
Component: | Core (Cache system) | Version: | 1.11 |
Severity: | Normal | Keywords: | memcached, TooBig |
Cc: | Triage Stage: | Accepted | |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
I'm trying to use the memcached backend cache in a project, and I'm get a TooBig error on some requests. The error occurs in django/core/cache/backends/memcached.py on line 86:
val = self._cache.get(key) if val is None: return default return val def set(self, key, value, timeout=DEFAULT_TIMEOUT, version=None): key = self.make_key(key, version=version) ** if not self._cache.set(key, value, self.get_backend_timeout(timeout)):** # make sure the key doesn't keep its old value in case of failure to set (memcached's 1MB limit) self._cache.delete(key) def delete(self, key, version=None): key = self.make_key(key, version=version) self._cache.delete(key)
I can reproduce it using the Django shell:
$ ./manage.py shell Python 2.7.12 (default, Nov 19 2016, 06:48:10) [GCC 5.4.0 20160609] on linux2 Type "help", "copyright", "credits" or "license" for more information. (InteractiveConsole) >>> from django.core.cache import cache >>> cache.get('foo') >>> cache.set('foo', 'bar') >>> cache.get('foo') 'bar' >>> big = [i for i in range(1024*1024)] >>> cache.set('big_foo', big) Traceback (most recent call last): File "<console>", line 1, in <module> File "/local/lib/python2.7/site-packages/django/core/cache/backends/memcached.py", line 86, in set if not self._cache.set(key, value, self.get_backend_timeout(timeout)): TooBig
I can replicate the same behavior in pylibmc:
$ python Python 2.7.12 (default, Nov 19 2016, 06:48:10) [GCC 5.4.0 20160609] on linux2 Type "help", "copyright", "credits" or "license" for more information. >>> from pylibmc.test import make_test_client >>> mc = make_test_client() >>> mc.get('foo') >>> mc.set('foo', 'bar') True >>> mc.get('foo') 'bar' >>> big = [i for i in range(1024*1024)] >>> mc.set('big_foo', big) Traceback (most recent call last): File "<console>", line 1, in <module> TooBig
The TooBig exception was added to pylibmc in 2016: https://github.com/lericson/pylibmc/commit/736e21276ad04d557b68bd81b7f28be1a4b4e1ec
The uncaught error breaks the application. Looking at the comment about the 1MB size limit on the next line (line 87), it seems like the original intent was to unset the value and fail silently. Therefore, the if block needs to be wrapped in try...except.
Change History (5)
comment:1 by , 7 years ago
comment:2 by , 7 years ago
Summary: | TooBig error in memcached → Make PyLibMCCache backend handle TooBig exception from pylibmc |
---|---|
Triage Stage: | Unreviewed → Accepted |
There's a related pull request.
comment:3 by , 5 years ago
I'm not convinced we should fail silently. I understand that some versions o memcached will, but I'd rather be warn the user that something went wrong instead of being consistent.
If we know something went wrong we should tell the user. I'd recommend raising a more descriptive exception, possibly using raise ... from ...
.
comment:4 by , 4 years ago
Has patch: | set |
---|---|
Owner: | changed from | to
Status: | new → assigned |
comment:5 by , 4 years ago
Resolution: | → needsinfo |
---|---|
Status: | assigned → closed |
Type: | Bug → Cleanup/optimization |
Like Flavio Curella and Ed Morley, I'm not convinced that we should catch this exception, it was shortly discussed in #19914. See also arguments in #28342 (this is not a duplicate but is related). The main question is: should we force consistency between these two backends? IMO it's better to respect their different policies of handling errors.
I think we should go back to the DevelopersMailingList and reach a consensus to move it forward.
To clarify, I'm using PyLibMCCache: