memcached-based-cache - timeout=0 does not work as intended by memcached

[Carsten Reimer]

Hello,

I am not quite sure if this is the right mailinglist but as long as my
remarks are about a core-component of django I hopefully chose the right
list.

Dealing with cache-stuff in Django I realized that it seems to be
impossible to use a timeout=0 (which in terms of memcached meant that
the item would never expire unless it has to make way for new items due
to memory shortage). This is because even still in the most current
trunk-version the timeout is calculated (in
django.core.backends.memcached.CacheClass._get_memcache_timeout) as

timeout = timeout or self.default_timeout

where timeout is one of the parameters given to _get_memcache_timeout.

So as long as timeout=0 always the default_timeout will be used.
Maybe this behaviour is intended to prevent memcached being filled up
with items that never expire.
In my opinion it may be better to enable developers to use the
timeout-values as intended by memcached (where 0 means no expiration at
all).

So I would like to suggest to change the default value in all method
signatures of the memcached.CacheClass (and whereever it whould be
necessary) from 0 to None and to replace the line above by an if-clause as

if timeout is None:
timeout = self.default_timeout

This is not as elegant as the original version but now it would be
possible to use 0 as a timeout.

If it would help I can try to open a ticket and provide a first path
against the current trunk.

With best regards

Carsten Reimer

--
Carsten Reimer
Web Developer
carsten...@galileo-press.de
Phone +49.228.42150.73

Galileo Press GmbH
Rheinwerkallee 4 - 53227 Bonn - Germany
Phone +49.228.42150.0 (Zentrale) .77 (Fax)
http://www.galileo-press.de/

Managing Directors: Tomas Wehren, Ralf Kaulisch, Rainer Kaltenecker
HRB 8363 Amtsgericht Bonn

[Tobias McNulty]

+1

Sent from my mobile device.

--
You received this message because you are subscribed to the Google Groups "Django developers" group.
To post to this group, send email to django-d...@googlegroups.com.
To unsubscribe from this group, send email to django-develop...@googlegroups.com.
For more options, visit this group at http://groups.google.com/group/django-developers?hl=en.

[Sergej dergatsjev eecho]

+1

2010/7/16 Tobias McNulty <tob...@caktusgroup.com>:

--
Sergej Dergatsjev
E-mail: sergej.d...@gmail.com
Skype: SergejDergatsjev
Site: http://www.eecho.info

[Russell Keith-Magee]

Sounds reasonable to me. This is a useful feature of the most useful
caching backend library. It makes sense that we should support that
feature, and the API approach you describe makes sense to me.

If you open a ticket and provide a patch (with docs to describe the
change), then this sounds like a good addition.

Yours,
Russ Magee %-)

[Alex Gaynor]

> --
> You received this message because you are subscribed to the Google Groups "Django developers" group.
> To post to this group, send email to django-d...@googlegroups.com.
> To unsubscribe from this group, send email to django-develop...@googlegroups.com.
> For more options, visit this group at http://groups.google.com/group/django-developers?hl=en.
>
>

FTR this exact same concept has been filed before, and closed wontfix
on backwards compatibility concerns (see Mike Malone's scaling Django
slides, where he's mentioned this several times).

Alex

--
"I disapprove of what you say, but I will defend to the death your
right to say it." -- Voltaire
"The people's good is the highest law." -- Cicero
"Code can always be simpler than you think, but never as simple as you
want" -- Me

[Russell Keith-Magee]

Really? What concern? Can you point at a ticket discussion?

Yours,
Russ Magee %-)

[Will Hardy]

I thought this was familiar too: <http://code.djangoproject.com/ticket/6988>

Is this the ticket you were thinking of? It seems to have been reopened.

Cheers,

Will Hardy

[Tobias McNulty]

The only concern in that ticket seems to be that 0 means different things for different cache backends.

There may have been some effort towards making them all behave the same when 0 is passed.

Personally I prefer the approach of not messing with the value at all, and passing it straight to the configured cache backend.  You don't want Django messing with your .extra() or .raw(), so why should it try to magically alter the parameters you pass to your cache backend?

Furthermore, correct me if I'm wrong, but:

timeout = timeout or self.default_timeout

seems like an oversight on the part of whoever wrote that line originally, not some intended functionality to prevent 0 from ever making it to the actual backend.

Chances are, if you configured a cache backend for your Django project, you chose one explicitly and have some cognition of how it works.  We could could change line this to:

timeout = timeout is not None and timeout or self.default_timeout

and simply add a big fat warning to the docs stating that 0 means different things on different backends.

Thoughts?

Tobias

--

You received this message because you are subscribed to the Google Groups "Django developers" group.
To post to this group, send email to django-d...@googlegroups.com.
To unsubscribe from this group, send email to django-develop...@googlegroups.com.
For more options, visit this group at http://groups.google.com/group/django-developers?hl=en.

--
Tobias McNulty
Caktus Consulting Group, LLC
P.O. Box 1454
Carrboro, NC 27510
USA: +1 (919) 951-0052
http://www.caktusgroup.com

[Jacob Kaplan-Moss]

On Fri, Jul 23, 2010 at 8:17 AM, Tobias McNulty <tob...@caktusgroup.com> wrote:
> The only concern in that ticket seems to be that 0 means different things
> for different cache backends.
> There may have been some effort towards making them all behave the same when
> 0 is passed.
> Personally I prefer the approach of not messing with the value at all, and
> passing it straight to the configured cache backend.  You don't want Django
> messing with your .extra() or .raw(), so why should it try to magically
> alter the parameters you pass to your cache backend?

I agree.

