django-redis-cache icon indicating copy to clipboard operation
django-redis-cache copied to clipboard

get_or_set sometimes returns None

Open gautamg795 opened this issue 8 years ago • 4 comments

Issue is as titled—sometimes (notably when under heavy load), get_or_set returns None. I believe the issue is in the if statement here (redis_cache/backends/base.py#L415) It seems that if the value is not found in the cache, but the dogpile_lock is not None (I assume this means the lock could not be acquired?), nothing is returned.

I think a better way to handle this would be to just evaluate the callable and return that value. From what I understand, this is how the bug was fixed in Django source.

gautamg795 avatar Feb 16 '17 00:02 gautamg795

I was surprised as well but I believe this is intended behavior: http://www.sobstel.org/blog/preventing-dogpile-effect/ That's the purpose of the dogpile effect.

I think the best behavior we could have here would be to serve the stale value, as mentioned in the link below. However when we're in the set part of the get_or_set(), the value already expired in Redis so we don't have access to the stale value.

I don't know if there is any easy way of implementing the "return stale value" behavior.

lraucy avatar Feb 28 '17 10:02 lraucy

Sorry I'm not very responsive. I have 5 month old twins, so my open source work has suffered a bit as you can imagine. I'll try and fix this in the next couple weeks or if either of you submit a pull request, I'll try and make time to review it.

sebleier avatar Feb 28 '17 20:02 sebleier

This is still an issue, even though the get_or_set function has since changed. The issue is that the lock timeout is longer than the value timeout, and if the lock exists without a value, then it returns None. Even though the intention may have been to provide a more advanced implementation, it doesn't really adhere to the Django spec.

jacobg avatar Jul 19 '22 17:07 jacobg

I believe we've come up against this issue whereby .get_or_set() returns None rather than default (and doesn't set default) after a .delete().

In [69]: cache.get("test3")                                                                                                              

In [70]: cache.get("test3", "qwe")                                                                                                       
Out[70]: 'qwe'

In [71]: cache.get_or_set("test3", "qwer")                                                                                               
Out[71]: 'qwer'

In [72]: cache.get_or_set("test3", "qwert")                                                                                              
Out[72]: 'qwer'

In [73]: cache.delete("test3")                                                                                                           
Out[73]: 1

In [74]: cache.get_or_set("test3", "qwerty")                                                                                             

In [75]:     

Django 4.0 packs a redis backend making this library unneeded so it's possible we could "simply" upgrade, but we're exploring alternative possibilities.

Asday avatar Sep 06 '22 14:09 Asday