Opened 8 years ago

Closed 8 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 Patryk Zawadzki, 8 years ago

Has patch: set

comment:2 by Tim Graham, 8 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 Patryk Zawadzki, 8 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 Keryn Knight, 8 years ago

Cc: django@… 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 Patryk Zawadzki, 8 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 Claude Paroz, 8 years ago

Resolution: duplicate
Status: newclosed

This is a duplicate of #15894.

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