> Furthermore, correct me if I'm wrong, but:
> timeout = timeout or self.default_timeout
> seems like an oversight on the part of whoever wrote that line originally,

I think that "whoever" was me, and I think you're right that it should
have been more like `timeout = self.default_timeout if timeout is None
else timeout` (or the equivalent 2.4 syntax).

I'd bless a patch making that simple fix. It's very slightly backwards
compatible, but almost certainly in a good way, so I'm fine with the
change.

Jacob

[Carl Meyer]

On Jul 23, 12:45 pm, Jacob Kaplan-Moss <ja...@jacobian.org> wrote:
> On Fri, Jul 23, 2010 at 8:17 AM, Tobias McNulty <tob...@caktusgroup.com> wrote:
> > The only concern in that ticket seems to be that 0 means different things
> > for different cache backends.
> > There may have been some effort towards making them all behave the same when
> > 0 is passed.
> > Personally I prefer the approach of not messing with the value at all, and
> > passing it straight to the configured cache backend.  You don't want Django
> > messing with your .extra() or .raw(), so why should it try to magically
> > alter the parameters you pass to your cache backend?
>
> I agree.

It's not obvious to me why .extra or .raw are the appropriate analogy
here, instead of the rest of the ORM API, which does attempt to
present the same semantics regardless of backend.

The argument that "you know what cache backend your project uses" does
not apply to the significant ecosystem of caching-related reusable
Django code: johnny-cache, cache-machine, cachebot, and more.

Last week I uploaded a patch to #6447 that maintains cross-backend
parity of allowed cache keys (without any performance hit on
memcached) by making sure the other backends will reject the same keys
memcached will reject. You approved this approach on IRC, Jacob, so
given the discrepancy I'm just curious whether it is in fact a design
goal to have semantic parity between the backends shipped with Django.

If it is, the memcached backend should probably still be changed to
the native behavior (for speed reasons, at least; the less code in the
memcached backend, the better), but it may be worth considering
emulating that same behavior in the other backends.

Carl

[Tobias McNulty]

On Sat, Jul 24, 2010 at 4:07 PM, Carl Meyer <carl.j...@gmail.com> wrote:

It's not obvious to me why .extra or .raw are the appropriate analogy

here, instead of the rest of the ORM API, which does attempt to
present the same semantics regardless of backend.

The issue is about values passed, not about semantics.  I admit, it wasn't a great analogy to begin with, but it does hold for other aspects of the ORM as well (e.g., you wouldn't want values that you set to 0 converted to None on model.save()).

The argument that "you know what cache backend your project uses" does
not apply to the significant ecosystem of caching-related reusable
Django code: johnny-cache, cache-machine, cachebot, and more.

True, hence the proposed big note in the docs stating that the value 0 should be used with care.

If such reusable code needs to cache things forever, which sounds perfectly reasonable to me, I'd still rather not co-opt "0" to mean "forever" in all cases.  Each backend that supports caching with no timeout could easily offer a class attribute, such as "TIMEOUT_FOREVER", that indicated how to set the timeout in these cases.

 

Last week I uploaded a patch to #6447 that maintains cross-backend
parity of allowed cache keys (without any performance hit on
memcached) by making sure the other backends will reject the same keys
memcached will reject. You approved this approach on IRC, Jacob, so
given the discrepancy I'm just curious whether it is in fact a design
goal to have semantic parity between the backends shipped with Django.

Rejecting a particular subset of cache keys is also not the greatest analogy to co-opting configuration values. :-)

Cheers,

Tobias

[Carl Meyer]

On Jul 26, 10:02 am, Tobias McNulty <tob...@caktusgroup.com> wrote:
> If such reusable code needs to cache things forever, which sounds perfectly
> reasonable to me, I'd still rather not co-opt "0" to mean "forever" in all
> cases.  Each backend that supports caching with no timeout could easily
> offer a class attribute, such as "TIMEOUT_FOREVER", that indicated how to
> set the timeout in these cases.

I'm not sure I see the problem with co-opting 0, since it already has
(or should have) that meaning on the most commonly-used backend, and
doesn't really have any other sensible meaning as a cache timeout
value. But the proposed class attribute is fine too.

> Rejecting a particular subset of cache keys is also not the greatest analogy
> to co-opting configuration values. :-)

The common thread, of course, is making it possible to write reusable
caching code without special-casing particular backends.

Carl

[SmileyChris]

On Jul 27, 9:22 pm, Carl Meyer <carl.j.me...@gmail.com> wrote:
> The common thread, of course, is making it possible to write reusable
> caching code without special-casing particular backends.

I agree with Carl.
We have an abstracted api - having a property with different meanings
for different backends makes things a lot less pluggable.

[Tobias McNulty]

On Tue, Jul 27, 2010 at 4:04 PM, SmileyChris <smile...@gmail.com> wrote:

I agree with Carl.
We have an abstracted api - having a property with different meanings
for different backends makes things a lot less pluggable.

Sure.  Upon closer investigation, I think this is pretty much a non-issue.

My only point is that all cache backends should receive whatever timeout they're passed, unmolested by Django.

Given that all the backends, besides memcache, are implemented in the Django source, I see no reason why they couldn't be modified on a case by case basis to support a "cache forever" option, indicated by timeout=0, while leaving the common code intact and without really co-opting anything.  Establishing this as a loose convention seems like a reasonable enough plan to me.

I will say that the memcache issue seems more like a bug, while special-casing timeout=0 for all the other backends seems more like a feature (and one that may require a lot more discussion, code, and testing relative to the 1-line memcache fix).

Cheers,

Tobias