[Bug 10503] New: incorrect initialization parameters passed Memoize::Memcached
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10503 Bug ID: 10503 Summary: incorrect initialization parameters passed Memoize::Memcached Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: normal Priority: P5 - low Component: Architecture, internals, and plumbing Assignee: gmcharlt@gmail.com Reporter: gmcharlt@gmail.com Currently, Memoize::Memcached is initialized like this: eval { if (C4::Context->ismemcached) { require Memoize::Memcached; import Memoize::Memcached qw(memoize_memcached); memoize_memcached( 'GetMarcStructure', memcached => C4::Context->memcached); } }; C4::Context->memcached returns a Cache::Memcached object. However, that's not what Memoize::Memcached expects: This function also accepts a memcached option, which expects a hashref. This is de-referenced and passed directly into an internal function which sets up the memcached configuration for that function. This contents of this hashref are mostly options passed to Cache::Memcached, with a few exceptions. However, passing a Cache::Memcached happened to work, by coincidence, as Memoize::Memcached was calling UNIVERSAL::isa (in a deprecated fashion [0]) to check to see whether the passed argument is a HASH. Nowadays, though, since UNIVERSAL->import is deprecated, the version of Memoize::Memcached packaged for Debian Wheezy now uses ref() to check whether the argument is a hash [1]. That means that the coincidence no longer works, and consequently MARC frameworks don't get cached via memcached in Debian Wheezy, leading to a performance degradation. The solution is to pass Memoize::Memcached what it needs -- a hashref with the Cache::Memcached arguments -- and not a Cache::Memcached object. [0] https://rt.cpan.org/Public/Bug/Display.html?id=68170 [1] http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=614868 -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10503 Galen Charlton <gmcharlt@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Summary|incorrect initialization |incorrect initialization |parameters passed |parameters passed to |Memoize::Memcached |Memoize::Memcached -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10503 Galen Charlton <gmcharlt@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10503 --- Comment #1 from Galen Charlton <gmcharlt@gmail.com> --- Created attachment 19232 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=19232&action=edit bug 10503: fix various issues with use of Memoize::Memcached Memoize::Memcached expects to be passed a hashref of connection parameters, which it in turn uses to initialize a Cache::Memcached object. However, commit 52afe06ddd started passing a Cache::Memcached object directly, not the required hashref. This happened to work by coincidence, but no longer works with recent versions of Memoize::Memcached packaged for Debian. This patch introduces a new C4::Context function, memcached_params(), that returns a hashref of connection parameters suitable for passing to the Cache::Memcached constructor. It also introduces a UNITCHECK block to C4::Context so that memcached_params() can be used during intialization of the module. This patch also corrects SQLHelper so that it no longer tries to use the deprecated (and removed) memcached_servers setting in koha-conf.xml; it now gets the memcached connection parameters from C4::Context->memcached_params(). To test: [1] This patch is most easily tested on Debian Wheezy, as version 0.03-2 of the libmemoize-memcached-perl package includes the fix for Debian bug 614868 that causes Koha's use of Memoize::Memcached to break. If you don't want to go so far as to install/upgrade to Wheezy, you can apply the following Debian patch to your copy of Memoize::Memcached to get the effect: http://patch-tracker.debian.org/patch/series/view/libmemoize-memcached-perl/... [2] Set up your Koha Apache configuration to point to a memcached server. [3] Restart memcached to start with a clear cache. [4] Do the following steps: - Create a new bib record. Note that it is sufficient to just open the editor; you don't need to save the bib. - Search for a patron record. [5] Use the memcdump command-line to get a list of keys in your memcached cache. You should *not* see keys that contain the strings GetMarcStructure, getTranslatedLanguages, or _get_columns. [6] Apply the patch, then go through step 4 again. [7] Use memcdump again. This time, you *should* see the keys mentioned in step 5. [8] Set the SessionStorage system preference to memcached. After logging in again, verify that your staff interface and OPAC sessions work without requiring that you log in for each and every page load. Signed-off-by: Galen Charlton <gmc@esilibrary.com> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10503 --- Comment #2 from Galen Charlton <gmcharlt@gmail.com> --- Note to testers: After running through the test plan, this is what I got when I ran memcdump: $ memcdump --servers 127.0.0.1:11211 KOHAGMCismemcached KOHAGMC970239eca4adb0bb2bb8f4dc761a3023 KOHAGMCmemoize-getTranslatedLanguages-scalar-intranet KOHAGMCkohaconf KOHAGMCmemoize-_get_columns-scalar-borrowers KOHAGMCmemoize-GetMarcStructure-scalar-1 The key names will vary based on what you set MEMCACHED_NAMESPACE to. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10503 Galen Charlton <gmcharlt@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #19232|0 |1 is obsolete| | --- Comment #3 from Galen Charlton <gmcharlt@gmail.com> --- Created attachment 19236 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=19236&action=edit bug 10503: fix various issues with use of Memoize::Memcached Memoize::Memcached expects to be passed a hashref of connection parameters, which it in turn uses to initialize a Cache::Memcached object. However, commit 52afe06ddd started passing a Cache::Memcached object directly, not the required hashref. This happened to work by coincidence, but no longer works with recent versions of Memoize::Memcached packaged for Debian. This patch introduces a new C4::Context function, memcached_params(), that returns a hashref of connection parameters suitable for passing to the Cache::Memcached constructor. It also introduces a UNITCHECK block to C4::Context so that memcached_params() can be used during intialization of the module. This patch also corrects SQLHelper so that it no longer tries to use the deprecated (and removed) memcached_servers setting in koha-conf.xml; it now gets the memcached connection parameters from C4::Context->memcached_params(). To test: [1] This patch is most easily tested on Debian Wheezy, as version 0.03-2 of the libmemoize-memcached-perl package includes the fix for Debian bug 614868 that causes Koha's use of Memoize::Memcached to break. If you don't want to go so far as to install/upgrade to Wheezy, you can apply the following Debian patch to your copy of Memoize::Memcached to get the effect: http://patch-tracker.debian.org/patch/series/view/libmemoize-memcached-perl/... [2] Set up your Koha Apache configuration to point to a memcached server. [3] Restart memcached to start with a clear cache. [4] Do the following steps: - Create a new bib record. Note that it is sufficient to just open the editor; you don't need to save the bib. - Search for a patron record. [5] Use the memcdump command-line to get a list of keys in your memcached cache. You should *not* see keys that contain the strings GetMarcStructure, getTranslatedLanguages, or _get_columns. [6] Apply the patch, then go through step 4 again. [7] Use memcdump again. This time, you *should* see the keys mentioned in step 5. [8] Set the SessionStorage system preference to memcached. After logging in again, verify that your staff interface and OPAC sessions work without requiring that you log in for each and every page load. Signed-off-by: Galen Charlton <gmc@esilibrary.com> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10503 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |katrin.fischer@bsz-bw.de -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10503 Srdjan Jankovic <srdjan@catalyst.net.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |srdjan@catalyst.net.nz --- Comment #4 from Srdjan Jankovic <srdjan@catalyst.net.nz> --- I can see GetMarcStructure and getTranslatedLanguages, but not _get_columns -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10503 --- Comment #5 from Galen Charlton <gmcharlt@gmail.com> --- (In reply to Srdjan Jankovic from comment #4)
I can see GetMarcStructure and getTranslatedLanguages, but not _get_columns
I tried again and it works for me: $ memcdump --servers localhost:11211 KOHAismemcached KOHAmemoize-getTranslatedLanguages-scalar-intranet KOHAmemoize-getTranslatedLanguages-list-intranet KOHAkohaconf KOHAmemoize-_get_columns-scalar-borrowers Could you try again, and if it doesn't work, provide more details about your test environment? -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10503 Srdjan Jankovic <srdjan@catalyst.net.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10503 Srdjan Jankovic <srdjan@catalyst.net.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #19236|0 |1 is obsolete| | --- Comment #6 from Srdjan Jankovic <srdjan@catalyst.net.nz> --- Created attachment 19735 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=19735&action=edit [SIGNED-OFF] bug 10503: fix various issues with use of Memoize::Memcached Memoize::Memcached expects to be passed a hashref of connection parameters, which it in turn uses to initialize a Cache::Memcached object. However, commit 52afe06ddd started passing a Cache::Memcached object directly, not the required hashref. This happened to work by coincidence, but no longer works with recent versions of Memoize::Memcached packaged for Debian. This patch introduces a new C4::Context function, memcached_params(), that returns a hashref of connection parameters suitable for passing to the Cache::Memcached constructor. It also introduces a UNITCHECK block to C4::Context so that memcached_params() can be used during intialization of the module. This patch also corrects SQLHelper so that it no longer tries to use the deprecated (and removed) memcached_servers setting in koha-conf.xml; it now gets the memcached connection parameters from C4::Context->memcached_params(). To test: [1] This patch is most easily tested on Debian Wheezy, as version 0.03-2 of the libmemoize-memcached-perl package includes the fix for Debian bug 614868 that causes Koha's use of Memoize::Memcached to break. If you don't want to go so far as to install/upgrade to Wheezy, you can apply the following Debian patch to your copy of Memoize::Memcached to get the effect: http://patch-tracker.debian.org/patch/series/view/libmemoize-memcached-perl/... [2] Set up your Koha Apache configuration to point to a memcached server. [3] Restart memcached to start with a clear cache. [4] Do the following steps: - Create a new bib record. Note that it is sufficient to just open the editor; you don't need to save the bib. - Search for a patron record. [5] Use the memcdump command-line to get a list of keys in your memcached cache. You should *not* see keys that contain the strings GetMarcStructure, getTranslatedLanguages, or _get_columns. [6] Apply the patch, then go through step 4 again. [7] Use memcdump again. This time, you *should* see the keys mentioned in step 5. [8] Set the SessionStorage system preference to memcached. After logging in again, verify that your staff interface and OPAC sessions work without requiring that you log in for each and every page load. Signed-off-by: Galen Charlton <gmc@esilibrary.com> Signed-off-by: Srdjan <srdjan@catalyst.net.nz> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10503 --- Comment #7 from Srdjan Jankovic <srdjan@catalyst.net.nz> --- Sorry, it is there. I'm going to see my optometrist today, that should help. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10503 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart@biblibre.co | |m --- Comment #8 from Jonathan Druart <jonathan.druart@biblibre.com> --- Looks good to me, the patch works as expected on Debian Wheezy.
This patch also corrects SQLHelper so that it no longer tries to use the deprecated (and removed) memcached_servers setting in koha-conf.xml;
Why do you say that? it is still in used by Koha/Template/Plugin/Cache.pm and appears in debian/templates/koha-conf-site.xml.in. Should we remove these occurrences too? -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10503 Mason James <mtj@kohaaloha.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |In Discussion CC| |mtj@kohaaloha.com --- Comment #9 from Mason James <mtj@kohaaloha.com> --- (In reply to Jonathan Druart from comment #8)
Looks good to me, the patch works as expected on Debian Wheezy.
This patch also corrects SQLHelper so that it no longer tries to use the deprecated (and removed) memcached_servers setting in koha-conf.xml;
Why do you say that? it is still in used by Koha/Template/Plugin/Cache.pm and appears in debian/templates/koha-conf-site.xml.in.
Should we remove these occurrences too?
hmm, lets switch to 'in discussion'... -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10503 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jcamins@cpbibliography.com -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10503 --- Comment #10 from Jared Camins-Esakov <jcamins@cpbibliography.com> --- (In reply to Jonathan Druart from comment #8)
Looks good to me, the patch works as expected on Debian Wheezy.
This patch also corrects SQLHelper so that it no longer tries to use the deprecated (and removed) memcached_servers setting in koha-conf.xml;
Why do you say that? it is still in used by Koha/Template/Plugin/Cache.pm and appears in debian/templates/koha-conf-site.xml.in.
Should we remove these occurrences too?
That would be a good idea, yes. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10503 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|In Discussion |Failed QA --- Comment #11 from Jonathan Druart <jonathan.druart@biblibre.com> --- I think the 2 occurrences remaining should be replace. Marked as Failed QA. -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org