Bug 28436 - Cache relatively static info that is used often and on many pages (instead of fetching from DB)
Summary: Cache relatively static info that is used often and on many pages (instead of...
Status: RESOLVED MOVED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: Main
Hardware: All All
: P5 - low minor (vote)
Assignee: Bugs List
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2021-05-24 12:52 UTC by Joonas Kylmälä
Modified: 2023-07-01 09:35 UTC (History)
3 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 28436: Add local cache to Template Plugins for branches and itemtypes (2.43 KB, patch)
2021-09-08 12:40 UTC, Nick Clemens
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Joonas Kylmälä 2021-05-24 12:52:40 UTC
We should try to cache (Koha::Cache) some of the most common info that is used on many of our OPAC and intranet pages in order to prevent making very slow SQL queries. E.g. I can cut the load time of biblio detail.pl page by several seconds by just storing the Koha::Template::Plugin::Branches::GetName results in cache/RAM (when the bib has around 1000 items).

Similar caching should be done for Koha::Template::Plugin::Categories::GetName, and so on.
Comment 1 Nick Clemens 2021-09-08 12:40:28 UTC Comment hidden (obsolete)
Comment 2 Nick Clemens 2021-09-08 12:42:28 UTC Comment hidden (obsolete)
Comment 3 David Cook 2021-09-09 02:39:51 UTC
(In reply to Nick Clemens from comment #2)
> We can definitely add more plugins here.
> 
> This feels a bit hacky, however, it reduces to a single lookup per page,
> avoids problems with cache coherence and is readable

Actually, it looks like there might be a cache coherence problem, since the our-scoped variable will exist for the life of the process. You could wind up with stale data if data is cached, the worker stays alive, data is changed in the staff interface, and then requests are sent to that original worker.

I'd suggest just caching at the request-level. 

I would suggest creating a new() method which sets an object variable with the required data. According to http://www.template-toolkit.org/docs/modules/Template/Plugin.html, the new() method will be called in the USE directive. 

This means that at template processing time you'll get 1 lookup, and then you can reuse that data for the life of that request. 

There are even cache examples on that TT link above.
Comment 4 David Cook 2021-09-09 02:42:47 UTC
It would be more robust and more readable. 

Or if we wanted to persist the cache beyond the request-level we could use Koha::Cache, and rely on the staff interface for Libraries and Item Types to invalidate the cache. 

That would be even better. 

That Koha::Cache could also be placed in the respective modules and have something like Koha::ItemTypes->get_list() or whatever and have the specific module handle it's own caching logic. 

Lots of options which would be less hacky and more robust.
Comment 5 David Cook 2021-09-09 02:43:31 UTC
But super keen to see this work go ahead. We do far too many unnecessary DB calls in Koha...
Comment 6 David Cook 2021-09-09 02:45:58 UTC
Since the patch is for template plugins, it's probably worth changing the title to reflect that the update is just for template plugins too
Comment 7 Fridolin Somers 2021-09-11 06:22:20 UTC
(In reply to David Cook from comment #4)
> It would be more robust and more readable. 
> 
> Or if we wanted to persist the cache beyond the request-level we could use
> Koha::Cache, and rely on the staff interface for Libraries and Item Types to
> invalidate the cache. 
> 
> That would be even better. 
> 
> That Koha::Cache could also be placed in the respective modules and have
> something like Koha::ItemTypes->get_list() or whatever and have the specific
> module handle it's own caching logic. 
> 
> Lots of options which would be less hacky and more robust.

Indeed.
I think we should use Memcached for those datas, they are not often changed.
Comment 8 Fridolin Somers 2021-11-11 01:05:54 UTC
Duplicate of Bug 29454 no ?
Comment 9 David Cook 2021-11-11 01:59:46 UTC
(In reply to Fridolin Somers from comment #8)
> Duplicate of Bug 29454 no ?

Yeah I'm not sure why that was opened as a new one..
Comment 10 Nick Clemens 2021-11-12 12:16:06 UTC
(In reply to David Cook from comment #9)
> (In reply to Fridolin Somers from comment #8)
> > Duplicate of Bug 29454 no ?
> 
> Yeah I'm not sure why that was opened as a new one..

Joonas opened this one with broader context, and comments indicated that memcached would be a good way to go. I moved mine to a new bug as it deals narrowly with 2 plugins and doesn't use the Koha cache, I could have changed the title and reopened a new bug, but moving my patches seemed cleaner
Comment 11 Katrin Fischer 2023-07-01 09:35:18 UTC
This was probably meant to be an omnibus bug, but as there are no bugs linked, I am inclined to close this as RESOLVED MOVED for now (indicating that it's been worked on on other bugs). Please reopen if you disagree!