Summary: | Changing frameworks should refresh cache | ||
---|---|---|---|
Product: | Koha | Reporter: | Katrin Fischer <katrin.fischer> |
Component: | System Administration | Assignee: | Paul Poulain <paul.poulain> |
Status: | CLOSED FIXED | QA Contact: | Ian Walls <koha.sekjal> |
Severity: | critical | ||
Priority: | P1 - high | CC: | chris, gmcharlt, jcamins |
Version: | 3.6 | ||
Hardware: | All | ||
OS: | All | ||
Change sponsored?: | --- | Patch complexity: | --- |
Documentation contact: | Documentation submission: | ||
Text to go in the release notes: | Version(s) released in: | ||
Attachments: |
Bug 7432 : Fix how we are setting expiry time when caching
Bug 7432 : Fixing caching for SQLHelper Bug 7432 : Fixing caching for SQLHelper Bug 7432 : Fixing caching for C4::Languages Bug 7432 : Fix how we are setting expiry time when caching Bug 7432 : Fixing caching for SQLHelper Bug 7432 : Fixing caching for C4::Languages |
Description
Katrin Fischer
2012-01-10 07:21:53 UTC
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. |