Opened 9 years ago

Closed 8 years ago

Last modified 5 years ago

#6464 closed (fixed)

memcached cache backend incr/decr not implemented

Reported by: panni@… Owned by: nobody
Component: Core (Cache system) Version: master
Severity: Keywords: cache memcached incr decr
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

The very efficient memcached.incr/decr() functions aren't implemented in django.core.cache.backends.memcached.CacheClass,
but accessible through cache._cache.incr/decr()

Attachments (2)

6464.memcached.incr.decr.diff (973 bytes) - added by Pete Crosier 9 years ago.
Adds decr and incr support to the memcached backend
6464.memcached.incr.decr.2.diff (4.2 KB) - added by Michael Malone 8 years ago.
Increment and decrement operations for cache backends

Download all attachments as: .zip

Change History (13)

comment:1 Changed 9 years ago by Simon Greenhill <dev@…>

Needs documentation: unset
Needs tests: unset
Patch needs improvement: unset
Triage Stage: UnreviewedAccepted

Changed 9 years ago by Pete Crosier

Adds decr and incr support to the memcached backend

comment:2 Changed 9 years ago by Pete Crosier

Has patch: set
Keywords: cache, memcached, incr, decrcache memcached incr decr
Needs documentation: set
Needs tests: set

Patch attached that adds decr and incr support to the memcached backend - if the key doesn't exist, the value of the key isn't an integer or the delta isn't an integer then None is returned.

comment:3 Changed 9 years ago by Pete Crosier

Heh, I should explain my own code better - if the key doesn't exist then return None, if the key's value and the delta are both integers then return the key's new value, otherwise return the key's value.

Changed 8 years ago by Michael Malone

Increment and decrement operations for cache backends

comment:4 Changed 8 years ago by Michael Malone

milestone: post-1.0

New patch is a simple wrapper around the memcache library for the memcached backend. It also includes a simple (non-atomic) implementation for other cache backends, and raises a NotImplementedException for the dummy cache backend (we can't return the incremented/decremented value since we don't have the original value).

Since non-memcache backends can't perform this operation atomically, it might be better to raise a NotImplementedException for all non-memcache backends. That said, I added docs that make it pretty clear that these operations are only atomic for memcached...

comment:5 Changed 8 years ago by Michael Malone

Needs documentation: unset
Needs tests: unset

comment:6 Changed 8 years ago by (none)

milestone: post-1.0

Milestone post-1.0 deleted

comment:7 Changed 8 years ago by Adam Gomaa

Triage Stage: AcceptedDesign decision needed

Marking as DDN: it's expanding the scope django.core.cache, and the unavoidable inconsistencies in the cache backends might make this something outside of Django's scope.

After all, incr/decr are not general caching primitives; they're memcached primitives. I'm not sure we should try to simulate them on other backends.

comment:8 Changed 8 years ago by Jacob

milestone: 1.1 beta
Triage Stage: Design decision neededAccepted

Thing is, though, that incr/decr are so ridiculously useful on memcached that I think not supporting them effectively cripples our cache support. Yes, emulating them on other backends makes those operations slower for those backends, but the fact is that if you want speed you should be using memcached, anyway. Thus, I'm going to make the design decision and accept this.

comment:9 Changed 8 years ago by Michael Malone

I agree w/ Jacob, and feel the same way about much of the memcache API. While some of the operations may not be "general" caching primitives, nearly all of them are extremely useful caching primitives. The memcache API is pretty simple, and if memcache implements a particular API and Django doesn't, I think it's worth seriously questioning why.

comment:10 Changed 8 years ago by Russell Keith-Magee

Resolution: fixed
Status: newclosed

(In [10031]) Fixed #6464 -- Added incr() and decr() operations on cache backends. Atomic on Memcache; implemented as a 2 stage retrieve/update on other backends. Includes refactor of the cache tests to ensure all the backends are actually tested, and a fix to the DB cache backend that was discovered as a result. Thanks to Michael Malone for the original patch.

comment:11 Changed 5 years ago by Jacob

milestone: 1.1 beta

Milestone 1.1 beta deleted

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