Don't invalidate menu_cache on Page changes #1774

Open
benliles opened this Issue May 7, 2013 · 12 comments

Comments

Projects
None yet
9 participants
Contributor

benliles commented May 7, 2013

The menu cache currently gets cleared when pages are changed or deleted by connecting to the pre_save and post_delete signals.

This is an expensive operation when a site is large and there are timeouts on the cache to handle flushing these changes in an appropriate amount of time.

Contributor

benliles commented May 8, 2013

By the way, just discovered that this is getting done in the Page.save method as well which means that it happens multiple times.

Collaborator

ojii commented May 10, 2013

The current strategy is to have the cache-timeout as high as possible, as the building of the menu is rather expensive as you noted. Short of more fine-grained caching or a way to pre-fill the cache, I don't see us changing this strategy any time soon.

Contributor

benliles commented May 10, 2013

I don't have an issue with high cache-timeouts, I like them. My issue is
that I'm willing to accept an inconsistent menu for that length of time
instead of clearing the cache on every change.

The time issues I'm running into aren't when the menu is rebuilt, it's that
clearing the menu in cache is expensive and done multiple times.

On Fri, May 10, 2013 at 4:29 AM, Jonas Obrist notifications@github.comwrote:

The current strategy is to have the cache-timeout as high as possible, as
the building of the menu is rather expensive as you noted. Short of more
fine-grained caching or a way to pre-fill the cache, I don't see us
changing this strategy any time soon.


Reply to this email directly or view it on GitHubhttps://github.com/divio/django-cms/issues/1774#issuecomment-17711675
.

I saw that the menu cache is not refresh when you just disable a page to "show on menu" and when you just "edit" her.

digi604 added this to the 3.0 milestone Feb 10, 2014

digi604 added the blocker label Mar 12, 2014

Contributor

johnraz commented Mar 12, 2014

@digi604 , @ojii : I would like to investigate this issue, do you have recommendation on the way to go?
Should we add a "clear cache" option in the toolbar ? add a setting to make this refresh on page change optional ?

Member

digi604 commented Mar 12, 2014

the chache version needs to be increased if something 'relevant' happens.

Contributor

benliles commented Mar 12, 2014

I don't use django-cms anymore, but for the use case I had, a delay of
minutes on changes was sufficient and the cache clears were triggered
multiple times and were expensive.
On Mar 12, 2014 3:45 PM, "Patrick Lauber" notifications@github.com wrote:

the chache version needs to be increased if something 'relevant' happens.

Reply to this email directly or view it on GitHubhttps://github.com/divio/django-cms/issues/1774#issuecomment-37478562
.

Contributor

johnraz commented Mar 12, 2014

@digi604 : define relevant ? :) Should we only "fix" the fact that the cache refresh is triggered multiple times?

Member

digi604 commented Mar 13, 2014

on the page:

  • if a page moves that is in the menu
  • if in menu changes
  • if application_urls or application_namespace changes
  • if soft_root changes
  • if navigation_extenders changes
  • if limit_visibility_in_menu changes

on title:

  • if title or menu_title changes
  • if path changes

multiple cache clears surely should be avoided

Contributor

johnraz commented Mar 13, 2014

ack - putting this on my todo list

digi604 removed the blocker label Mar 14, 2014

Collaborator

mkoistinen commented Mar 29, 2014

Hey all, just to provide some info to this. i recently added/updated, the page cache so that it uses a "Cache Versioning Strategy", so, this is about as inexpensive as it can get for cache invalidation, since everything, including the very keys we use for the cache, are stored in the cache itself.

Read here for more detail: https://github.com/divio/django-cms/blob/develop/cms/views.py#L296

I recently looked into retrofitting the menu cache to use a similar strategy. Unfortunately, such a change will not be a trivial exercise since the current menu cache mechanism needs to perform queries to be able to find the correct keys to clear. This is because there are multiple "dimensions" that the cached menu can exist within [ menu × language × site × ? ]. The existing code uses the database to store the keys, but more importantly, to provide the query mechanism required to find the keys to invalidate.

Perhaps someone with more brains and/or more determination than I can figure out how to implement this querying capability with the cache versioning strategy, then, invalidating the menu cache will be tremendously more efficient to the point that performing hundreds of invalidations back-to-back will have virtually no performance impact, and thereby addressing @benliles’ requirements and will generally make the whole menu cache more performant.

@yakky yakky modified the milestone: 3.0.X, 3.0 May 1, 2014

@yakky yakky modified the milestone: 3.0.X, 3.0.6 Oct 1, 2014

@yakky yakky modified the milestone: 3.0.X, 3.X Oct 17, 2014

mkoistinen added the menus label Oct 26, 2014

FinalAngel referenced this issue Oct 20, 2015

Open

[META] Menus/pagetree refactor #4597

1 of 9 tasks complete
Owner

FinalAngel commented Oct 20, 2015

closed in favour of #4597

FinalAngel closed this Oct 20, 2015

czpython referenced this issue Mar 29, 2016

Open

[META] Menus cleanup #5112

0 of 26 tasks complete

czpython reopened this Mar 29, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment