Opened 9 years ago
Closed 9 years ago
#25783 closed Cleanup/optimization (duplicate)
Site.objects.get_current() should use Django's cache framework
Reported by: | Patryk Zawadzki | Owned by: | nobody |
---|---|---|---|
Component: | contrib.sites | Version: | 1.8 |
Severity: | Normal | Keywords: | |
Cc: | django@… | Triage Stage: | Unreviewed |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
Currently sites are cached using a module global. This solution makes it impossible to edit the site object in multiprocess environments as it's impossible to notify other processes that they need to invalidate local caches. Having to restart all servers/workers after a database operation is not intuitive.
Change History (6)
comment:1 by , 9 years ago
Has patch: | set |
---|
comment:2 by , 9 years ago
I'm not sure about this. The caching was added in #3766 to avoid database queries but with this change I think anyone using the database cache backend no longer gets any benefit.
comment:3 by , 9 years ago
I've filed this under “cleanup/optimization” but it also qualifies as a bug report. In any environment that use more than one process the cache will not invalidate properly.
Consider two worker processes using the same SITE_ID
. If one of them updates current Site
object's name
field, only this process will see signals being fired and only its own local cache will be invalidated. For every second request the old name will be seen.
Django offers no inter-process messaging to notify other workers that their caches are no longer valid. To properly work around this problem you need to implement your own get_current()
(using a shared-state cache like the cache framework) and monkey-patch either Django or your third-party dependencies.
What's worse, runserver
does not use multiple processes. The buggy behaviour is isolated to production environments and is impossible to reproduce locally.
comment:4 by , 9 years ago
Cc: | added |
---|
Additional point for consideration: the default setting for CACHES['default']
is LocMemCache
, which would have the same problem as the current situation where other processes would not be notified of the change, as far as I know.
comment:5 by , 9 years ago
CACHES['default']
is something you control. If you use multiple processes, you should set it to a shared-state storage. This is something that Django should document and recommend.
comment:6 by , 9 years ago
Resolution: | → duplicate |
---|---|
Status: | new → closed |
This is a duplicate of #15894.
Pull request: https://github.com/django/django/pull/5692