[Django] #22845: Memcached backend handles infinite (None) timeout defined in default settings improperly
Django
settings improperly
-------------------------------------+------------------------------
Reporter: Althalus | Owner: nobody
Type: Bug | Status: new
Component: Core (Cache system) | Version: master
Severity: Normal | Keywords: cache, memcached
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+------------------------------
#9595 introduced a way to use `None` as way to set infinite memcached
timeout.
It works fine while you specify this timeout explicitly like
`cache.set('key', 'val', None)`.
But it works wrong if you define it as a default cache timeout in
django.conf.settings.CACHES.
Consider the following parts:
{{{#!python
# base.py
class BaseCache(object):
def __init__(self, params):
timeout = params.get('timeout', params.get('TIMEOUT', 300))
if timeout is not None:
try:
timeout = int(timeout)
except (ValueError, TypeError):
timeout = 300
self.default_timeout = timeout
}}}
{{{#!python
# memcached.py
def get_backend_timeout(self, timeout=DEFAULT_TIMEOUT):
"""
Memcached deals with long (> 30 days) timeouts in a special
way. Call this function to obtain a safe value for your timeout.
"""
if timeout == DEFAULT_TIMEOUT:
return self.default_timeout
if timeout is None:
# Using 0 in memcache sets a non-expiring timeout.
return 0
elif int(timeout) == 0:
# Other cache backends treat 0 as set-and-expire. To achieve
this
# in memcache backends, a negative timeout must be passed.
timeout = -1
if timeout > 2592000: # 60*60*24*30, 30 days
# See http://code.google.com/p/memcached/wiki/FAQ
# "You can set expire times up to 30 days in the future. After
that
# memcached interprets it as a date, and will expire the item
after
# said date. This is a simple (but obscure) mechanic."
#
# This means that we have to switch to absolute timestamps.
timeout += int(time.time())
return int(timeout)
}}}
As you can see special handling of `None`, `0` and long timeouts is
performed in `get_backend_timeout`.
But it only handles explicit timeouts. For default timeout there's no
special processing while it's needed - timeout is just taken from
`BaseCache.__init__`.
So when trying to set `None` as default timeout django just transfers it
to the memcached driver and which leads to errors.
Also when timeout is set to some high value (more then 30 days) it is not
converted to timestamp as memcached requires. In my case it caused all my
cache entries to expire immidiately.
--
Ticket URL: <https://code.djangoproject.com/ticket/22845>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
settings improperly
Reporter: Althalus | Owner: nobody
Type: Bug | Status: new
Component: Core (Cache system) | Version: master
Keywords: cache, memcached | Triage Stage:
Needs tests: 0 | Needs documentation: 0
Easy pickings: 0 | Patch needs improvement: 0
| UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Althalus):
* needs_better_patch: => 0
* has_patch: 0 => 1
* severity: Normal => Release blocker
* needs_tests: => 0
* needs_docs: => 0
Comment:
In django 1.6 docs there's no information about infinite (`None`) cache
timeout (though code to use it was already delivered).
While in 1.7 it's described properly so I think we can consider it as a
new feature. Assuming that this ticket should be a release blocker.
[https://docs.djangoproject.com/en/1.7/topics/cache/#cache-arguments].
Also I've made a small and quite straightforward pull request:
https://github.com/django/django/pull/2817
On my current project that patch worked as expected.
--
Ticket URL: <https://code.djangoproject.com/ticket/22845#comment:1>
Django
settings improperly
Reporter: Althalus | Owner: nobody
Type: Bug | Status: new
Component: Core (Cache system) | Version: master
Keywords: cache, memcached | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 1 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timo):
* needs_tests: 0 => 1
* stage: Unreviewed => Accepted
Comment:
[6fe22b30] is the commit that introduced the new behavior. The pull
request lacks a test.
--
Ticket URL: <https://code.djangoproject.com/ticket/22845#comment:2>
Django
settings improperly
Reporter: Althalus | Owner: nobody
Type: Bug | Status: new
Component: Core (Cache system) | Version: master
Keywords: cache, memcached | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Althalus):
* needs_tests: 1 => 0
Comment:
Added 2 regression tests. All cache tests on my local machine run
successfully (debian / python 2.7 / python-memcached).
--
Ticket URL: <https://code.djangoproject.com/ticket/22845#comment:3>
Django
settings improperly
Reporter: Althalus | Owner: nobody
Type: Bug | Status: closed
Component: Core (Cache system) | Version: master
Keywords: cache, memcached | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham <timograham@…>):
* status: new => closed
* resolution: => fixed
Comment:
In [changeset:"4529af9ecf7debc9281021462531ec4d9697013b"]:
{{{
#!CommitTicketReference repository=""
revision="4529af9ecf7debc9281021462531ec4d9697013b"
Fixed #22845 -- Correctly handled memcached default timeout value.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/22845#comment:4>
Django
settings improperly
Reporter: Althalus | Owner: nobody
Type: Bug | Status: closed
Component: Core (Cache system) | Version: master
Keywords: cache, memcached | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tim Graham <timograham@…>):
In [changeset:"6e248d8f8c95da9cbd654d43df76c20e6d0ec352"]:
{{{
#!CommitTicketReference repository=""
revision="6e248d8f8c95da9cbd654d43df76c20e6d0ec352"
[1.7.x] Fixed #22845 -- Correctly handled memcached default timeout value.
Backport of 4529af9ecf from master
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/22845#comment:5>
Django
settings improperly
Reporter: Althalus | Owner: nobody
Type: Bug | Status: closed
Component: Core (Cache system) | Version: master
Keywords: cache, memcached | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tim Graham <timograham@…>):
In [changeset:"8568e7cfa4fb87cd0a9ead1b3fbb7a39d19e98b9"]:
{{{
#!CommitTicketReference repository=""
revision="8568e7cfa4fb87cd0a9ead1b3fbb7a39d19e98b9"
Added backwards incompatibility note for refs #22845; refs #23082.
Thanks Kyle Owens for the report.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/22845#comment:6>
Django
settings improperly
Reporter: Althalus | Owner: nobody
Type: Bug | Status: closed
Component: Core (Cache system) | Version: master
Keywords: cache, memcached | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tim Graham <timograham@…>):
In [changeset:"663af270b58f350bcbd9df5cd04ffe69e8ac37c8"]:
{{{
#!CommitTicketReference repository=""
revision="663af270b58f350bcbd9df5cd04ffe69e8ac37c8"
[1.7.x] Added backwards incompatibility note for refs #22845; refs #23082.
Thanks Kyle Owens for the report.
Backport of 8568e7cfa4 from master
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/22845#comment:7>