[Bug 9434] New: Static variables used for caches are not Plack-friendly
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9434 Bug ID: 9434 Summary: Static variables used for caches are not Plack-friendly Classification: Unclassified Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: major Priority: P5 - low Component: Architecture, internals, and plumbing Assignee: gmcharlt@gmail.com Reporter: jcamins@cpbibliography.com At the moment we cache numerous pieces of information in module-level variables which then do not get updated in other threads/processes when they are changed by the user. This is a serious usability issue. Examples of this include the way we treat sysprefs (there is now a method to disable the syspref cache, but by default it is enabled), notices, frameworks, field mappings, and koha-conf.xml, at least. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9434 Jared Camins-Esakov <jcamins@cpbibliography.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff Patch complexity|--- |Large patch -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9434 --- Comment #1 from Jared Camins-Esakov <jcamins@cpbibliography.com> --- Created attachment 14764 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=14764&action=edit Bug 9434: Introduce new tied cache system At the moment we cache numerous pieces of information in module-level variables which then do not get updated in other threads/processes when they are changed by the user. This is a serious usability issue. Examples of this include the way we treat sysprefs (there is now a method to disable the syspref cache, but by default it is enabled), notices, frameworks, field mappings, and koha-conf.xml, at least. This patch sets the stage for eliminating this problem by making it possible to convert module-level cache variables into variables that are actually backed by whatever caching system may be configured. This is done through a special Koha::Cache::Object class which can be tied to the variables that are being used for caching and provided with a constructor method/closure to allow the cache to be reloaded when it expires. For example: my $cache = Koha::Cache->new(); my $data = 'whatever'; my $variable = Koha::Cache->create_scalar( { 'key' => 'whatever', 'timeout' => 2, 'constructor' => sub { return $data; }, } ); print "$$variable\n"; # Prints "whatever" The one change this necessitates for accessing the data is that the variable must be dereferenced an additional time before use (i.e. $$variable instead of $variable). There is no difference when the variable tied is a hash (created with Koha::Cache->create_hash). This is a small price to pay for Koha working in a multi-threaded, persistent environment. This change will also make caching easier in general. CHI was incompatible with the variable tying, so this patch also removes the dependency on CHI, using instead Cache::Memcached::Fast, Cache::FastMmap, and Cache::Memory, when they are available. To test: 1) Apply patch. 2) Run unit test t/Cache.t (after setting the MEMCACHED_SERVERS and CACHING_SYSTEM environment variables). As no changes were made to the tests already in that file, this passing demonstrates there are no regressions. 3) With memcached caching enabled (you must set the MEMCACHED_SERVERS and CACHING_SYSTEM environment variables) and DEBUG turned on (i.e. the DEBUG environment variable set to 1), try running a report via the web service ([intranet]/cgi-bin/koha/svc/report?id=1 and check your web server logs to confirm that there are messages like "get_from_cache for intranet:report:id:1" in them. 4) If the reports worked, sign off. NOTE: Technically you could test this without needing memcached by installing libcache-fastmmap-perl and setting CACHING_SYSTEM to 'fastmmap' instead of 'memcached'. You could also install libcache-perl and set CACHING_SYSTEM to 'memory' but there would be little point as the cached variables would go out of scope in between runs. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9434 Jared Camins-Esakov <jcamins@cpbibliography.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |8089 -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9434 Jared Camins-Esakov <jcamins@cpbibliography.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |8089 Depends on|8089 | -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9434 Jared Camins-Esakov <jcamins@cpbibliography.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #14764|0 |1 is obsolete| | --- Comment #2 from Jared Camins-Esakov <jcamins@cpbibliography.com> --- Created attachment 14786 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=14786&action=edit Bug 9434: Introduce new tied cache system At the moment we cache numerous pieces of information in module-level variables which then do not get updated in other threads/processes when they are changed by the user. This is a serious usability issue. Examples of this include the way we treat sysprefs (there is now a method to disable the syspref cache, but by default it is enabled), notices, frameworks, field mappings, and koha-conf.xml, at least. This patch sets the stage for eliminating this problem by making it possible to convert module-level cache variables into variables that are actually backed by whatever caching system may be configured. This is done through a special Koha::Cache::Object class which can be tied to the variables that are being used for caching and provided with a constructor method/closure to allow the cache to be reloaded when it expires. For example: my $cache = Koha::Cache->new(); my $data = 'whatever'; my $variable = Koha::Cache->create_scalar( { 'key' => 'whatever', 'timeout' => 2, 'constructor' => sub { return $data; }, } ); print "$$variable\n"; # Prints "whatever" The one change this necessitates for accessing the data is that the variable must be dereferenced an additional time before use (i.e. $$variable instead of $variable). There is no difference when the variable tied is a hash (created with Koha::Cache->create_hash). This is a small price to pay for Koha working in a multi-threaded, persistent environment. This change will also make caching easier in general. CHI was incompatible with the variable tying, so this patch also removes the dependency on CHI, using instead Cache::Memcached::Fast, Cache::FastMmap, and Cache::Memory, when they are available. To test: 1) Apply patch. 2) Run unit test t/Cache.t (after setting the MEMCACHED_SERVERS and CACHING_SYSTEM environment variables). As no changes were made to the tests already in that file, this passing demonstrates there are no regressions. 3) With memcached caching enabled (you must set the MEMCACHED_SERVERS and CACHING_SYSTEM environment variables) and DEBUG turned on (i.e. the DEBUG environment variable set to 1), try running a report via the web service ([intranet]/cgi-bin/koha/svc/report?id=1 and check your web server logs to confirm that there are messages like "get_from_cache for intranet:report:id:1" in them. 4) If the reports worked, sign off. NOTE: Technically you could test this without needing memcached by installing libcache-fastmmap-perl and setting CACHING_SYSTEM to 'fastmmap' instead of 'memcached'. You could also install libcache-perl and set CACHING_SYSTEM to 'memory' but there would be little point as the cached variables would go out of scope in between runs. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9434 Jonathan Druart <jonathan.druart@biblibre.com> 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=9434 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #14786|0 |1 is obsolete| | --- Comment #3 from Jonathan Druart <jonathan.druart@biblibre.com> --- Created attachment 14894 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=14894&action=edit Bug 9434: Introduce new tied cache system At the moment we cache numerous pieces of information in module-level variables which then do not get updated in other threads/processes when they are changed by the user. This is a serious usability issue. Examples of this include the way we treat sysprefs (there is now a method to disable the syspref cache, but by default it is enabled), notices, frameworks, field mappings, and koha-conf.xml, at least. This patch sets the stage for eliminating this problem by making it possible to convert module-level cache variables into variables that are actually backed by whatever caching system may be configured. This is done through a special Koha::Cache::Object class which can be tied to the variables that are being used for caching and provided with a constructor method/closure to allow the cache to be reloaded when it expires. For example: my $cache = Koha::Cache->new(); my $data = 'whatever'; my $variable = Koha::Cache->create_scalar( { 'key' => 'whatever', 'timeout' => 2, 'constructor' => sub { return $data; }, } ); print "$$variable\n"; # Prints "whatever" The one change this necessitates for accessing the data is that the variable must be dereferenced an additional time before use (i.e. $$variable instead of $variable). There is no difference when the variable tied is a hash (created with Koha::Cache->create_hash). This is a small price to pay for Koha working in a multi-threaded, persistent environment. This change will also make caching easier in general. CHI was incompatible with the variable tying, so this patch also removes the dependency on CHI, using instead Cache::Memcached::Fast, Cache::FastMmap, and Cache::Memory, when they are available. To test: 1) Apply patch. 2) Run unit test t/Cache.t (after setting the MEMCACHED_SERVERS and CACHING_SYSTEM environment variables). As no changes were made to the tests already in that file, this passing demonstrates there are no regressions. 3) With memcached caching enabled (you must set the MEMCACHED_SERVERS and CACHING_SYSTEM environment variables) and DEBUG turned on (i.e. the DEBUG environment variable set to 1), try running a report via the web service ([intranet]/cgi-bin/koha/svc/report?id=1 and check your web server logs to confirm that there are messages like "get_from_cache for intranet:report:id:1" in them. 4) If the reports worked, sign off. NOTE: Technically you could test this without needing memcached by installing libcache-fastmmap-perl and setting CACHING_SYSTEM to 'fastmmap' instead of 'memcached'. You could also install libcache-perl and set CACHING_SYSTEM to 'memory' but there would be little point as the cached variables would go out of scope in between runs. Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9434 Elliott Davis <elliott@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA CC| |elliott@bywatersolutions.co | |m --- Comment #4 from Elliott Davis <elliott@bywatersolutions.com> --- I am getting a failed test with the unit test provided. elliott@ByWater-Test:~/kohaclone$ prove ./t/Cache.t ./t/Cache.t .. 1/29 Use of uninitialized value $ip in hash element at /usr/share/perl5/Cache/Memcached.pm line 253. Use of uninitialized value in subroutine entry at /usr/share/perl5/Cache/Memcached.pm line 288. # Failed test 'use C4::Context;' # at ./t/Cache.t line 15. # Tried to use 'C4::Context'. # Error: Bad arg length for Socket::pack_sockaddr_in, length is 0, should be 4 at /usr/lib/perl/5.10/Socket.pm line 214. # BEGIN failed--compilation aborted at /home/elliott/kohaclone/C4/Context.pm line 99. # Compilation failed in require at (eval 14) line 2. # BEGIN failed--compilation aborted at (eval 14) line 2. ./t/Cache.t .. 5/29 # Failed test 'fetching valid item from cache' # at ./t/Cache.t line 39. # got: undef # expected: 'I AM MORE DATA' # Failed test 'fetching valid item from cache (after clearing another item)' # at ./t/Cache.t line 49. # got: undef # expected: 'I AM MORE DATA22' # Failed test 'Data retrieved from cache' # at ./t/Cache.t line 76. # got: '2' # expected: '1' ./t/Cache.t .. 14/29 # Failed test 'Constructor called again when timeout reached' # at ./t/Cache.t line 78. # got: '3' # expected: '2' # Failed test 'Stored new value to cache' # at ./t/Cache.t line 80. # got: '4' # expected: '5' # Failed test 'Constructor not called after storing value' # at ./t/Cache.t line 81. # got: '4' # expected: '2' # Failed test 'Setting hash member worked' # at ./t/Cache.t line 104. # got: undef # expected: 'surprise' ./t/Cache.t .. 25/29 # Looks like you failed 8 tests of 29. ./t/Cache.t .. Dubious, test returned 8 (wstat 2048, 0x800) Failed 8/29 subtests Test Summary Report ------------------- ./t/Cache.t (Wstat: 2048 Tests: 29 Failed: 8) Failed tests: 3, 6, 8, 13-16, 21 Non-zero exit status: 8 Files=1, Tests=29, 8 wallclock secs ( 0.04 usr 0.01 sys + 0.14 cusr 0.03 csys = 0.22 CPU) Result: FAIL -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9434 Paul Poulain <paul.poulain@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Passed QA CC| |paul.poulain@biblibre.com Summary|Static variables used for |Introduce new tied cache |caches are not |system |Plack-friendly | QA Contact| |paul.poulain@biblibre.com --- Comment #5 from Paul Poulain <paul.poulain@biblibre.com> --- (In reply to comment #4)
I am getting a failed test with the unit test provided. Failed 8/29 subtests
The fail probably came from the fact that the following prerequisite were failing: sudo apt-get install libcache-fastmmap-perl export CACHING_SYSTEM=fastmmap It works OK for me when I have it, and I get errors when I don't. So QAing again: * passes koha-qa.pl * unit test provided * I trieds without caching and couldn't see anything wrong. I tried with cache enabled, and it still work. passed QA (and changed the bug description, to reflect the patch description) -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9434 Paul Poulain <paul.poulain@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #14894|0 |1 is obsolete| | --- Comment #6 from Paul Poulain <paul.poulain@biblibre.com> --- Created attachment 16547 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=16547&action=edit [PASSED QA] Bug 9434: Introduce new tied cache system At the moment we cache numerous pieces of information in module-level variables which then do not get updated in other threads/processes when they are changed by the user. This is a serious usability issue. Examples of this include the way we treat sysprefs (there is now a method to disable the syspref cache, but by default it is enabled), notices, frameworks, field mappings, and koha-conf.xml, at least. This patch sets the stage for eliminating this problem by making it possible to convert module-level cache variables into variables that are actually backed by whatever caching system may be configured. This is done through a special Koha::Cache::Object class which can be tied to the variables that are being used for caching and provided with a constructor method/closure to allow the cache to be reloaded when it expires. For example: my $cache = Koha::Cache->new(); my $data = 'whatever'; my $variable = Koha::Cache->create_scalar( { 'key' => 'whatever', 'timeout' => 2, 'constructor' => sub { return $data; }, } ); print "$$variable\n"; # Prints "whatever" The one change this necessitates for accessing the data is that the variable must be dereferenced an additional time before use (i.e. $$variable instead of $variable). There is no difference when the variable tied is a hash (created with Koha::Cache->create_hash). This is a small price to pay for Koha working in a multi-threaded, persistent environment. This change will also make caching easier in general. CHI was incompatible with the variable tying, so this patch also removes the dependency on CHI, using instead Cache::Memcached::Fast, Cache::FastMmap, and Cache::Memory, when they are available. To test: 1) Apply patch. 2) Run unit test t/Cache.t (after setting the MEMCACHED_SERVERS and CACHING_SYSTEM environment variables). As no changes were made to the tests already in that file, this passing demonstrates there are no regressions. 3) With memcached caching enabled (you must set the MEMCACHED_SERVERS and CACHING_SYSTEM environment variables) and DEBUG turned on (i.e. the DEBUG environment variable set to 1), try running a report via the web service ([intranet]/cgi-bin/koha/svc/report?id=1 and check your web server logs to confirm that there are messages like "get_from_cache for intranet:report:id:1" in them. 4) If the reports worked, sign off. NOTE: Technically you could test this without needing memcached by installing libcache-fastmmap-perl and setting CACHING_SYSTEM to 'fastmmap' instead of 'memcached'. You could also install libcache-perl and set CACHING_SYSTEM to 'memory' but there would be little point as the cached variables would go out of scope in between runs. Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com> Signed-off-by: Paul Poulain <paul.poulain@biblibre.com> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9434 Jared Camins-Esakov <jcamins@cpbibliography.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to Master --- Comment #7 from Jared Camins-Esakov <jcamins@cpbibliography.com> --- This patch has been pushed to master. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9434 Mason James <mtj@kohaaloha.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks|8089 | Referenced Bugs: http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8089 [Bug 8089] Use Koha::Cache everywhere -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9434 Mason James <mtj@kohaaloha.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |8089 Referenced Bugs: http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8089 [Bug 8089] Use Koha::Cache everywhere -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon@bugs.koha-community.org