Closed (outdated)
Project:
Drupal core
Version:
9.5.x-dev
Component:
base system
Priority:
Normal
Category:
Task
Assigned:
Reporter:
Created:
25 Oct 2007 at 19:43 UTC
Updated:
20 Jul 2023 at 12:05 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #1
catchPatch checks cache_lifetime_$table_$cid then cache_lifetime_$table then cache_lifetime. This would allow contrib to set cache lifetime on a per block basis. It would also allow just for basic stuff like having block caching set at 2 minutes and page caching at 30 minutes.
Was going to do this with a big ternary but Damz and Berdir in irc suggested the nested variable_get() calls instead. This might be done for free if we can get hook_variable_info() in with fallbacks.
Comment #2
moshe weitzman commentedLooks reasonable to me. Passes all tests, so I will rtbc after a couple of days unless ill comments arrive.
Comment #3
swentel commentedI like it, I created a D7 branch of block cache alter which allready profits if #186636: Block caching when node access modules are enabled. gets in. I'm hoping to try and apply this patch and see if I can make the module profit from it too - which it should normally.
Comment #5
catchRe-rolled.
Comment #6
moshe weitzman commentedcode looks nice and gives us some needed flexibility with cache expiry.
Comment #7
dwwSure, that'd help. However, it's really just playing with your food so long as setting "minimum" cache lifetime actually sets a maximum cache lifetime, and plugable cache backends are free to interpret cache lifetimes as a hint, not a "this data should persist until X" directive. So, great, you can now have a lot more variables to tell the system "please keep block X around for an hour", but your cache is cleared all the friggin' time anyway, so what does it really help?
See #458808: Add a way to specify CACHE_PERMANENT for block caching for part of the story. I don't know where the issue is for "don't clear all cache entries after the minimum cache lifetime expires". There's #227228: cache_clear_all and cache_get fail to clear caches when Minimum cache lifetime is on but that's the other side of the bug. Minimum cache lifetime is just all kinds of broken. :(
Bigger picture, I don't know if there's already an issue for "make the plugable cache backend system support both volatile and nonvolatile caches", but we desperately need that. As it is now, the main alternative for core's cache.inc is memcache, which entirely assumes volatile caching. There's no way to specify certain cache tables or cids need to be treated nonvolatile. This patch here is sort of a step in the right direction, in that at least you can specify a fine-grained hint. But, so long as everything can assume all caching is volatile, we still can't (or shouldn't) use the core cache API for things like {cache_form}, {cache_update} (#220592-52: Core cache API breaks update.module: fetches data way too often, kills site performance, etc), etc. :(
Comment #8
catchLosing track of my patches so need to start using the assigned field :(
Comment #9
dries commentedI'm not sure this is the best solution -- it sounds a bit like a quick hack. For example, it makes the cache lifetime setting on the Performance settings page unreliable and confusing. I wonder if we need to skip this patch, and rework the cache lifetime code in a bit more drastic fashion? Based on #7, I think @dww might be sharing the same feelings. Let's discuss this some more.
Comment #10
catchHaving spoken to dww, I'm starting to think we need a "keep until" column in each cache table and remove minimum_lifetime altogether.
So in cache_set() you could specify a timestamp (say time() + 86400), and unless the cache entry gets specifically cleared by cid, or the entire cache table is truncated, it's kept for that long.
How exactly that value gets set, what happens when it's smaller than the $expires value, whether we give admins any control over it (i.e. page caching), I'm not sure about though.
Comment #11
catchComment #26
quietone commentedI confirmed with catch that this is now outdated.