I see several small problems when using memcache and making changes to the bibliographic frameworks: Changes are not reflected immediately, but only after restarting the memcache server. This is a problem, because often you want to configure and immediately check if your changes are correct by refreshing the cataloging screen. At the moment this is really confusing. I am not sure when or if the cache would be updated without restarting memcache. To reproduce: 1) Open cataloguing default framework 2) Change bibliographic framework, for example: set 773$a hidden value from -6 to 0 3) Refresh cataloguing page > change and 773 will not be visible 4) Restart memcache > change will be visible
This is a bug in the way we are using memoize_memcached I will do patches to fix it in all the places we use it. The first one will be for C4/Biblio and will show what we are doing wrong.
Now they will only be cached for 10 mins max, so will reflect the change without needing a restart. I will do this for all instances of memoize_memcached and attach them here.
Created attachment 7426 [details] [review] Bug 7432 : Fix how we are setting expiry time when caching
Created attachment 7427 [details] [review] Bug 7432 : Fixing caching for SQLHelper
Created attachment 7428 [details] [review] Bug 7432 : Fixing caching for SQLHelper
Created attachment 7429 [details] [review] Bug 7432 : Fixing caching for C4::Languages
Looks like all these patches fold the expire_time into the memcached hash, instead of as as separate param to memoize_memcached(). That's what's expected by the function according to CPAN, so I think this is a valid change. Chris, your commit messages for these changes pretty empty. Could you provide a test plan so we can confirm this is working as expected?
(In reply to comment #7) > Looks like all these patches fold the expire_time into the memcached hash, > instead of as as separate param to memoize_memcached(). That's what's expected > by the function according to CPAN, so I think this is a valid change. > > Chris, your commit messages for these changes pretty empty. Could you provide a > test plan so we can confirm this is working as expected? Seriously? Ok, switch on caching, go to cataloguing, pick a framework, look at it. Change the framework, check it again, it wont have have changed in cataloguing. Wait 10 minutes look again. Currently it never ever changes unless you restart memcached. Katrin already has the test plan, use that, except that after 10 minutes the change will show instead of having to restart memcached.
Created attachment 7585 [details] [review] Bug 7432 : Fix how we are setting expiry time when caching Signed-off-by: Jared Camins-Esakov <jcamins@cpbibliography.com>
Created attachment 7586 [details] [review] Bug 7432 : Fixing caching for SQLHelper Signed-off-by: Jared Camins-Esakov <jcamins@cpbibliography.com>
Created attachment 7587 [details] [review] Bug 7432 : Fixing caching for C4::Languages Signed-off-by: Jared Camins-Esakov <jcamins@cpbibliography.com>
2 patches set > expire_time => 600 saying it's 10mn One patch (http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=7586) set > expire_time => 600000 also saying it's 10mn I suspect the 60000 is a bug, marking failed QA: pls explain or follow-up, thx (I tried to dig into Cache::Memcached documentation, and found: The $exptime (expiration time) defaults to "never" if unspecified. If you want the key to expire in memcached, pass an integer $exptime. If value is less than 60*60*24*30 (30 days), time is assumed to be relative from the present. If larger, it's considered an absolute Unix time.
Yes the 60000 is a bug, thats why i changed it to 600 (someone elses patch had set it to 60000 so i fixed it while fixing the fact it was set in the wrong place) If you look carefully this patch is changing it from 60000 to 600.
OK, /me must change his glasses... passed QA
This fix was included in the 3.6.x branch prior to 3.6.4.