Opened 17 years ago

Closed 16 years ago

Last modified 13 years ago

#6464 closed (fixed)

memcached cache backend incr/decr not implemented

Reported by: panni@… Owned by: nobody
Component: Core (Cache system) Version: dev
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: no UI/UX: no

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 17 years ago.
Adds decr and incr support to the memcached backend
6464.memcached.incr.decr.2.diff (4.2 KB ) - added by Michael Malone 16 years ago.
Increment and decrement operations for cache backends

Download all attachments as: .zip

Change History (13)

comment:1 by Simon Greenhill <dev@…>, 17 years ago

Triage Stage: UnreviewedAccepted

by Pete Crosier, 17 years ago

Adds decr and incr support to the memcached backend

comment:2 by Pete Crosier, 17 years ago

Has patch: set
Keywords: cache, memcached, incr, decr → cache 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 by Pete Crosier, 17 years ago

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.

by Michael Malone, 16 years ago

Increment and decrement operations for cache backends

comment:4 by Michael Malone, 16 years ago

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 by Michael Malone, 16 years ago

Needs documentation: unset
Needs tests: unset

comment:6 by (none), 16 years ago

milestone: post-1.0

Milestone post-1.0 deleted

comment:7 by Adam Gomaa, 16 years ago

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 by Jacob, 16 years ago

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 by Michael Malone, 16 years ago

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 by Russell Keith-Magee, 16 years ago

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 by Jacob, 13 years ago

milestone: 1.1 beta

Milestone 1.1 beta deleted

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