Closed (won't fix)
Project:
Nodequeue
Version:
7.x-2.x-dev
Component:
Code
Priority:
Major
Category:
Bug report
Assigned:
Unassigned
Reporter:
Created:
3 Jan 2008 at 16:53 UTC
Updated:
11 Feb 2015 at 21:58 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #1
merlinofchaos commentedI'm guessing that you have page caching turned on for anonymous users.
Nodequeue currently doesn't perform a cache clear on the page cache when it makes changes of this nature; however, that's also kind of an expensive operation, so in a lot of cases we don't actually want to. We'll have to come up with a solution that will both work in your case and not add extra burden to other cases. Possibly by adding a checkbox somewhere.
A temporary workaround would be to manually clear the cache after making your changes (I realize it's not ideal but it will at least allow your changes to be seen). You can do this in two ways; 1) using the devel module, you can use its 'empty cache' option, or 2), you can go directly into your database and enter the command "TRUNCATE cache_page" (if you are using database prefixing youll have to add your prefix to that as well).
Comment #2
moshe weitzman commentedI guess nodequeue could provide a clear cache button. just use the same hook that devel uses.
Comment #3
moondancerjen commentedThanks for your help. We did have page-caching turned on for anonymous users, and just determined to turn that off, and it solved the problem!
Comment #4
merlinofchaos commentedComment #5
ezra-g commentedHow about a configurable option for each queue where administrators can specify the maximum amount of time that can pass since the queue or any of its subqueues was is updated and the page cache is cleared. On each crun run, nodequeue could decide to clear the page cache.
Allowing each queue to have a unique lifetime allows changes to more important queues, (ie "Frontpage Queue") to be visible to anonymous users more quickly without clearing the cache each time a less important queue (ie "Joe User's personal favorites queue") is updated. Having it be cron-based prevents administrators from having to keep track of queue changes and clear the page cache manually.
If Merlin approves, I'll submit a feature request and put forth a patch.
Comment #6
ezra-g commentedJust bumping this a little bit in case I finish the GSOC with extra time. Does this proposal sound like a good idea?
Comment #7
merlinofchaos commentedezra: That's a nifty idea. +1.
Comment #8
ezra-g commentedThe feature request is at #293312: Per-queue page cache lifetime to be cleared on cron run. . Leaving this bug report open in the meantime.
Comment #9
ezra-g commentedComment #10
tsokura commentedCache invalidation should really be automatic in cases where nodequeues are changed. Or at least provide a setting for administrators to be able to choose whether to do it automatically or not.
We have a pretty static front page and it can take ages for changes to nodequeues (lead story teaser, for example) to propagate if caches aren't cleared manually. The editors shouldn't need to know to press "clear caches" button every time they change something. Actually they don't even have the necessary permissions to do that..
Comment #11
amateescu commentedUntil nodequeue gets some support for cache invalidation, you can do this in a custom module with Cache Actions and the hooks provided by nodequeue (hook_nodequeue_add, hook_nodequeue_remove, hook_nodequeue_swap, hook_nodequeue_sort_alter).
Marking as a duplicate of #293312: Per-queue page cache lifetime to be cleared on cron run. because we don't need two issues for the same feature request.
Comment #12
joachim commentedThe feature requested at #293312: Per-queue page cache lifetime to be cleared on cron run. is rather heavy for the problem.
node module just clears all caches when a node is saved for whatever reason, because that is going to invalidate caches of not just that page but all sorts of pages node module can't know about.
I would suggest the simple and correct fix here is to do the same: when a nodequeue's contents are changed (added, removed, reordered), clear all caches.
Comment #13
joachim commentedEven this simple fix is actually harder than it sounds, as there are so many API functions that affect queues in one way or another.
My take on it is that it would make more sense if all functions that change the queue end in a nodequeue_save(), which then takes care of clearing caches.
Here's a first stab at it which fixes the problem with reordering items in a queue. There are probably other workflows it's not catching :)
Comment #15
jennypanighetti commentedWas this issue ever resolved or this patch ever put into the module? I'm running into a similar issue in 6.x-2.11.
1. Cron is running every hour yet the nodequeue contents are two days old for anonymous users.
2. I'd like to clear the nodequeue block cache upon every update, as is proposed (but hasn't yet been assigned)
Comment #16
joachim commented> Was this issue ever resolved or this patch ever put into the module?
No, it's not in the module yet. The issue status tells you what stage this is at: 'needs work'. When it's in, the maintainer will change it to 'fixed' ;)
Looks like the patch needs a reroll.
Comment #17
joachim commentedThis is still an issue in the latest version.
Comment #18
joachim commentedBetter title.
I think the right fix is to give nodequeues a setting to allow them to clear caches when they get updated.
Comment #19
ibonelli commentedWe are getting complains about caching from clients and I decided to take the matter on my own hands. I tried a couple of approaches but in the end decided to follow joachim suggestion.
I added a configuration that when enable will trigger a cache_clear_all() call with each form submit request. In some cases I end up with two calls at the same time depending what I'm saving, but it is a price I'm willing to pay to avoid current problems (and alternative solutions).
Alternative suggested approaches (without code) would have been to use rules, but not all our sites have rules and this patch is short and straight forward. Another one would be to simply clear cache with every cron run, and make cron run more often. But it seemed more demanding for the server and decided to use this approach instead.
Patch applies smoothly to nodequeue-7.x-2.0-beta1 within includes directory. I just realized I forgot to include the path on the patch. Will try to do that later.
Looking forward to hearing comments!
Comment #20
ibonelli commentedAs said I want to be able to nodequeue-7.x-2.0-beta1, so that's the patch I issued. But I guess for inclussion it would be best to apply to dev branch 7.x-2.x, so here it is that patch. Cheers!
Comment #21
skruf commentedOne way around this is to add a custom submit handler to the subqueue form. Then clear cache in the submit handler.
Comment #22
ibonelli commentedYes, I agree. I tried that first, but I couldn't make the handler work.
I didn't have time to debug properly and decided to follow joachim suggested path, which was easier and worked much faster. But I agree with you, that would be a cleaner approach. Cheers!
Comment #23
davidwbarratt commentedAs joachim pointed out in #12. I don't think that clearing the cache everytime the queue is updated is a terrible idea, since that's exactly what node module does:
https://api.drupal.org/api/drupal/modules%21node%21node.pages.inc/functi...
I'm not even sure it's worth giving the user a choice. I do think the clear cache operation should be attached to the submit handler of a form and not an api/function (if it is, it might get called more than one time).
Comment #24
othermachines commentedFurther to @davidwbarratt's comment (#23) and for the sake of moving things along, seems to me we can resolve this with a simple
cache_clear_all()in all of the important submit handlers. It won't do any harm (I think?) and can be revisited later if necessary.This patch is essentially the same as @ibonelli's except it removes the config option and replaces his custom function with
cache_clear_all().Comment #25
fizk commentedI've read and re-read this issue trying to decide if we should forcibly clear all the caches. In the end I don't think this is the best approach overall because it doesn't work for everyone.
With this approach, if users don't want all caches cleared when updating node queues, they will have to hack the module to remove this functionality. A better, more flexible approach would be to use Rules. The following issues are working on adding Rules support:
#1125922: Rules integration
#952448: Add rules events support
Comment #26
othermachines commentedHmm. I was actually going to recommend this be a release blocker considering it has caused quite a bit of trouble for people.
I hope you don't mind if I offer up a couple of cents. Feel free to disregard.
If the solution becomes Rules integration, that should make #1125922: Rules integration a release blocker, and that could hold things off for a long time. (I was actually considering submitting my own patch in the next few days but of course the review process can go on.)
I don't think there have been many complaints about node module's
cache_clear_allevery time a node is saved, so I'm having a hard time seeing how it could be problematic here. (It's possible I'm missing something entirely here so please set me straight if that is the case!)On the flip side, when a change isn't registered on the front end because it's stuck in cache that can be problematic and potentially embarrassing for site builders (as demonstrated above).
Also, a lot of people don't like Rules. I LOVE Rules, but it isn't the easiest system to wrap your head around. IMHO it's too complex an answer for such a simple (yet serious) problem.
/ .02
Edit: I think there's some progress being made at #952448: Add rules events support so I've closed #1125922: Rules integration as a dupe.